Revert of Handle Java assert using a customized function in base (patchset #11...

Revert of Handle Java assert using a customized function in base (patchset #11 id:470001 of https://codereview.chromium.org/2971063003/ )

Reason for revert:
Reland changes weren't reviewed

Original issue's description:
> Create java_assertion_handler, a Java binary that takes in a JAR file
> and modifies the bytecode of classes.
>
> Replace the Java ASSERT with JavaExceptionReporter.assertFailureHandler
> that throws Exception.
>
> Output is written to a new JAR file.
>
>
> BUG=672945
>
> Review-Url: https://codereview.chromium.org/2971063003
> Cr-Original-Commit-Position: refs/heads/master@{#487903}
> Committed: https://chromium.googlesource.com/chromium/src/+/33133e5767a9c1ca1f7e9defad2d7b3347065589
> Review-Url: https://codereview.chromium.org/2971063003
> Cr-Commit-Position: refs/heads/master@{#488381}
> Committed: https://chromium.googlesource.com/chromium/src/+/bef3daf02ae6035f166782025fd7e4e54d465271

TBR=yfriedman@chromium.org,agrieve@chromium.org,zpeng@chromium.org,ranj@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=672945

Review-Url: https://codereview.chromium.org/2985613002
Cr-Commit-Position: refs/heads/master@{#488390}

base/BUILD.gn

@@ -2545,7 +2545,6 @@ if (is_android) {
     ]
 
     deps = [
-      "//build/android:build_hooks_java",
       "//third_party/android_tools:android_support_annotations_java",
       "//third_party/android_tools:android_support_multidex_java",
       "//third_party/jsr-305:jsr_305_javalib",

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

@@ -9,7 +9,6 @@ import android.support.annotation.UiThread;
 import org.chromium.base.annotations.CalledByNative;
 import org.chromium.base.annotations.JNINamespace;
 import org.chromium.base.annotations.MainDex;
-import org.chromium.buildhooks.Callback;
 
 /**
  * This UncaughtExceptionHandler will create a breakpad minidump when there is an uncaught
@@ -55,16 +54,6 @@ public class JavaExceptionReporter implements Thread.UncaughtExceptionHandler {
         nativeReportJavaStackTrace(stackTrace);
     }
 
-    /**
-     * This class will be passed in ChromeApplication that handlers all AssertionError.
-     */
-    public static class AssertFailureCallback implements Callback<AssertionError> {
-        @Override
-        public void run(AssertionError assertionError) {
-            throw assertionError;
-        }
-    }
-
     @CalledByNative
     private static void installHandler(boolean crashAfterReport) {
         Thread.setDefaultUncaughtExceptionHandler(new JavaExceptionReporter(

base/android/java/src/org/chromium/base/process_launcher/ChildProcessLauncher.java

@@ -224,6 +224,7 @@ public class ChildProcessLauncher {
             });
             return false;
         }
+        assert mConnection != null;
 
         if (setupConnection) {
             setupConnection();

build/android/BUILD.gn

@@ -6,14 +6,6 @@ import("//build/config/android/config.gni")
 import("//build/config/android/rules.gni")
 
 if (enable_java_templates) {
-  android_library("build_hooks_java") {
-    java_files = [
-      "buildhooks/java/org/chromium/buildhooks/BuildHooks.java",
-      "buildhooks/java/org/chromium/buildhooks/Callback.java",
-    ]
-    no_build_hooks = true
-  }
-
   import("//third_party/ijar/ijar.gni")
 
   sun_tools_jar_path = "$root_gen_dir/sun_tools_jar/tools.jar"

build/android/buildhooks/java/org/chromium/buildhooks/BuildHooks.java

-// 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.buildhooks;
-
-/**
- * Callback interface.
- */
-public class BuildHooks {
-    static Callback<AssertionError> sAssertCallback;
-    /**
-     * Handle AssertionError, decide whether throw or report without crashing base on gn arg.
-     */
-    public static void assertFailureHandler(AssertionError assertionError) throws AssertionError {
-        if (sAssertCallback != null) {
-            sAssertCallback.run(assertionError);
-        } else {
-            throw assertionError;
-        }
-    }
-
-    public static void setAssertCallback(Callback<AssertionError> callback) {
-        sAssertCallback = callback;
-    }
-}

build/android/buildhooks/java/org/chromium/buildhooks/Callback.java

-// 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.buildhooks;
-
-/**
- * Callback interface.
- */
-public interface Callback<T> { void run(T arg); }

build/android/java_assertion_enabler/java/org/chromium/javaassertionenabler/AssertionEnabler.java

@@ -7,7 +7,6 @@ package org.chromium.javaassertionenabler;
 import org.objectweb.asm.ClassReader;
 import org.objectweb.asm.ClassVisitor;
 import org.objectweb.asm.ClassWriter;
-import org.objectweb.asm.Label;
 import org.objectweb.asm.MethodVisitor;
 import org.objectweb.asm.Opcodes;
 
@@ -27,44 +26,15 @@ import java.util.zip.ZipInputStream;
 import java.util.zip.ZipOutputStream;
 
 /**
- * An application that replace Java ASSERT statements with a function by modifying Java bytecode. It
- * takes in a JAR file, modifies bytecode of classes that use ASSERT, and outputs the bytecode to a
- * new JAR file.
- *
- * We do this in two steps, first step is to enable assert.
- * Following bytecode is generated for each class with ASSERT statements:
- * 0: ldc #8 // class CLASSNAME
- * 2: invokevirtual #9 // Method java/lang/Class.desiredAssertionStatus:()Z
- * 5: ifne 12
- * 8: iconst_1
- * 9: goto 13
- * 12: iconst_0
- * 13: putstatic #2 // Field $assertionsDisabled:Z
- * Replaces line #13 to the following:
- * 13: pop
- * Consequently, $assertionsDisabled is assigned the default value FALSE.
- * This is done in the first if statement in overridden visitFieldInsn. We do this per per-assert.
- *
- * Second step is to replace assert statement with a function:
- * The followed instructions are generated by a java assert statement:
- * getstatic     #3     // Field $assertionsDisabled:Z
- * ifne          118    // Jump to instruction as if assertion if not enabled
- * ...
- * ifne          19
- * new           #4     // class java/lang/AssertionError
- * dup
- * ldc           #5     // String (don't have this line if no assert message given)
- * invokespecial #6     // Method java/lang/AssertionError.
- * athrow
- * Replace athrow with:
- * invokestatic  #7     // Method org/chromium/base/JavaExceptionReporter.assertFailureHandler
- * goto          118
- * JavaExceptionReporter.assertFailureHandler is a function that handles the AssertionError,
- * 118 is the instruction to execute as if assertion if not enabled.
+ * An application that enables Java ASSERT statements by modifying Java bytecode. It takes in a JAR
+ * file, modifies bytecode of classes that use ASSERT, and outputs the bytecode to a new JAR file.
  */
 class AssertionEnabler {
+    static final String ASSERTION_DISABLED_NAME = "$assertionsDisabled";
     static final String CLASS_FILE_SUFFIX = ".class";
+    static final String STATIC_INITIALIZER_NAME = "<clinit>";
     static final String TEMPORARY_FILE_SUFFIX = ".temp";
+
     static final int BUFFER_SIZE = 16384;
 
     static class AssertionEnablerVisitor extends ClassVisitor {
@@ -75,56 +45,33 @@ class AssertionEnabler {
         @Override
         public MethodVisitor visitMethod(final int access, final String name, String desc,
                 String signature, String[] exceptions) {
-            return new RewriteAssertMethodVisitorWriter(
-                    Opcodes.ASM5, super.visitMethod(access, name, desc, signature, exceptions)) {};
-        }
-    }
-
-    static class RewriteAssertMethodVisitorWriter extends MethodVisitor {
-        static final String ASSERTION_DISABLED_NAME = "$assertionsDisabled";
-        static final String INSERT_INSTRUCTION_OWNER = "org/chromium/buildhooks/BuildHooks";
-        static final String INSERT_INSTRUCTION_NAME = "assertFailureHandler";
-        static final String INSERT_INSTRUCTION_DESC = "(Ljava/lang/AssertionError;)V";
-        static final boolean INSERT_INSTRUCTION_ITF = false;
-
-        boolean mStartLoadingAssert;
-        Label mGotoLabel;
-
-        public RewriteAssertMethodVisitorWriter(int api, MethodVisitor mv) {
-            super(api, mv);
-        }
-
-        @Override
-        public void visitFieldInsn(int opcode, String owner, String name, String desc) {
-            if (opcode == Opcodes.PUTSTATIC && name.equals(ASSERTION_DISABLED_NAME)) {
-                super.visitInsn(Opcodes.POP); // enable assert
-            } else if (opcode == Opcodes.GETSTATIC && name.equals(ASSERTION_DISABLED_NAME)) {
-                mStartLoadingAssert = true;
-                super.visitFieldInsn(opcode, owner, name, desc);
-            } else {
-                super.visitFieldInsn(opcode, owner, name, desc);
-            }
-        }
-
-        @Override
-        public void visitJumpInsn(int opcode, Label label) {
-            if (mStartLoadingAssert && opcode == Opcodes.IFNE && mGotoLabel == null) {
-                mGotoLabel = label;
-            }
-            super.visitJumpInsn(opcode, label);
-        }
-
-        @Override
-        public void visitInsn(int opcode) {
-            if (!mStartLoadingAssert || opcode != Opcodes.ATHROW) {
-                super.visitInsn(opcode);
-            } else {
-                super.visitMethodInsn(Opcodes.INVOKESTATIC, INSERT_INSTRUCTION_OWNER,
-                        INSERT_INSTRUCTION_NAME, INSERT_INSTRUCTION_DESC, INSERT_INSTRUCTION_ITF);
-                super.visitJumpInsn(Opcodes.GOTO, mGotoLabel);
-                mStartLoadingAssert = false;
-                mGotoLabel = null;
+            // Patch static initializer.
+            if ((access & Opcodes.ACC_STATIC) != 0 && name.equals(STATIC_INITIALIZER_NAME)) {
+                return new MethodVisitor(Opcodes.ASM5,
+                        super.visitMethod(access, name, desc, signature, exceptions)) {
+                    // The following bytecode is generated for each class with ASSERT statements:
+                    // 0: ldc #8 // class CLASSNAME
+                    // 2: invokevirtual #9 // Method java/lang/Class.desiredAssertionStatus:()Z
+                    // 5: ifne 12
+                    // 8: iconst_1
+                    // 9: goto 13
+                    // 12: iconst_0
+                    // 13: putstatic #2 // Field $assertionsDisabled:Z
+                    //
+                    // This function replaces line #13 to the following:
+                    // 13: pop
+                    // Consequently, $assertionsDisabled is assigned the default value FALSE.
+                    @Override
+                    public void visitFieldInsn(int opcode, String owner, String name, String desc) {
+                        if (opcode == Opcodes.PUTSTATIC && name.equals(ASSERTION_DISABLED_NAME)) {
+                            mv.visitInsn(Opcodes.POP);
+                        } else {
+                            super.visitFieldInsn(opcode, owner, name, desc);
+                        }
+                    }
+                };
             }
+            return super.visitMethod(access, name, desc, signature, exceptions);
         }
     }
 
@@ -135,15 +82,16 @@ class AssertionEnabler {
         while ((numRead = inputStream.read(data, 0, data.length)) != -1) {
             buffer.write(data, 0, numRead);
         }
+
         return buffer.toByteArray();
     }
 
     static void enableAssertionInJar(String inputJarPath, String outputJarPath) {
         String tempJarPath = outputJarPath + TEMPORARY_FILE_SUFFIX;
         try (ZipInputStream inputStream = new ZipInputStream(
-                     new BufferedInputStream(new FileInputStream(inputJarPath)));
-                ZipOutputStream tempStream = new ZipOutputStream(
-                        new BufferedOutputStream(new FileOutputStream(tempJarPath)))) {
+                    new BufferedInputStream(new FileInputStream(inputJarPath)));
+             ZipOutputStream tempStream = new ZipOutputStream(
+                    new BufferedOutputStream(new FileOutputStream(tempJarPath)))) {
             ZipEntry entry = null;
 
             while ((entry = inputStream.getNextEntry()) != null) {
@@ -182,8 +130,6 @@ class AssertionEnabler {
             System.out.println("Example usage: java_assertion_enabler input.jar output.jar");
             System.exit(-1);
         }
-        String inputJarPath = args[0];
-        String outputJarPath = args[1];
-        enableAssertionInJar(inputJarPath, outputJarPath);
+        enableAssertionInJar(args[0], args[1]);
     }
 }

build/config/android/internal_rules.gni

@@ -2440,12 +2440,6 @@ if (enable_java_templates) {
     }
     assert(_android_manifest != "")  # Mark as used.
 
-    if (_supports_android &&
-        (!defined(invoker.no_build_hooks) ||
-         (defined(invoker.no_build_hooks) && !invoker.no_build_hooks))) {
-      _accumulated_deps += [ "//build/android:build_hooks_java" ]
-    }
-
     if (defined(invoker.run_findbugs_override)) {
       _run_findbugs = invoker.run_findbugs_override
     } else {

chrome/android/BUILD.gn

@@ -181,7 +181,6 @@ android_library("chrome_java") {
     ":document_tab_model_info_proto_java",
     ":partner_location_descriptor_proto_java",
     "//base:base_java",
-    "//build/android:build_hooks_java",
     "//chrome/android/webapk/libs/client:client_java",
     "//chrome/android/webapk/libs/common:common_java",
     "//chrome/android/webapk/libs/runtime_library:webapk_service_aidl_java",
@@ -734,29 +733,29 @@ if (current_toolchain == default_toolchain) {
               "/libmonochrome$shlib_extension.whitelist"
       output = monochrome_resource_whitelist
     }
-
+    
     # Use custom resource ID list instead of android_webview's compiler
     # resource whitelist because //android_webview: generate_webui_resources
     # and //android_webview: generate_components_resources use hand-written
     # resource whitelists.
     action("system_webview_locale_resource_id_list") {
       script = "//tools/grit/pak_util.py"
-
+      
       _system_webview_en_US_locale_pak =
           "$root_out_dir/android_webview/locales/en-US.pak"
-
+      
       inputs = [
         _system_webview_en_US_locale_pak,
       ]
-
+      
       outputs = [
         system_webview_locale_resource_id_list,
       ]
-
+      
       deps = [
         "//android_webview:repack_locales",
       ]
-
+      
       args = [
         "list-id",
         "--output",
@@ -764,25 +763,25 @@ if (current_toolchain == default_toolchain) {
         rebase_path(_system_webview_en_US_locale_pak, root_build_dir),
       ]
     }
-
+    
     action("monochrome_locale_whitelist") {
       script = "//tools/resources/filter_resource_whitelist.py"
-
+      
       inputs = [
         monochrome_resource_whitelist,
         system_webview_locale_resource_id_list,
       ]
-
+      
       outputs = [
         monochrome_locale_whitelist,
       ]
-
+      
       deps = [
         ":monochrome_resource_whitelist",
         ":system_webview_locale_resource_id_list",
         "//android_webview:system_webview_pak_whitelist",
       ]
-
+      
       args = [
         "--input",
         rebase_path(monochrome_resource_whitelist, root_build_dir),
@@ -791,7 +790,7 @@ if (current_toolchain == default_toolchain) {
         "--output",
         rebase_path(monochrome_locale_whitelist, root_build_dir),
       ]
-    }
+    } 
   }
 
   chrome_paks("monochrome_paks") {
@@ -809,7 +808,7 @@ if (current_toolchain == default_toolchain) {
       deps += [ ":monochrome_locale_whitelist" ]
     }
   }
-
+  
   # This target is separate from monochrome_pak_assets because it does not
   # disable compression.
   android_assets("monochrome_locale_pak_assets") {
@@ -817,7 +816,7 @@ if (current_toolchain == default_toolchain) {
     foreach(_locale, locales - android_chrome_omitted_locales) {
       sources += [ "$target_gen_dir/monochrome_paks/locales/$_locale.pak" ]
     }
-
+  
     deps = [
       ":monochrome_paks",
     ]

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

@@ -17,7 +17,6 @@ import org.chromium.base.ContextUtils;
 import org.chromium.base.Log;
 import org.chromium.base.VisibleForTesting;
 import org.chromium.base.annotations.CalledByNative;
-import org.chromium.base.annotations.SuppressFBWarnings;
 import org.chromium.base.metrics.RecordHistogram;
 import org.chromium.chrome.browser.externalauth.ExternalAuthUtils;
 import org.chromium.chrome.browser.externalauth.UserRecoverableErrorHandler;
@@ -71,7 +70,6 @@ public class BackgroundSyncLauncher {
     /**
      * Called when the C++ counterpart is deleted.
      */
-    @SuppressFBWarnings("ST_WRITE_TO_STATIC_FROM_INSTANCE_METHOD")
     @VisibleForTesting
     @CalledByNative
     protected void destroy() {

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

@@ -13,13 +13,11 @@ import org.chromium.base.ActivityState;
 import org.chromium.base.ApplicationStatus;
 import org.chromium.base.CommandLineInitUtil;
 import org.chromium.base.ContextUtils;
-import org.chromium.base.JavaExceptionReporter;
 import org.chromium.base.ThreadUtils;
 import org.chromium.base.TraceEvent;
 import org.chromium.base.annotations.MainDex;
 import org.chromium.base.annotations.SuppressFBWarnings;
 import org.chromium.base.library_loader.ProcessInitException;
-import org.chromium.buildhooks.BuildHooks;
 import org.chromium.chrome.browser.document.DocumentActivity;
 import org.chromium.chrome.browser.document.IncognitoDocumentActivity;
 import org.chromium.chrome.browser.init.InvalidStartupDialog;
@@ -48,7 +46,6 @@ public class ChromeApplication extends ContentApplication {
     protected void attachBaseContext(Context base) {
         super.attachBaseContext(base);
         ContextUtils.initApplicationContext(this);
-        BuildHooks.setAssertCallback(new JavaExceptionReporter.AssertFailureCallback());
     }
 
     /**

chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java

@@ -1555,6 +1555,7 @@ public class ContextualSearchManager implements ContextualSearchManagementDelega
                 if (mContext == null) return;
 
                 // Update the UI to show the resolve is in progress.
+                assert mContext.getTextContentFollowingSelection() != null;
                 mSearchPanel.setContextDetails(
                         selection, mContext.getTextContentFollowingSelection());
             }

chrome/android/java/src/org/chromium/chrome/browser/offlinepages/OfflinePageTabObserver.java

@@ -109,6 +109,7 @@ public class OfflinePageTabObserver
      */
     public static void addObserverForTab(Tab tab) {
         OfflinePageTabObserver observer = getObserverForActivity(tab.getActivity());
+        assert observer != null;
         observer.startObservingTab(tab);
         observer.maybeShowReloadSnackbar(tab, false);
     }

chrome/android/webapk/libs/runtime_library/src/org/chromium/webapk/lib/runtime_library/WebApkServiceImpl.java

@@ -44,6 +44,7 @@ public class WebApkServiceImpl extends IWebApkApi.Stub {
         mContext = context;
         mSmallIconId = bundle.getInt(KEY_SMALL_ICON_ID);
         mHostUid = bundle.getInt(KEY_HOST_BROWSER_UID);
+        assert mHostUid >= 0;
     }
 
     @Override

chrome/android/webapk/shell_apk/shell_apk_version.gni

@@ -6,7 +6,7 @@
 # (including AndroidManifest.xml) is updated. This version should be incremented
 # prior to uploading a new ShellAPK to the WebAPK Minting Server.
 # Does not affect Chrome.apk
-template_shell_apk_version = 16
+template_shell_apk_version = 15
 
 # The ShellAPK version expected by Chrome. Chrome will try to update the WebAPK
 # if the WebAPK's ShellAPK version is less than |expected_shell_apk_version|.

chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/HostBrowserClassLoader.java

@@ -7,6 +7,7 @@ package org.chromium.webapk.shell_apk;
 import android.content.Context;
 import android.content.SharedPreferences;
 import android.content.pm.PackageManager;
+import android.os.Looper;
 import android.util.Log;
 
 import org.chromium.webapk.lib.common.WebApkConstants;
@@ -50,6 +51,7 @@ public class HostBrowserClassLoader {
      * @return The ClassLoader.
      */
     public static ClassLoader getClassLoaderInstance(Context context, String canaryClassName) {
+        assertRunningOnUiThread();
         Context remoteContext = WebApkUtils.getHostBrowserContext(context);
         if (remoteContext == null) {
             Log.w(TAG, "Failed to get remote context.");
@@ -171,4 +173,11 @@ public class HostBrowserClassLoader {
         }
         return value;
     }
+
+    /**
+     * Asserts that current thread is the UI thread.
+     */
+    private static void assertRunningOnUiThread() {
+        assert Looper.getMainLooper().equals(Looper.myLooper());
+    }
 }

chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/WebApkSandboxedProcessService.java

@@ -62,6 +62,7 @@ public class WebApkSandboxedProcessService extends Service {
             Method bindMethod =
                     mWebApkChildProcessServiceImplClass.getMethod("bind", Intent.class, int.class);
             int hostBrowserUid = WebApkUtils.getHostBrowserUid(this);
+            assert hostBrowserUid >= 0;
             return (IBinder) bindMethod.invoke(
                     mWebApkChildProcessServiceImplInstance, intent, hostBrowserUid);
         } catch (NoSuchMethodException | IllegalAccessException | InvocationTargetException e) {