Revert of Reverting due to Android builders breakage. Subsequent fixe…

…s did not fix. (patchset chromium#1 id:1 of https://codereview.chromium.org/843913006/)

Reason for revert:
Didn't fix the Android builders.

Original issue's description:
> Reverting due to Android builders breakage. Subsequent fixes did not fix.
>
> Revert "Use SuppressFBWarnings to suppress findbugs warnings"
>
> This reverts commit 7ff0672.
>
> Revert "Temporarily add a workaround for findbugs."
>
> This reverts commit 790b64b.
>
> Revert "Temporarily remove some findbugs "known bugs""
>
> This reverts commit 5a1c592.
>
> NOTRY=true
> TBR=cjhopman@chromium.org
>
> Committed: https://crrev.com/710d2f69b76e7b997307a7096e31029857e8c7fb
> Cr-Commit-Position: refs/heads/master@{#311630}

TBR=cjhopman@chromium.org
NOTREECHECKS=true
NOTRY=true

Review URL: https://codereview.chromium.org/827123006

Cr-Commit-Position: refs/heads/master@{#311640}

android_webview/java/src/org/chromium/android_webview/AwContentsClient.java

@@ -18,6 +18,7 @@
 import android.webkit.WebChromeClient;
 
 import org.chromium.android_webview.permission.AwPermissionRequest;
+import org.chromium.base.annotations.SuppressFBWarnings;
 
 import java.security.Principal;
 import java.util.HashMap;
@@ -76,6 +77,7 @@ final void onBackgroundColorChanged(int color) {
     /**
      * Parameters for the {@link AwContentsClient#showFileChooser} method.
      */
+    @SuppressFBWarnings("URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD")
     public static class FileChooserParams {
         public int mode;
         public String acceptTypes;

android_webview/javatests/src/org/chromium/android_webview/test/ArchiveTest.java

@@ -11,6 +11,7 @@
 
 import org.chromium.android_webview.AwContents;
 import org.chromium.base.ThreadUtils;
+import org.chromium.base.annotations.SuppressFBWarnings;
 import org.chromium.base.test.util.Feature;
 import org.chromium.base.test.util.UrlUtils;
 
@@ -120,6 +121,7 @@ public void testAutoGoodPath() throws Throwable {
 
     @SmallTest
     @Feature({"AndroidWebView"})
+    @SuppressFBWarnings("DMI_HARDCODED_ABSOLUTE_FILENAME")
     public void testExplicitBadPath() throws Throwable {
         final String path = new File("/foo/bar/baz.mht").getAbsolutePath();
         deleteFile(path);
@@ -132,6 +134,7 @@ public void testExplicitBadPath() throws Throwable {
 
     @SmallTest
     @Feature({"AndroidWebView"})
+    @SuppressFBWarnings("DMI_HARDCODED_ABSOLUTE_FILENAME")
     public void testAutoBadPath() throws Throwable {
         final String path = new File("/foo/bar/").getAbsolutePath();
         deleteFile(path);

base/android/java/src/org/chromium/base/PerfTraceEvent.java

@@ -8,6 +8,8 @@
 import android.os.Debug.MemoryInfo;
 import android.util.Log;
 
+import org.chromium.base.annotations.SuppressFBWarnings;
+
 import org.json.JSONArray;
 import org.json.JSONException;
 import org.json.JSONObject;
@@ -36,6 +38,7 @@
  * the @TracePerf annotation.  Thus, unlike TraceEvent, we do not
  * support an implicit trace name based on the callstack.
  */
+@SuppressFBWarnings("CHROMIUM_SYNCHRONIZED_METHOD")
 public class PerfTraceEvent {
     private static final int MAX_NAME_LENGTH = 40;
     private static final String MEMORY_TRACE_NAME_SUFFIX = "_BZR_PSS";

base/android/java/src/org/chromium/base/annotations/SuppressFBWarnings.java

@@ -0,0 +1,20 @@
+// Copyright 2014 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.base.annotations;
+
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+
+/**
+ *  @SuppressFBWarnings is used to suppress FindBugs warnings.
+ *
+ *  The long name of FindBugs warnings can be found at
+ *  http://findbugs.sourceforge.net/bugDescriptions.html
+ */
+@Retention(RetentionPolicy.CLASS)
+public @interface SuppressFBWarnings {
+    String[] value() default {};
+    String justification() default "";
+}

base/android/java/templates/NativeLibraries.template

@@ -4,6 +4,9 @@
 
 package org.chromium.base.library_loader;
 
+import org.chromium.base.annotations.SuppressFBWarnings;
+
+@SuppressFBWarnings
 public class NativeLibraries {
     /**
      * IMPORTANT NOTE: The variables defined here must _not_ be 'final'.

build/android/findbugs_filter/findbugs_exclude.xml

@@ -17,29 +17,8 @@ In particular, ~ at the start of a string means it's a regex.
   <Match>
     <Class name="~org\.chromium\..*\.Manifest(\$\w+)?" />
   </Match>
-  <!-- Ignore bugs in NativeLibraries.java (the auto-generation confuses findbugs). -->
-  <Match>
-    <Class name="~org\.chromium\.base\..*\.NativeLibraries.*?" />
-  </Match>
-  <!--
-  Ignore bugs in CleanupReferenceTest.java (redundant null check)
-  TODO(joth): Group all GC related tests and filter them out, since the null
-  check is necessary to make sure the nullification is flushed to memory.
-  -->
-  <Match>
-    <Class name="~org\.chromium\.content\..*\.CleanupReferenceTest.*?" />
-  </Match>
-  <!-- Ignore errors in JavaBridge due to reflection. -->
-  <Match>
-    <Class name="~.*\.JavaBridge.*"/>
-    <Bug code="UuF,UrF,UMAC" />
-  </Match>
-  <!-- "Struct" like classes expect to have unused public data members -->
-  <Match>
-    <Class name="~.*android_webview.*FileChooserParams"/>
-    <Bug code="UrF" />
-  </Match>
   <!-- Ignore "reliance on default String encoding" warnings, as we're not multi-platform -->
+
   <Bug pattern="DM_DEFAULT_ENCODING" />
   <!-- Ignore bugs that are often false-positives in test code -->
   <Match>
@@ -49,68 +28,4 @@ In particular, ~ at the start of a string means it's a regex.
       <Bug pattern="DM_GC" />
     </Or>
   </Match>
-  <!--
-  Ignore calls to System.exit() following errors during loading the native library.
-  There is no way to recover from such errors without restarting the application,
-  so System.exit() is the best solution.
-  -->
-  <Match>
-    <Class name="~org\.chromium\.chrome\..*\.ChromiumSyncAdapter.*" />
-    <Method name="run" />
-    <Bug code="Dm" />
-  </Match>
-  <Match>
-    <Class name="~org\.chromium\.chrome\..*\.ChromiumSyncAdapter" />
-    <Method name="startBrowserProcessesSync" />
-    <Bug code="Dm" />
-  </Match>
-  <Match>
-    <Class name="~org\.chromium\.chrome\..*\.ChromeShellActivity" />
-    <Method name="onCreate" />
-    <Bug code="Dm" />
-  </Match>
-  <Match>
-    <Class name="~org\.chromium\.chrome\..*\.AccountsChangedReceiver.*" />
-    <Method name="run" />
-    <Bug code="Dm" />
-  </Match>
-  <Match>
-    <Class name="~org\.chromium\.chrome\..*\.NotificationService" />
-    <Method name="dispatchIntentOnUIThread" />
-    <Bug code="Dm" />
-  </Match>
-  <Match>
-    <Class name="org.chromium.chrome.browser.preferences.Preferences" />
-    <Method name="onCreate" />
-    <Bug code="Dm" />
-  </Match>
-  <Match>
-    <Class name="~org\.chromium\.content\..*\.ChildProcessService.*" />
-    <Method name="run" />
-    <Bug code="Dm" />
-  </Match>
-  <Match>
-    <Class name="~org\.chromium\..*ContentBrowserTestsActivity" />
-    <Method name="onCreate" />
-    <Bug code="Dm" />
-  </Match>
-  <Match>
-    <Class name="~org\.chromium\..*ContentShellActivity" />
-    <Method name="onCreate" />
-    <Bug code="Dm" />
-  </Match>
-  <Match>
-    <Class name="~org\.chromium\.components\.gcm_driver\..*\.GCMDriver" />
-    <Method name="launchNativeThen" />
-    <Bug code="Dm" />
-  </Match>
-  <!--
-  Ignore write to static field in GCMDriver, as it's the cleanest way to mark
-  the singleton as null when the native counterpart is destroyed.
-  -->
-  <Match>
-    <Class name="~org\.chromium\.components\.gcm_driver\..*\.GCMDriver" />
-    <Method name="destroy" />
-    <Bug code="ST" />
-  </Match>
 </FindBugsFilter>

build/android/findbugs_filter/findbugs_known_bugs.txt

@@ -1,29 +1,4 @@
-M C CSM: Shouldn't use synchronized method, please narrow down the synchronization scope.  At JavaBridgeArrayCoercionTest.java
-M C CSM: Shouldn't use synchronized method, please narrow down the synchronization scope.  At JavaBridgeArrayTest.java
-M C CSM: Shouldn't use synchronized method, please narrow down the synchronization scope.  At JavaBridgeBasicsTest.java
-M C CSM: Shouldn't use synchronized method, please narrow down the synchronization scope.  At JavaBridgeChildFrameTest.java
-M C CSM: Shouldn't use synchronized method, please narrow down the synchronization scope.  At JavaBridgeCoercionTest.java
-M C CSM: Shouldn't use synchronized method, please narrow down the synchronization scope.  At JavaBridgeFieldsTest.java
-M C CSM: Shouldn't use synchronized method, please narrow down the synchronization scope.  At JavaBridgeReturnValuesTest.java
-M C CSM: Shouldn't use synchronized method, please narrow down the synchronization scope.  At JavaBridgeTestBase.java
-M C CSM: Shouldn't use synchronized method, please narrow down the synchronization scope.  At PerfTraceEvent.java
-M C CSM: Shouldn't use synchronized method, please narrow down the synchronization scope.  At SimpleSynchronizedMethod.java
-M C CSM: Shouldn't use synchronized method, please narrow down the synchronization scope.  At SimpleSynchronizedStaticMethod.java
-M C CST: Shouldn't use synchronized(this), please narrow down the synchronization scope.  At SimpleSynchronizedThis.java
-M D DMI: Hard coded reference to an absolute pathname in org.chromium.android_webview.test.ArchiveTest.testAutoBadPath()  At ArchiveTest.java
-M D DMI: Hard coded reference to an absolute pathname in org.chromium.android_webview.test.ArchiveTest.testExplicitBadPath()  At ArchiveTest.java
-M D SF: Switch statement found in org.chromium.chrome.browser.ChromeBrowserProvider.insert(Uri, ContentValues) where one case falls through to the next case  At ChromeBrowserProvider.java
-M M UG: org.chromium.content.browser.JavaBridgeReturnValuesTest$TestObject.getBooleanValue() is unsynchronized, org.chromium.content.browser.JavaBridgeReturnValuesTest$TestObject.setBooleanValue(boolean) is synchronized  At JavaBridgeReturnValuesTest.java
-M M UG: org.chromium.content.browser.JavaBridgeReturnValuesTest$TestObject.getStringValue() is unsynchronized, org.chromium.content.browser.JavaBridgeReturnValuesTest$TestObject.setStringValue(String) is synchronized  At JavaBridgeReturnValuesTest.java
-M V EI2: org.chromium.chrome.browser.ChromeBrowserProvider$BookmarkNode.setFavicon(byte[]) may expose internal representation by storing an externally mutable object into ChromeBrowserProvider$BookmarkNode.mFavicon  At ChromeBrowserProvider.java
-M V EI2: org.chromium.chrome.browser.ChromeBrowserProvider$BookmarkNode.setThumbnail(byte[]) may expose internal representation by storing an externally mutable object into ChromeBrowserProvider$BookmarkNode.mThumbnail  At ChromeBrowserProvider.java
-M V EI: org.chromium.chrome.browser.ChromeBrowserProvider$BookmarkNode.favicon() may expose internal representation by returning ChromeBrowserProvider$BookmarkNode.mFavicon  At ChromeBrowserProvider.java
-M V EI: org.chromium.chrome.browser.ChromeBrowserProvider$BookmarkNode.thumbnail() may expose internal representation by returning ChromeBrowserProvider$BookmarkNode.mThumbnail  At ChromeBrowserProvider.java
-M M LI: Incorrect lazy initialization of static field org.chromium.chrome.browser.sync.ProfileSyncService.sSyncSetupManager in org.chromium.chrome.browser.sync.ProfileSyncService.get(Context)  At ProfileSyncService.java
 M M LI: Incorrect lazy initialization of static field org.chromium.chrome.browser.download.DownloadManagerService.sDownloadManagerService in org.chromium.chrome.browser.download.DownloadManagerService.getDownloadManagerService(Context)  At DownloadManagerService.java
-M V EI2: org.chromium.content_public.browser.LoadUrlParams.setPostData(byte[]) may expose internal representation by storing an externally mutable object into LoadUrlParams.mPostData  At LoadUrlParams.java
-M V EI: org.chromium.content_public.browser.LoadUrlParams.getPostData() may expose internal representation by returning LoadUrlParams.mPostData  At LoadUrlParams.java
-M V EI2: org.chromium.net.ChromiumUrlRequest.setUploadData(String, byte[]) may expose internal representation by storing an externally mutable object into ChromiumUrlRequest.mUploadData  At ChromiumUrlRequest.java
 M D UrF: Unread public/protected field: org.chromium.chrome.browser.document.PendingDocumentData.extraHeaders  At DocumentTabModelSelector.java
 M D UrF: Unread public/protected field: org.chromium.chrome.browser.document.PendingDocumentData.postData  At DocumentTabModelSelector.java
 M D UrF: Unread public/protected field: org.chromium.chrome.browser.document.PendingDocumentData.referrer  At DocumentTabModelSelector.java

build/get_landmines.py

@@ -28,8 +28,7 @@ def print_landmines():
       builder() == 'ninja'):
     print 'Need to clobber winja goma due to backend cwd cache fix.'
   if platform() == 'android':
-    print 'Clobber: To delete newly generated mojo class files.'
-    print 'Clobber to ensure that recipe tests do not break (issue 680923002).'
+    print 'Clobber: to handle new way of suppressing findbugs failures.'
   if platform() == 'win' and builder() == 'ninja':
     print 'Compile on cc_unittests fails due to symbols removed in r185063.'
   if platform() == 'linux' and builder() == 'ninja':
@@ -58,9 +57,6 @@ def print_landmines():
   print '[chromium-dev] PSA: clobber build needed for IDR_INSPECTOR_* compil...'
   print 'blink_resources.grd changed: crbug.com/400860'
   print 'ninja dependency cycle: crbug.com/408192'
-  if platform() == 'android':
-    print 'Delete stale generated .java files yet again. crbug.com/349592'
-    print 'Clobber to delete incompatible object binary format with NDK r10c'
   print 'Clobber to fix missing NaCl gyp dependencies (crbug.com/427427).'
   print 'Another clobber for missing NaCl gyp deps (crbug.com/427427).'
   print 'Clobber to fix GN not picking up increased ID range (crbug.com/444902)'

chrome/android/BUILD.gn

@@ -347,6 +347,7 @@ jinja_template("chrome_shell_manifest") {
 android_apk("chrome_shell_apk") {
   testonly = true
   deps = [
+    "//base:base_java",
     ":chrome_shell_resources",
     ":chrome_shell_java",
     ":chrome_shell_assets",
@@ -383,6 +384,7 @@ jinja_template("chrome_sync_shell_manifest") {
 android_apk("chrome_sync_shell_apk") {
   testonly = true
   deps = [
+    "//base:base_java",
     ":chrome_shell_resources",
     ":chrome_shell_java",
     ":chrome_shell_assets",

chrome/android/java/src/org/chromium/chrome/browser/ChromeBrowserProvider.java

@@ -33,6 +33,7 @@
 import org.chromium.base.CalledByNativeUnchecked;
 import org.chromium.base.ThreadUtils;
 import org.chromium.base.VisibleForTesting;
+import org.chromium.base.annotations.SuppressFBWarnings;
 import org.chromium.chrome.browser.database.SQLiteCursor;
 import org.chromium.sync.notifier.SyncStatusHelper;
 
@@ -386,6 +387,7 @@ public Cursor query(Uri uri, String[] projection, String selection, String[] sel
     }
 
     @Override
+    @SuppressFBWarnings("SF_SWITCH_FALLTHROUGH")
     public Uri insert(Uri uri, ContentValues values) {
         if (!canHandleContentProviderApiCall() || !hasWriteAccess()) return null;
 
@@ -832,13 +834,15 @@ public Type type() {
         /**
          * @return The bookmark favicon, if any.
          */
+        @SuppressFBWarnings("EI_EXPOSE_REP")
         public byte[] favicon() {
             return mFavicon;
         }
 
         /**
          * @return The bookmark thumbnail, if any.
          */
+        @SuppressFBWarnings("EI_EXPOSE_REP")
         public byte[] thumbnail() {
             return mThumbnail;
         }
@@ -907,11 +911,13 @@ private static BookmarkNode create(
         }
 
         @VisibleForTesting
+        @SuppressFBWarnings("EI_EXPOSE_REP2")
         public void setFavicon(byte[] favicon) {
             mFavicon = favicon;
         }
 
         @VisibleForTesting
+        @SuppressFBWarnings("EI_EXPOSE_REP2")
         public void setThumbnail(byte[] thumbnail) {
             mThumbnail = thumbnail;
         }

chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationService.java

@@ -12,6 +12,7 @@
 
 import org.chromium.base.CommandLine;
 import org.chromium.base.ThreadUtils;
+import org.chromium.base.annotations.SuppressFBWarnings;
 import org.chromium.base.library_loader.LibraryLoader;
 import org.chromium.base.library_loader.ProcessInitException;
 import org.chromium.content.app.ContentApplication;
@@ -67,6 +68,7 @@ public void run() {
      *
      * @param intent The intent containing the notification's information.
      */
+    @SuppressFBWarnings("DM_EXIT")
     private void dispatchIntentOnUIThread(Intent intent) {
         Context context = getApplicationContext();
         if (!CommandLine.isInitialized()) {

chrome/android/java/src/org/chromium/chrome/browser/preferences/Preferences.java

@@ -23,6 +23,7 @@
 
 import org.chromium.base.ApiCompatibilityUtils;
 import org.chromium.base.VisibleForTesting;
+import org.chromium.base.annotations.SuppressFBWarnings;
 import org.chromium.base.library_loader.ProcessInitException;
 import org.chromium.chrome.R;
 
@@ -68,6 +69,7 @@ public abstract class Preferences extends ActionBarActivity implements
      */
     public abstract void showUrl(int titleResId, int urlResId);
 
+    @SuppressFBWarnings("DM_EXIT")
     @Override
     protected void onCreate(Bundle savedInstanceState) {
         ensureActivityNotExported();

chrome/android/java/src/org/chromium/chrome/browser/sync/ChromiumSyncAdapter.java

@@ -19,6 +19,7 @@
 
 import org.chromium.base.ThreadUtils;
 import org.chromium.base.VisibleForTesting;
+import org.chromium.base.annotations.SuppressFBWarnings;
 import org.chromium.base.library_loader.ProcessInitException;
 import org.chromium.chrome.browser.invalidation.InvalidationServiceFactory;
 import org.chromium.chrome.browser.profiles.Profile;
@@ -102,6 +103,7 @@ private void startBrowserProcess(
         try {
             ThreadUtils.runOnUiThreadBlocking(new Runnable() {
                 @Override
+                @SuppressFBWarnings("DM_EXIT")
                 public void run() {
                     ContentApplication.initCommandLine(getContext());
                     if (mAsyncStartup) {
@@ -126,6 +128,7 @@ public void run() {
         }
     }
 
+    @SuppressFBWarnings("DM_EXIT")
     private void startBrowserProcessesSync(
             final BrowserStartupController.StartupCallback callback) {
         try {

chrome/android/java/src/org/chromium/chrome/browser/sync/ProfileSyncService.java

@@ -14,6 +14,7 @@
 import org.chromium.base.CalledByNative;
 import org.chromium.base.ThreadUtils;
 import org.chromium.base.VisibleForTesting;
+import org.chromium.base.annotations.SuppressFBWarnings;
 import org.chromium.chrome.browser.identity.UniqueIdentificationGenerator;
 import org.chromium.chrome.browser.invalidation.InvalidationServiceFactory;
 import org.chromium.chrome.browser.profiles.Profile;
@@ -74,6 +75,7 @@ public interface SyncStateChangedListener {
      * @param context the ApplicationContext is retrieved from the context used as an argument.
      * @return a singleton instance of the SyncSetupManager
      */
+    @SuppressFBWarnings("LI_LAZY_INIT")
     public static ProfileSyncService get(Context context) {
         ThreadUtils.assertOnUiThread();
         if (sSyncSetupManager == null) {

chrome/android/java/src/org/chromium/chrome/browser/tabmodel/document/DocumentTabModel.java

@@ -8,6 +8,7 @@
 
 import org.chromium.base.ThreadUtils;
 import org.chromium.base.VisibleForTesting;
+import org.chromium.base.annotations.SuppressFBWarnings;
 import org.chromium.chrome.browser.Tab;
 import org.chromium.chrome.browser.TabState;
 import org.chromium.chrome.browser.tabmodel.TabModel;
@@ -18,6 +19,7 @@
  */
 public interface DocumentTabModel extends TabModel {
     /** Stores information about a DocumentActivity. */
+    @SuppressFBWarnings({"URF_UNREAD", "UUF_UNUSED"})
     public static final class Entry {
         public final int tabId;
         public boolean canGoBack;