diff --git a/BUILD.gn b/BUILD.gn index 23d79047d6a985..66f617bc675f11 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -304,13 +304,14 @@ group("gn_all") { "//testing/android/junit:junit_unit_tests", "//third_party/android_async_task:android_async_task_java", "//third_party/catapult/devil", - "//third_party/errorprone:chromium_errorprone", + "//third_party/errorprone:errorprone_java", "//third_party/smhasher:murmurhash3", "//tools/android:android_tools", "//tools/android:memconsumer", "//tools/android:push_apps_to_background", "//tools/android/audio_focus_grabber:audio_focus_grabber_apk", "//tools/android/customtabs_benchmark:customtabs_benchmark_apk", + "//tools/android/errorprone_plugin:errorprone_plugin_java", "//tools/android/kerberos/SpnegoAuthenticator:spnego_authenticator_apk", "//tools/cygprofile:cygprofile_unittests", "//ui/android:ui_junit_tests", diff --git a/DEPS b/DEPS index 4d80203506cb9a..5f7b5c5a89d337 100644 --- a/DEPS +++ b/DEPS @@ -250,6 +250,11 @@ deps = { 'condition': 'checkout_android', }, + 'src/third_party/auto/src': { + 'url': Var('chromium_git') + '/external/github.com/google/auto.git' + '@' + '71802f2ae74dae2744abd999f8434e13055c4ee3', + 'condition': 'checkout_android', + }, + 'src/third_party/bidichecker': Var('chromium_git') + '/external/bidichecker/lib.git' + '@' + '97f2aa645b74c28c57eca56992235c79850fa9e0', diff --git a/android_webview/glue/java/src/com/android/webview/chromium/ContentSettingsAdapter.java b/android_webview/glue/java/src/com/android/webview/chromium/ContentSettingsAdapter.java index 4aa7a485bb49c9..a64ff3dd86ec09 100644 --- a/android_webview/glue/java/src/com/android/webview/chromium/ContentSettingsAdapter.java +++ b/android_webview/glue/java/src/com/android/webview/chromium/ContentSettingsAdapter.java @@ -19,7 +19,7 @@ * Type adaptation layer between {@link android.webkit.WebSettings} and * {@link org.chromium.android_webview.AwSettings}. */ -@SuppressWarnings("deprecation") +@SuppressWarnings({"deprecation", "NoSynchronizedMethodCheck"}) @SuppressFBWarnings("CHROMIUM_SYNCHRONIZED_METHOD") @TargetApi(Build.VERSION_CODES.LOLLIPOP) public class ContentSettingsAdapter extends android.webkit.WebSettings { diff --git a/android_webview/glue/java/src/com/android/webview/chromium/CookieManagerAdapter.java b/android_webview/glue/java/src/com/android/webview/chromium/CookieManagerAdapter.java index 7ae88a95030b81..77dc95528fb341 100644 --- a/android_webview/glue/java/src/com/android/webview/chromium/CookieManagerAdapter.java +++ b/android_webview/glue/java/src/com/android/webview/chromium/CookieManagerAdapter.java @@ -18,7 +18,7 @@ * Chromium implementation of CookieManager -- forwards calls to the * chromium internal implementation. */ -@SuppressWarnings("deprecation") +@SuppressWarnings({"deprecation", "NoSynchronizedMethodCheck"}) @SuppressFBWarnings("CHROMIUM_SYNCHRONIZED_METHOD") public class CookieManagerAdapter extends CookieManager { private static final String TAG = "CookieManager"; diff --git a/android_webview/glue/java/src/com/android/webview/chromium/ServiceWorkerSettingsAdapter.java b/android_webview/glue/java/src/com/android/webview/chromium/ServiceWorkerSettingsAdapter.java index dc22ab2f460822..8698a31ad48da9 100644 --- a/android_webview/glue/java/src/com/android/webview/chromium/ServiceWorkerSettingsAdapter.java +++ b/android_webview/glue/java/src/com/android/webview/chromium/ServiceWorkerSettingsAdapter.java @@ -11,6 +11,7 @@ * Type adaptation layer between {@link android.webkit.ServiceWorkerWebSettings} * and {@link org.chromium.android_webview.AwServiceWorkerSettings}. */ +@SuppressWarnings("NoSynchronizedMethodCheck") @SuppressFBWarnings("CHROMIUM_SYNCHRONIZED_METHOD") public class ServiceWorkerSettingsAdapter extends android.webkit.ServiceWorkerWebSettings { private AwServiceWorkerSettings mAwServiceWorkerSettings; diff --git a/android_webview/glue/java/src/com/android/webview/chromium/WebBackForwardListChromium.java b/android_webview/glue/java/src/com/android/webview/chromium/WebBackForwardListChromium.java index 7b10cabee8cba7..2512a2cde014a5 100644 --- a/android_webview/glue/java/src/com/android/webview/chromium/WebBackForwardListChromium.java +++ b/android_webview/glue/java/src/com/android/webview/chromium/WebBackForwardListChromium.java @@ -17,9 +17,8 @@ * WebView Chromium implementation of WebBackForwardList. Simple immutable * wrapper around NavigationHistory. */ -@SuppressFBWarnings({ - "CHROMIUM_SYNCHRONIZED_METHOD", - "SE_BAD_FIELD"}) +@SuppressWarnings("NoSynchronizedMethodCheck") +@SuppressFBWarnings({"CHROMIUM_SYNCHRONIZED_METHOD", "SE_BAD_FIELD"}) public class WebBackForwardListChromium extends WebBackForwardList { private final List mHistroryItemList; private final int mCurrentIndex; diff --git a/android_webview/glue/java/src/com/android/webview/chromium/WebHistoryItemChromium.java b/android_webview/glue/java/src/com/android/webview/chromium/WebHistoryItemChromium.java index d492bdff3a64a5..3303464320c291 100644 --- a/android_webview/glue/java/src/com/android/webview/chromium/WebHistoryItemChromium.java +++ b/android_webview/glue/java/src/com/android/webview/chromium/WebHistoryItemChromium.java @@ -80,12 +80,12 @@ private WebHistoryItemChromium(String url, String originalUrl, String title, Bit /** * See {@link android.webkit.WebHistoryItem#clone}. */ - @SuppressFBWarnings({ - "CHROMIUM_SYNCHRONIZED_METHOD", - "CN_IDIOM_NO_SUPER_CALL", + @SuppressWarnings("NoSynchronizedMethodCheck") + @SuppressFBWarnings({"CHROMIUM_SYNCHRONIZED_METHOD", "CN_IDIOM_NO_SUPER_CALL", "CN_IMPLEMENTS_CLONE_BUT_NOT_CLONEABLE"}) @Override - public synchronized WebHistoryItemChromium clone() { + public synchronized WebHistoryItemChromium + clone() { return new WebHistoryItemChromium(mUrl, mOriginalUrl, mTitle, mFavicon); } } diff --git a/base/android/java/src/org/chromium/base/NonThreadSafe.java b/base/android/java/src/org/chromium/base/NonThreadSafe.java index 26f6d72fd8a790..2ab654c442274a 100644 --- a/base/android/java/src/org/chromium/base/NonThreadSafe.java +++ b/base/android/java/src/org/chromium/base/NonThreadSafe.java @@ -33,6 +33,7 @@ public synchronized void detachFromThread() { * Assigns the current thread if no thread was assigned. */ @SuppressFBWarnings("CHROMIUM_SYNCHRONIZED_METHOD") + @SuppressWarnings("NoSynchronizedMethodCheck") public synchronized boolean calledOnValidThread() { ensureThreadIdAssigned(); return mThreadId.equals(Thread.currentThread().getId()); diff --git a/build/android/gyp/jar.py b/build/android/gyp/jar.py index 2b19c587b78f03..2f54c1ae8d2b71 100755 --- a/build/android/gyp/jar.py +++ b/build/android/gyp/jar.py @@ -66,11 +66,11 @@ def Jar(class_files, classes_dir, jar_path, manifest_file=None, def JarDirectory(classes_dir, jar_path, manifest_file=None, predicate=None, provider_configurations=None, additional_files=None): - class_files = build_utils.FindInDirectory(classes_dir, '*.class') + all_files = build_utils.FindInDirectory(classes_dir, '*') if predicate: - class_files = [f for f in class_files if predicate(f)] + all_files = [f for f in all_files if predicate(f)] - Jar(class_files, classes_dir, jar_path, manifest_file=manifest_file, + Jar(all_files, classes_dir, jar_path, manifest_file=manifest_file, provider_configurations=provider_configurations, additional_files=additional_files) diff --git a/build/android/gyp/javac.py b/build/android/gyp/javac.py index d9921253fdd173..c6134261ab73f4 100755 --- a/build/android/gyp/javac.py +++ b/build/android/gyp/javac.py @@ -332,6 +332,9 @@ def _ParseOptions(argv): dest='processors', action='append', help='Annotation processor to use.') + parser.add_option( + '--processorpath', + help='Where javac should look for annotation processors.') parser.add_option( '--processor-arg', dest='processor_args', @@ -469,6 +472,8 @@ def main(argv): if options.processors: javac_cmd.extend(['-processor', ','.join(options.processors)]) + if options.processorpath: + javac_cmd.extend(['-processorpath', options.processorpath]) if options.processor_args: for arg in options.processor_args: javac_cmd.extend(['-A%s' % arg]) diff --git a/build/config/android/internal_rules.gni b/build/config/android/internal_rules.gni index 2172d48e48be5b..55a2920c62ec13 100644 --- a/build/config/android/internal_rules.gni +++ b/build/config/android/internal_rules.gni @@ -2541,10 +2541,14 @@ if (enable_java_templates) { args += [ "--chromium-code=1" ] } if (_enable_errorprone) { - deps += [ "//third_party/errorprone:chromium_errorprone($default_toolchain)" ] + deps += + [ "//third_party/errorprone:errorprone_java($default_toolchain)" ] + deps += [ "//tools/android/errorprone_plugin:errorprone_plugin_java($default_toolchain)" ] args += [ "--use-errorprone-path", - "bin/chromium_errorprone", + "bin/errorprone_java", + "--processorpath", + "lib.java/tools/android/errorprone_plugin/errorprone_plugin_java.jar", ] } foreach(e, _provider_configurations) { diff --git a/chrome/android/java/src/org/chromium/chrome/browser/preferences/ChromePreferenceManager.java b/chrome/android/java/src/org/chromium/chrome/browser/preferences/ChromePreferenceManager.java index c53ba8972de9e4..e189c2356a5b64 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/preferences/ChromePreferenceManager.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/preferences/ChromePreferenceManager.java @@ -8,7 +8,6 @@ import android.os.StrictMode; import org.chromium.base.ContextUtils; -import org.chromium.base.annotations.SuppressFBWarnings; import org.chromium.chrome.browser.ChromeSwitches; import org.chromium.chrome.browser.ChromeVersionInfo; import org.chromium.chrome.browser.crash.MinidumpUploadService.ProcessType; @@ -70,7 +69,7 @@ public class ChromePreferenceManager { public static final String CHROME_HOME_SHARED_PREFERENCES_KEY = "chrome_home_enabled_date"; - private static ChromePreferenceManager sPrefs; + private static ChromePreferenceManager sPrefs = new ChromePreferenceManager(); private final SharedPreferences mSharedPreferences; @@ -82,11 +81,7 @@ private ChromePreferenceManager() { * Get the static instance of ChromePreferenceManager if exists else create it. * @return the ChromePreferenceManager singleton */ - @SuppressFBWarnings("CHROMIUM_SYNCHRONIZED_METHOD") - public static synchronized ChromePreferenceManager getInstance() { - if (sPrefs == null) { - sPrefs = new ChromePreferenceManager(); - } + public static ChromePreferenceManager getInstance() { return sPrefs; } diff --git a/components/gcm_driver/instance_id/android/java/src/org/chromium/components/gcm_driver/instance_id/InstanceIDBridge.java b/components/gcm_driver/instance_id/android/java/src/org/chromium/components/gcm_driver/instance_id/InstanceIDBridge.java index 08a7036845f8b4..c3e725f767aa3b 100644 --- a/components/gcm_driver/instance_id/android/java/src/org/chromium/components/gcm_driver/instance_id/InstanceIDBridge.java +++ b/components/gcm_driver/instance_id/android/java/src/org/chromium/components/gcm_driver/instance_id/InstanceIDBridge.java @@ -189,6 +189,7 @@ private abstract class BridgeAsyncTask { public void execute() { AsyncTask task = new AsyncTask() { @Override + @SuppressWarnings("NoSynchronizedThisCheck") // Only used/accessible by native. protected Result doInBackground(Void... params) { synchronized (InstanceIDBridge.this) { if (mInstanceID == null) { diff --git a/third_party/.gitignore b/third_party/.gitignore index 777630ba97b4df..24cf2103537024 100644 --- a/third_party/.gitignore +++ b/third_party/.gitignore @@ -24,6 +24,7 @@ /apache-win32/modules/*.dll /apk-patch-size-estimator/lib/*.jar /asan +/auto/src /bazel/desugar/Desugar.jar /bidichecker /bison diff --git a/third_party/auto/BUILD.gn b/third_party/auto/BUILD.gn new file mode 100644 index 00000000000000..56b283e2fd0e1f --- /dev/null +++ b/third_party/auto/BUILD.gn @@ -0,0 +1,36 @@ +# Copyright 2017 The Chromium Authors. All rights reserved. +# Use of this source code is governed by a BSD-style license that can be +# found in the LICENSE file. + +import("//build/config/android/rules.gni") + +java_library("auto_common_java") { + java_files = [ + "src/common/src/main/java/com/google/auto/common/AnnotationMirrors.java", + "src/common/src/main/java/com/google/auto/common/BasicAnnotationProcessor.java", + "src/common/src/main/java/com/google/auto/common/MoreTypes.java", + "src/common/src/main/java/com/google/auto/common/SuperficialValidation.java", + "src/common/src/main/java/com/google/auto/common/AnnotationValues.java", + "src/common/src/main/java/com/google/auto/common/MoreElements.java", + "src/common/src/main/java/com/google/auto/common/Overrides.java", + "src/common/src/main/java/com/google/auto/common/Visibility.java", + ] + + deps = [ + "//third_party/guava:guava_java", + ] +} + +java_library("auto_service_java") { + java_files = [ + "src/service/src/main/java/com/google/auto/service/AutoService.java", + "src/service/src/main/java/com/google/auto/service/processor/AutoServiceProcessor.java", + "src/service/src/main/java/com/google/auto/service/processor/package-info.java", + "src/service/src/main/java/com/google/auto/service/processor/ServicesFiles.java", + ] + + deps = [ + ":auto_common_java", + "//third_party/guava:guava_java", + ] +} diff --git a/third_party/auto/OWNERS b/third_party/auto/OWNERS new file mode 100644 index 00000000000000..80e6172b625a52 --- /dev/null +++ b/third_party/auto/OWNERS @@ -0,0 +1,3 @@ +agrieve@chromium.org +nyquist@chromium.org +wnwen@chromium.org diff --git a/third_party/auto/README.chromium b/third_party/auto/README.chromium new file mode 100644 index 00000000000000..008a3eab20e2c0 --- /dev/null +++ b/third_party/auto/README.chromium @@ -0,0 +1,15 @@ +Name: Auto +Short Name: auto +URL: https://github.com/google/auto +Version: 71802f2ae74dae2744abd999f8434e13055c4ee3 +Date: November 1, 2017 +License: Apache 2.0 +License File: NOT_SHIPPED +Security Critical: no + +Description: +* A collection of source code generators for Java. +* AutoService used for custom plugins in //tools/android/errorprone_plugin. + +Local Modifications: +* Add BUILD.gn. diff --git a/third_party/errorprone/BUILD.gn b/third_party/errorprone/BUILD.gn index bac14b93ee8d9c..13b2e418cb9b89 100644 --- a/third_party/errorprone/BUILD.gn +++ b/third_party/errorprone/BUILD.gn @@ -5,7 +5,7 @@ import("//build/config/android/rules.gni") if (current_toolchain == default_toolchain) { - java_prebuilt("chromium_errorprone") { + java_prebuilt("errorprone_java") { jar_path = "lib/error_prone_ant-2.0.15.jar" main_class = "com.google.errorprone.ErrorProneCompiler" } diff --git a/third_party/errorprone/OWNERS b/third_party/errorprone/OWNERS index 56e1e2841991b2..e4f73890b27e53 100644 --- a/third_party/errorprone/OWNERS +++ b/third_party/errorprone/OWNERS @@ -1,3 +1,4 @@ agrieve@chromium.org jbudorick@chromium.org +nyquist@chromium.org wnwen@chromium.org diff --git a/third_party/errorprone/README.chromium b/third_party/errorprone/README.chromium index 809b3404ff2b36..18210b2deda636 100644 --- a/third_party/errorprone/README.chromium +++ b/third_party/errorprone/README.chromium @@ -4,7 +4,7 @@ URL: http://errorprone.info/ Version: 2.0.15 Date: October 18, 2017 License: Apache 2.0 -License File: LICENSE +License File: NOT_SHIPPED Security Critical: no Description: diff --git a/third_party/guava/BUILD.gn b/third_party/guava/BUILD.gn index 8b798b4b83b10a..fdadb226a1466c 100644 --- a/third_party/guava/BUILD.gn +++ b/third_party/guava/BUILD.gn @@ -5,7 +5,13 @@ import("//build/config/android/rules.gni") java_prebuilt("guava_java") { - testonly = true + # Since our build tooling depends on this target, we need it to not be + # "testonly = true", but production code must not depend on this as it + # results in size regressions. Thus, do not use this outside of tests and + # build tooling. supports_android = true jar_path = "lib/guava-20.0.jar" + + # Avoids dependency cycle. + no_build_hooks = true } diff --git a/tools/android/errorprone_plugin/BUILD.gn b/tools/android/errorprone_plugin/BUILD.gn new file mode 100644 index 00000000000000..b15ff20d0d8fa7 --- /dev/null +++ b/tools/android/errorprone_plugin/BUILD.gn @@ -0,0 +1,23 @@ +# Copyright 2017 The Chromium Authors. All rights reserved. +# Use of this source code is governed by a BSD-style license that can be +# found in the LICENSE file. + +import("//build/config/android/rules.gni") + +java_library("errorprone_plugin_java") { + java_files = [ + "src/org/chromium/tools/errorprone/plugin/NoSynchronizedMethodCheck.java", + "src/org/chromium/tools/errorprone/plugin/NoSynchronizedThisCheck.java", + ] + + processors_javac = + [ "com.google.auto.service.processor.AutoServiceProcessor" ] + + # Necessary to avoid dependency cycle + enable_errorprone = false + + deps = [ + "//third_party/auto:auto_service_java", + "//third_party/errorprone:errorprone_java", + ] +} diff --git a/tools/android/errorprone_plugin/OWNERS b/tools/android/errorprone_plugin/OWNERS new file mode 100644 index 00000000000000..80e6172b625a52 --- /dev/null +++ b/tools/android/errorprone_plugin/OWNERS @@ -0,0 +1,3 @@ +agrieve@chromium.org +nyquist@chromium.org +wnwen@chromium.org diff --git a/tools/android/errorprone_plugin/src/org/chromium/tools/errorprone/plugin/NoSynchronizedMethodCheck.java b/tools/android/errorprone_plugin/src/org/chromium/tools/errorprone/plugin/NoSynchronizedMethodCheck.java new file mode 100644 index 00000000000000..b99f1871e52053 --- /dev/null +++ b/tools/android/errorprone_plugin/src/org/chromium/tools/errorprone/plugin/NoSynchronizedMethodCheck.java @@ -0,0 +1,52 @@ +// Copyright 2017 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +package org.chromium.tools.errorprone.plugin; + +import com.google.auto.service.AutoService; +import com.google.errorprone.BugPattern; +import com.google.errorprone.VisitorState; +import com.google.errorprone.bugpatterns.BugChecker; +import com.google.errorprone.fixes.SuggestedFixes; +import com.google.errorprone.matchers.Description; +import com.google.errorprone.util.ASTHelpers; +import com.sun.source.tree.MethodTree; +import com.sun.tools.javac.code.Symbol; + +import javax.lang.model.element.Modifier; + +/** + * Triggers an error for public methods that use "synchronized" in their signature. + */ +@AutoService(BugChecker.class) +@BugPattern(name = "NoSynchronizedMethodCheck", category = BugPattern.Category.JDK, + summary = "Use of synchronized in public method signature disallowed.", + severity = BugPattern.SeverityLevel.ERROR, linkType = BugPattern.LinkType.CUSTOM, + link = "https://stackoverflow.com/questions/20906548/why-is-synchronized-block-better-than-synchronized-method") +public class NoSynchronizedMethodCheck extends BugChecker implements BugChecker.MethodTreeMatcher { + @Override + public Description matchMethod(MethodTree methodTree, VisitorState visitorState) { + Symbol.MethodSymbol method = ASTHelpers.getSymbol(methodTree); + // Skip methods that aren't synchronized and non-public methods + if (!method.getModifiers().contains(Modifier.SYNCHRONIZED) + || !method.getModifiers().contains(Modifier.PUBLIC)) { + return Description.NO_MATCH; + } + // Skip methods that are only public due to VisibleForTesting + if (ASTHelpers.hasDirectAnnotationWithSimpleName(method, "VisibleForTesting")) { + return Description.NO_MATCH; + } + // Skip non-public classes + Symbol.ClassSymbol enclosingClass = ASTHelpers.enclosingClass(method); + if (!enclosingClass.getModifiers().contains(Modifier.PUBLIC)) { + return Description.NO_MATCH; + } + return buildDescription(methodTree) + .addFix(SuggestedFixes.removeModifiers( + methodTree, visitorState, Modifier.SYNCHRONIZED)) + .setMessage(String.format( + "Used synchronized modifier in public method %s", method.getSimpleName())) + .build(); + } +} diff --git a/tools/android/errorprone_plugin/src/org/chromium/tools/errorprone/plugin/NoSynchronizedThisCheck.java b/tools/android/errorprone_plugin/src/org/chromium/tools/errorprone/plugin/NoSynchronizedThisCheck.java new file mode 100644 index 00000000000000..f82664e567c61d --- /dev/null +++ b/tools/android/errorprone_plugin/src/org/chromium/tools/errorprone/plugin/NoSynchronizedThisCheck.java @@ -0,0 +1,46 @@ +// Copyright 2017 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +package org.chromium.tools.errorprone.plugin; + +import com.google.auto.service.AutoService; +import com.google.errorprone.BugPattern; +import com.google.errorprone.VisitorState; +import com.google.errorprone.bugpatterns.BugChecker; +import com.google.errorprone.matchers.Description; +import com.google.errorprone.util.ASTHelpers; +import com.sun.source.tree.SynchronizedTree; +import com.sun.tools.javac.code.Symbol; +import com.sun.tools.javac.tree.JCTree; +import com.sun.tools.javac.tree.TreeInfo; + +import javax.lang.model.element.Modifier; + +/** + * This class detects the synchronized method. + */ +@AutoService(BugChecker.class) +@BugPattern(name = "NoSynchronizedThisCheck", category = BugPattern.Category.JDK, + summary = "Do not synchronized on 'this' in public classes", + severity = BugPattern.SeverityLevel.ERROR, linkType = BugPattern.LinkType.CUSTOM, + link = "https://stackoverflow.com/questions/442564/avoid-synchronizedthis-in-java") +public class NoSynchronizedThisCheck + extends BugChecker implements BugChecker.SynchronizedTreeMatcher { + @Override + public Description matchSynchronized(SynchronizedTree tree, VisitorState visitorState) { + Symbol lock = ASTHelpers.getSymbol(TreeInfo.skipParens((JCTree) tree.getExpression())); + // Skip locks that are not 'this' + if (!lock.getSimpleName().contentEquals("this")) { + return Description.NO_MATCH; + } + // Skip non-public classes + Symbol.ClassSymbol enclosingClass = ASTHelpers.enclosingClass(lock); + if (!enclosingClass.getModifiers().contains(Modifier.PUBLIC)) { + return Description.NO_MATCH; + } + return buildDescription(tree) + .setMessage("Used instance variable as synchronization lock") + .build(); + } +}