Skip to content

Commit

Permalink
Reland "Android: Add third_party/auto for errorprone""
Browse files Browse the repository at this point in the history
Original CL: https://crrev.com/c/739725

Fix:
- errorprone_plugin_java depends on errorprone_java, which is only
  available on $default_toolchain.

BUG=777572
TBR=fgorski@chromium.org,brettw@chromium.org,yfriedman@chromium.org,jbudorick@chromium.org,peter@chromium.org,torne@chromium.org,agrieve@chromium.org

Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I2dceece332036a98014943cff4ec82b152075622
Reviewed-on: https://chromium-review.googlesource.com/750645
Commit-Queue: Peter Wen <wnwen@chromium.org>
Reviewed-by: Peter Wen <wnwen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513513}
  • Loading branch information
Peter Wen authored and Commit Bot committed Nov 2, 2017
1 parent 0c61a1e commit d0cc6d6
Show file tree
Hide file tree
Showing 25 changed files with 223 additions and 25 deletions.
3 changes: 2 additions & 1 deletion BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
5 changes: 5 additions & 0 deletions DEPS
Original file line number Diff line number Diff line change
Expand Up @@ -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',

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<WebHistoryItemChromium> mHistroryItemList;
private final int mCurrentIndex;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
1 change: 1 addition & 0 deletions base/android/java/src/org/chromium/base/NonThreadSafe.java
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
6 changes: 3 additions & 3 deletions build/android/gyp/jar.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
5 changes: 5 additions & 0 deletions build/android/gyp/javac.py
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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])
Expand Down
8 changes: 6 additions & 2 deletions build/config/android/internal_rules.gni
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;

Expand All @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@ private abstract class BridgeAsyncTask<Result> {
public void execute() {
AsyncTask<Void, Void, Result> task = new AsyncTask<Void, Void, Result>() {
@Override
@SuppressWarnings("NoSynchronizedThisCheck") // Only used/accessible by native.
protected Result doInBackground(Void... params) {
synchronized (InstanceIDBridge.this) {
if (mInstanceID == null) {
Expand Down
1 change: 1 addition & 0 deletions third_party/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
/apache-win32/modules/*.dll
/apk-patch-size-estimator/lib/*.jar
/asan
/auto/src
/bazel/desugar/Desugar.jar
/bidichecker
/bison
Expand Down
36 changes: 36 additions & 0 deletions third_party/auto/BUILD.gn
Original file line number Diff line number Diff line change
@@ -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",
]
}
3 changes: 3 additions & 0 deletions third_party/auto/OWNERS
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
agrieve@chromium.org
nyquist@chromium.org
wnwen@chromium.org
15 changes: 15 additions & 0 deletions third_party/auto/README.chromium
Original file line number Diff line number Diff line change
@@ -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.
2 changes: 1 addition & 1 deletion third_party/errorprone/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
Expand Down
1 change: 1 addition & 0 deletions third_party/errorprone/OWNERS
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
agrieve@chromium.org
jbudorick@chromium.org
nyquist@chromium.org
wnwen@chromium.org
2 changes: 1 addition & 1 deletion third_party/errorprone/README.chromium
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
8 changes: 7 additions & 1 deletion third_party/guava/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
23 changes: 23 additions & 0 deletions tools/android/errorprone_plugin/BUILD.gn
Original file line number Diff line number Diff line change
@@ -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",
]
}
3 changes: 3 additions & 0 deletions tools/android/errorprone_plugin/OWNERS
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
agrieve@chromium.org
nyquist@chromium.org
wnwen@chromium.org
Original file line number Diff line number Diff line change
@@ -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();
}
}
Loading

0 comments on commit d0cc6d6

Please sign in to comment.