Skip to content

Latest commit

 

History

History
302 lines (237 loc) · 11.2 KB

File metadata and controls

302 lines (237 loc) · 11.2 KB

Chromium Java style guide

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]

Java 10 Language Features

Type deduction using var

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.
}

Java 8 Language Features

D8 is used to rewrite some Java 7 & 8 language constructs in a way that is compatible with Java 6 (and thus all Android versions). Use of these features is encouraged.

Java Library APIs

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.

Other Language Features & APIs

Exceptions

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 only RemoteException was being caught. In these cases, use catch (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);
}

Logging

  • Use org.chromium.base.Log instead of android.util.Log.
    • It provides %s support, and ensures log stripping works correctly.
  • Minimize the use of Log.w() and Log.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.

Asserts

The Chromium build system strips asserts in release builds (via ProGuard) and enables them in debug builds (or when dcheck_always_on=true) (via a build step). You should use asserts in the same scenarios where C++ DCHECK()s make sense. For multi-statement asserts, use org.chromium.build.BuildConfig.ENABLE_ASSERTS to guard your code (similar to #if DCHECK_IS_ON() in C++).

Example assert:

assert someCallWithoutSideEffects() : "assert description";

Example use of BuildConfig.ENABLE_ASSERTS:

import org.chromium.build.BuildConfig;

...

if (BuildConfig.ENABLE_ASSERTS) {
  // Any code here will be stripped in Release by ProGuard.
  ...
}

Finalizers

In line with Google'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.

AndroidX Annotations

  • Use them! They are documented here.
    • They generally improve readability.
    • Some make lint more useful.
  • javax.annotation.Nullable vs androidx.annotation.Nullable
    • Always prefer androidx.annotation.Nullable.
    • It uses @Retention(SOURCE) rather than @Retention(RUNTIME).

IntDef Instead of Enum

Java enums generate far more bytecode than integer constants. When integers are sufficient, prefer using an @IntDef annotation, which will have usage checked by Android lint.

Values can be declared outside or inside the @interface. We recommend the latter, with constants nested within it as follows:

@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.

Tools

Automatically formatting edited files

A checkout should give you clang-format to automatically format Java code. It is suggested that Clang's formatting of code should be accepted in code reviews.

You can run git cl format to apply the automatic formatting.

IDE Setup

For automatically using the correct style, follow the guide to set up your favorite IDE:

Checkstyle

Checkstyle is automatically run by the build bots, and to ensure you do not have any surprises, you can also set up checkstyle locally using this guide.

Lint

Lint is run as part of the build. For more information, see here.

Style / Formatting

File Headers

TODOs

  • TODO should follow chromium convention. Examples:
    • TODO(username): Some sentence here.
    • TODO(crbug.com/123456): Even better to use a bug for context.

Code formatting

  • Fields should not be explicitly initialized to default values (see here).

Curly braces

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;

Import Order

  • Static imports go before other imports.
  • Each import group must be separated by an empty line.

This is the order of the import groups:

  1. android
  2. androidx
  3. com (except com.google.android.apps.chrome)
  4. dalvik
  5. junit
  6. org
  7. com.google.android.apps.chrome
  8. org.chromium
  9. java
  10. javax

Test-only Code

Functions used only for testing should be restricted to test-only usages with the testing suffixes supported PRESUMBIT.py. ForTesting is the conventional suffix although similar patterns, such as ForTest, are also accepted. These suffixes are checked at presubmit time to ensure the functions are called only by test files.

It's generally bad practice to directly call test-only methods from non-test-only code. However, occasionally it has to be done, and if so, you should guard the check with an if (BuildConfig.IS_FOR_TEST) so that our Java optimizer can still remove the call in non-test builds.

Location

"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.

Miscellany

  • Use UTF-8 file encodings and LF line endings.