For other languages, please see theChromium style guides.
Chromium follows theAndroid Open Source style guide unless an exception is listed below.
You can propose changes to this style guide by sending an email tojava@chromium.org
. 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.
A variable declaration can use thevar
keyword in place of the type (similar to theauto
keyword in C++). In line with theguidance for C++, thevar
keyword may be used when it aids readability and the type of the value is already clear (ex.var bundle = new Bundle()
is OK, butvar something = returnValueIsNotObvious()
may be unclear to readers who are new to this part of the code).
Thevar
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.}
A quick primer:
Throwable
: Base class for all exceptionsError
: Base class for exceptions which are meant to crash the app.Exception
: Base class for exceptions that make sense thecatch
.RuntimeException
: Base class for exceptions that do not need to be declared asthrows
(“unchecked exceptions”).Use catch statements that do not catch exceptions they are not meant to.
catch (Throwable t)
, since that includes the (generally unrecoverable)Error
types.Usecatch (Exception e)
when working with OS APIs that might throw (assuming the program can recover from them).
IllegalStateException
/IllegalArgumentException
/SecurityException
being thrown where onlyRemoteException
was being caught. Unless catch handlers will differ based on exception type, just catchException
.Do not usecatch (RuntimeException e)
.
RuntimeException
to make unchecked exception types, but the type does not make much sense incatch
clauses, as there are not times when you'd want to catch all unchecked exceptions, but not also want to catch all checked exceptions.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:thrownewRuntimeException("Failed to parse a file.", e);// Good - conveys that this block failed along with the "caused by" exception.thrownewRuntimeException(e);// Good - adds useful information.thrownewRuntimeException(String.format("Failed to parse %s", fileName), e);}
It is common to wrap a checked exception with a RuntimeException for cases where a checked exception is not recoverable, or not possible. In order to reduce the number of stack trace “caused by” clauses, and to save on binary size, useJavaUtils.throwUnchecked()
instead.
try{ somethingThatThrowsIOException();}catch(IOException e){// Bad - RuntimeException adds no context and creates longer stack traces.thrownewRuntimeException(e);// Good - Original exception is preserved.throwJavaUtils.throwUnchecked(e);}
throwUnchecked()
when the exception may want to be caught.The build system:
// 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, useBuildConfig.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
andassert
are similar, but our guidance for them differs:
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()
orgetDescription()
) instead oftoString()
when dynamic dispatch is not required.
toString()
is unused, so overrides will not be stripped when unused.// Banned.recordRectangle(float length,float width){}
Rationale:
@AutoValue
generateequals()
,hashCode()
, andtoString()
, whichR8
is unable to remove when unused.record
requires build system work (crbug/1493366).Example withequals()
andhashCode()
:
publicclassValueClass{privatefinalSomeClass mObjMember;privatefinalint mIntMember;@Overridepublicboolean equals(Object o){return oinstanceofValueClass vc&&Objects.equals(mObjMember, vc.mObjMember)&& mIntMember== vc.mIntMember;}@Overridepublicint hashCode(){returnObjects.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 withGoogle's Java style guide andAndroid's Java style guide, never overrideObject.finalize()
.
Custom finalizers:
Classes that need destructor logic should provide an explicitdestroy()
method. UseLifetimeAssert to ensure in debug builds and tests thatdestroy()
is called.
A migration to add@NullMarked
to all Java files is currently underway (crbug.com/389129271). Seenullaway.md for how to use@Nullable
and related annotations.
Android provides the ability to bundle copies ofjava.*
APIs alongside application code, known asJava Library Desugaring. However, since this bundling comes with a performance cost, Chrome does not use it. Treatjava.*
APIs the same as you wouldandroid.*
ones and guard them withBuild.VERSION.SDK_INT
checkswhen necessary. The one exception is if the method isdirectly 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 correspondingBuild.VERSION.SDK_INT
guard or@RequiresApi
annotation.
org.chromium.base.Log
instead ofandroid.util.Log
.%s
support, and ensures log stripping works correctly.Log.w()
andLog.e()
.Log.d(TAG,"There are %d cats", countCats());// countCats() not stripped.
UsingJava streams outside of tests is strongly 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 than their loop equivalents (seecrbug.com/344943957 for examples).
Theparallel()
andparallelStream()
APIs are simpler than their loop equivalents, but are banned due to a lack of a compelling use case in Chrome. If you find one, please discuss onjava@chromium.org
.
Use ofstream()
without a lambda / method reference is allowed. E.g.:
@SuppressWarnings("NoStreams")privatestaticList<Integer> boxInts(int[] arr){returnArrays.stream(arr).boxed().collect(Collectors.toList());}@SuppressWarnings("NoStreams")privatestaticList<String> readLines(BufferedReader bufferedReader){return bufferedReader.lines().collect(Collectors.toList());}
androidx.annotation.Nullable
?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@interfaceContactsPickerAction{int CANCEL=0;int CONTACTS_SELECTED=1;int SELECT_ALL=2;int UNDO_SELECT_ALL=3;int NUM_ENTRIES=4;}// ...void onContactsPickerUserAction(@ContactsPickerActionint action,...);
Values ofInteger
type are also supported, which allows using a sentinelnull
if needed.
TODO(username): Some sentence here.
TODO(crbug.com/40192027): Even better to use a bug for context.
Useparameter comments when they aid in the readability of a function call.
E.g.:
someMethod(/* enabled= */true,/* target= */null, defaultValue);
Conditional braces should be used, but are optional if the conditional and the statement can be on a single line.
Do:
if(someConditional)returnfalse;for(int i=0; i<10;++i) callThing(i);
or
if(someConditional){returnfalse;}
Do NOT do:
if(someConditional)returnfalse;
This is the order of the import groups:
Googlers, seego/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.
setFooForTesting() [...]
method to make the test contract explicit.ResettersForTesting.register()
from withinForTesting()
methods to ensure that state is reset between tests.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 restartFunctions and fields used only for testing should haveForTesting
as a suffix so that:
android-binary-size
trybot canensure they are removed in non-test optimized builds (by R8).PRESUMBIT.py
can ensure no calls are made to such methods outside of tests, andForTesting
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 aForTesting
suffixshould not be annotated with@VisibleForTesting
. Whileotherwise=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 nameorg.chromium.<top level directory>
. Each top level directory's Java should build into a distinct JAR that honors the abstraction specified in a nativecheckdeps (e.g.org.chromium.base
does not importorg.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 namedbase/android/java/org/chromium/base/Class.java
. This would get compiled into achromium_base.jar
(final JAR name TBD).
org.chromium.chrome.browser.foo.Class
would live inchrome/android/java/org/chromium/chrome/browser/foo/Class.java
.
New<top level directory>/android
directories should have anOWNERS
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 rungit cl format
to apply the automatic formatting.
Chromium also makes use of severalstatic analysis tools.