For other languages, please see the Chromium style guides.
Chromium follows the Android Open Source style guide unless an exception is listed below.
You can propose changes to this style guide by sending an email to
[email protected]
. Ideally, the list will arrive at some consensus and you can
request review for a change to this file. If there's no consensus,
//styleguide/java/OWNERS
get to decide.
[TOC]
A variable declaration can use the var
keyword in place of the type (similar
to the auto
keyword in C++). In line with the guidance for
C++, the
var
keyword may be used when it aids readability and the type of the value is
already clear (ex. var bundle = new Bundle()
is OK, but var something = returnValueIsNotObvious()
may be unclear to readers who are new to this part of
the code).
The var
keyword may also be used in try-with-resources when the resource is
not directly accessed (or when it falls under the previous guidance), such as:
try (var ignored = StrictModeContext.allowDiskWrites()) {
// 'var' is permitted so long as the 'ignored' variable is not used directly
// in the code.
}
We discourage overly broad catches via Throwable
, Exception
, or
RuntimeException
, except when dealing with RemoteException
or similar
system APIs.
- There have been many cases of crashes caused by
IllegalStateException
/IllegalArgumentException
/SecurityException
being thrown where onlyRemoteException
was being caught. In these cases, usecatch (RemoteException | RuntimeException e)
. - For all broad catch expressions, add a comment to explain why.
Avoid adding messages to exceptions that do not aid in debugging. For example:
try {
somethingThatThrowsIOException();
} catch (IOException e) {
// Bad - message does not tell you more than the stack trace does:
throw new RuntimeException("Failed to parse a file.", e);
// Good - conveys that this block failed along with the "caused by" exception.
throw new RuntimeException(e);
// Good - adds useful information.
throw new RuntimeException(String.format("Failed to parse %s", fileName), e);
}
The build system:
- strips asserts in release builds (via R8),
- enables them in debug builds,
- and enables them in report-only mode for Canary builds.
// Code for assert expressions & messages is removed when asserts are disabled.
assert someCallWithoutSideEffects(param) : "Call failed with: " + param;
Use your judgement for when to use asserts vs exceptions. Generally speaking, use asserts to check program invariants (e.g. parameter constraints) and exceptions for unrecoverable error conditions (e.g. OS errors). You should tend to use exceptions more in privacy / security-sensitive code.
Do not add checks when the code will crash anyways. E.g.:
// Don't do this.
assert(foo != null);
foo.method(); // This will throw anyways.
For multi-statement asserts, use BuildConfig.ENABLE_ASSERTS
to guard your
code (similar to #if DCHECK_IS_ON()
in C++). E.g.:
import org.chromium.build.BuildConfig;
...
if (BuildConfig.ENABLE_ASSERTS) {
// Any code here will be stripped in release builds by R8.
...
}
DCHECK
and assert
are similar, but our guidance for them differs:
- CHECKs are preferred in C++, whereas asserts are preferred in Java.
This is because as a memory-safe language, logic bugs in Java are much less likely to be exploitable.
Use explicit serialization methods (e.g. toDebugString()
or getDescription()
)
instead of toString()
when dynamic dispatch is not required.
- R8 cannot detect when
toString()
is unused, so overrides will not be stripped when unused. - R8 cannot optimize / inline these calls as well as non-overriding methods.
// Banned.
record Rectangle(float length, float width) {}
Rationale:
- To avoid dead code:
- Records and
@AutoValue
generateequals()
,hashCode()
, andtoString()
, whichR8
is unable to remove when unused. - When these methods are required, implement them explicitly so that the intention is clear.
- Records and
- Also - supporting
record
requires build system work (crbug/1493366).
Example with equals()
and hashCode()
:
public class ValueClass {
private final SomeClass mObjMember;
private final int mIntMember;
@Override
public boolean equals(Object o) {
return o instanceof ValueClass vc
&& Objects.equals(mObjMember, vc.mObjMember)
&& mIntMember == vc.mIntMember;
}
@Override
public int hashCode() {
return Objects.hash(mObjMember, mIntMember);
}
}
Banned. Use @IntDef
instead.
Rationale:
Java enums generate a lot of bytecode. Use constants where possible. When a custom type hierarchy is required, use explicit classes with inheritance.
In line with Google's Java style guide and Android's Java style guide,
never override Object.finalize()
.
Custom finalizers:
- are called on a background thread, and at an unpredicatble point in time,
- swallow all exceptions (asserts won't work),
- causes additional garbage collector jank.
Classes that need destructor logic should provide an explicit destroy()
method. Use LifetimeAssert
to ensure in debug builds and tests that destroy()
is called.
Android provides the ability to bundle copies of java.*
APIs alongside
application code, known as Java Library Desugaring. However, since this
bundling comes with a performance cost, Chrome does not use it. Treat java.*
APIs the same as you would android.*
ones and guard them with
Build.VERSION.SDK_INT
checks when necessary. The one exception is if the
method is directly backported by D8 (these are okay to use, since they are
lightweight). Android Lint will fail if you try to use an API without a
corresponding Build.VERSION.SDK_INT
guard or @RequiresApi
annotation.
- Use
org.chromium.base.Log
instead ofandroid.util.Log
.- It provides
%s
support, and ensures log stripping works correctly.
- It provides
- Minimize the use of
Log.w()
andLog.e()
.- Debug and Info log levels are stripped by ProGuard in release builds, and so have no performance impact for shipping builds. However, Warning and Error log levels are not stripped.
- Function calls in log parameters are not stripped by ProGuard.
Log.d(TAG, "There are %d cats", countCats()); // countCats() not stripped.
Most uses of Java streams are discouraged. If you can write your code as an explicit loop, then do so. The primary reason for this guidance is because the lambdas (and method references) needed for streams almost always result in larger binary size (example.
The parallel()
and parallelStream()
APIs are simpler than their loop
equivalents, but are are currently banned due to a lack of a compelling use case
in Chrome. If you find one, please discuss on [email protected]
.
- Use them liberally. They are documented here.
- They generally improve readability.
- Many make lint more useful.
javax.annotation.Nullable
vsandroidx.annotation.Nullable
- Always prefer
androidx.annotation.Nullable
. - It uses
@Retention(SOURCE)
rather than@Retention(RUNTIME)
.
- Always prefer
Values can be declared outside or inside the @interface
. Chromium style is
to declare inside.
@IntDef({ContactsPickerAction.CANCEL, ContactsPickerAction.CONTACTS_SELECTED,
ContactsPickerAction.SELECT_ALL, ContactsPickerAction.UNDO_SELECT_ALL})
@Retention(RetentionPolicy.SOURCE)
public @interface ContactsPickerAction {
int CANCEL = 0;
int CONTACTS_SELECTED = 1;
int SELECT_ALL = 2;
int UNDO_SELECT_ALL = 3;
int NUM_ENTRIES = 4;
}
// ...
void onContactsPickerUserAction(@ContactsPickerAction int action, ...);
Values of Integer
type are also supported, which allows using a sentinel
null
if needed.
- Use the same format as in the C++ style guide.
- TODO should follow chromium convention. Examples:
TODO(username): Some sentence here.
TODO(crbug.com/123456): Even better to use a bug for context.
Use parameter comments when they aid in the readability of a function call.
E.g.:
someMethod(/* enabled= */ true, /* target= */ null, defaultValue);
- Fields should not be explicitly initialized to default values (see here).
Conditional braces should be used, but are optional if the conditional and the statement can be on a single line.
Do:
if (someConditional) return false;
for (int i = 0; i < 10; ++i) callThing(i);
or
if (someConditional) {
return false;
}
Do NOT do:
if (someConditional)
return false;
- Static imports go before other imports.
- Each import group must be separated by an empty line.
This is the order of the import groups:
- android
- androidx
- com (except com.google.android.apps.chrome)
- dalvik
- junit
- org
- com.google.android.apps.chrome
- org.chromium
- java
- javax
Googlers, see go/clank-test-strategy.
In summary:
-
Use real dependencies when feasible and fast. Use Mockito’s
@Mock
most of the time, but write fakes for frequently used dependencies. -
Do not use Robolectric Shadows for Chromium code. Instead, use
setForTesting()
methods so that it is clear that test hooks exist.- When
setForTesting()
methods alter global state, useResettersForTesting.register()
to ensure that the state is reset between tests. Omit resetting them via@After
methods.
- When
-
Use Robolectric when possible (when tests do not require native). Other times, use on-device tests with one of the following annotations:
@Batch(UNIT_TESTS)
for unit tests@Batch(PER_CLASS)
for integration tests@DoNotBatch
for when each test method requires an app restart
Functions and fields used only for testing should have ForTesting
as a
suffix so that:
- The
android-binary-size
trybot can ensure they are removed in non-test optimized builds (by R8). PRESUMBIT.py
can ensure no calls are made to such methods outside of tests, and
ForTesting
methods that are @CalledByNative
should use
@CalledByNativeForTesting
instead.
Symbols that are made public (or package-private) for the sake of tests
should be annotated with @VisibleForTesting
. Android Lint will check
that calls from non-test code respect the "otherwise" visibility.
Symbols with a ForTesting
suffix should not be annotated with
@VisibleForTesting
. While otherwise=VisibleForTesting.NONE
exists, it
is redundant given the "ForTesting" suffix and the associated lint check
is redundant given our trybot check. You should, however, use it for
test-only constructors.
"Top level directories" are defined as directories with a GN file, such as
//base
and
//content,
Chromium Java should live in a directory named
<top level directory>/android/java
, with a package name
org.chromium.<top level directory>
. Each top level directory's Java should
build into a distinct JAR that honors the abstraction specified in a native
checkdeps
(e.g. org.chromium.base
does not import org.chromium.content
). The full
path of any java file should contain the complete package name.
For example, top level directory //base
might contain a file named
base/android/java/org/chromium/base/Class.java
. This would get compiled into a
chromium_base.jar
(final JAR name TBD).
org.chromium.chrome.browser.foo.Class
would live in
chrome/android/java/org/chromium/chrome/browser/foo/Class.java
.
New <top level directory>/android
directories should have an OWNERS
file
much like
//base/android/OWNERS.
google-java-format
is used to auto-format Java files. Formatting of its code
should be accepted in code reviews.
You can run git cl format
to apply the automatic formatting.
Chromium also makes use of several static analysis tools.
- Use UTF-8 file encodings and LF line endings.