Commit 14d4eb4f authored by mgersh's avatar mgersh Committed by Commit Bot

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

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

Reason for revert:
Broke compile on Cronet bots with error about "can't find referenced class
org.chromium.base.JavaExceptionReporter": https://build.chromium.org/p/chromium.android/builders/Android%20Cronet%20x86%20Builder%20%28dbg%29/builds/61854

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-Commit-Position: refs/heads/master@{#487903}
> Committed: https://chromium.googlesource.com/chromium/src/+/33133e5767a9c1ca1f7e9defad2d7b3347065589

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/2981273003
Cr-Commit-Position: refs/heads/master@{#487914}
parent c6e56f94
...@@ -54,13 +54,6 @@ public class JavaExceptionReporter implements Thread.UncaughtExceptionHandler { ...@@ -54,13 +54,6 @@ public class JavaExceptionReporter implements Thread.UncaughtExceptionHandler {
nativeReportJavaStackTrace(stackTrace); nativeReportJavaStackTrace(stackTrace);
} }
/**
* Handle AssertionError, decide whether throw or report without crashing base on gn arg.
*/
public static void assertFailureHandler(AssertionError assertionError) throws AssertionError {
throw assertionError;
}
@CalledByNative @CalledByNative
private static void installHandler(boolean crashAfterReport) { private static void installHandler(boolean crashAfterReport) {
Thread.setDefaultUncaughtExceptionHandler(new JavaExceptionReporter( Thread.setDefaultUncaughtExceptionHandler(new JavaExceptionReporter(
......
...@@ -224,6 +224,7 @@ public class ChildProcessLauncher { ...@@ -224,6 +224,7 @@ public class ChildProcessLauncher {
}); });
return false; return false;
} }
assert mConnection != null;
if (setupConnection) { if (setupConnection) {
setupConnection(); setupConnection();
......
...@@ -7,7 +7,6 @@ package org.chromium.javaassertionenabler; ...@@ -7,7 +7,6 @@ package org.chromium.javaassertionenabler;
import org.objectweb.asm.ClassReader; import org.objectweb.asm.ClassReader;
import org.objectweb.asm.ClassVisitor; import org.objectweb.asm.ClassVisitor;
import org.objectweb.asm.ClassWriter; import org.objectweb.asm.ClassWriter;
import org.objectweb.asm.Label;
import org.objectweb.asm.MethodVisitor; import org.objectweb.asm.MethodVisitor;
import org.objectweb.asm.Opcodes; import org.objectweb.asm.Opcodes;
...@@ -27,44 +26,15 @@ import java.util.zip.ZipInputStream; ...@@ -27,44 +26,15 @@ import java.util.zip.ZipInputStream;
import java.util.zip.ZipOutputStream; import java.util.zip.ZipOutputStream;
/** /**
* An application that replace Java ASSERT statements with a function by modifying Java bytecode. It * An application that enables Java ASSERT statements by modifying Java bytecode. It takes in a JAR
* takes in a JAR file, modifies bytecode of classes that use ASSERT, and outputs the bytecode to a * file, modifies bytecode of classes that use ASSERT, and outputs the bytecode to a new JAR file.
* 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.
*/ */
class AssertionEnabler { class AssertionEnabler {
static final String ASSERTION_DISABLED_NAME = "$assertionsDisabled";
static final String CLASS_FILE_SUFFIX = ".class"; static final String CLASS_FILE_SUFFIX = ".class";
static final String STATIC_INITIALIZER_NAME = "<clinit>";
static final String TEMPORARY_FILE_SUFFIX = ".temp"; static final String TEMPORARY_FILE_SUFFIX = ".temp";
static final int BUFFER_SIZE = 16384; static final int BUFFER_SIZE = 16384;
static class AssertionEnablerVisitor extends ClassVisitor { static class AssertionEnablerVisitor extends ClassVisitor {
...@@ -75,56 +45,33 @@ class AssertionEnabler { ...@@ -75,56 +45,33 @@ class AssertionEnabler {
@Override @Override
public MethodVisitor visitMethod(final int access, final String name, String desc, public MethodVisitor visitMethod(final int access, final String name, String desc,
String signature, String[] exceptions) { String signature, String[] exceptions) {
return new RewriteAssertMethodVisitorWriter( // Patch static initializer.
Opcodes.ASM5, super.visitMethod(access, name, desc, signature, exceptions)) {}; 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:
static class RewriteAssertMethodVisitorWriter extends MethodVisitor { // 0: ldc #8 // class CLASSNAME
static final String ASSERTION_DISABLED_NAME = "$assertionsDisabled"; // 2: invokevirtual #9 // Method java/lang/Class.desiredAssertionStatus:()Z
static final String INSERT_INSTRUCTION_OWNER = "org/chromium/base/JavaExceptionReporter"; // 5: ifne 12
static final String INSERT_INSTRUCTION_NAME = "assertFailureHandler"; // 8: iconst_1
static final String INSERT_INSTRUCTION_DESC = "(Ljava/lang/AssertionError;)V"; // 9: goto 13
static final boolean INSERT_INSTRUCTION_ITF = false; // 12: iconst_0
// 13: putstatic #2 // Field $assertionsDisabled:Z
boolean mStartLoadingAssert; //
Label mGotoLabel; // This function replaces line #13 to the following:
// 13: pop
public RewriteAssertMethodVisitorWriter(int api, MethodVisitor mv) { // Consequently, $assertionsDisabled is assigned the default value FALSE.
super(api, mv); @Override
} public void visitFieldInsn(int opcode, String owner, String name, String desc) {
if (opcode == Opcodes.PUTSTATIC && name.equals(ASSERTION_DISABLED_NAME)) {
@Override mv.visitInsn(Opcodes.POP);
public void visitFieldInsn(int opcode, String owner, String name, String desc) { } else {
if (opcode == Opcodes.PUTSTATIC && name.equals(ASSERTION_DISABLED_NAME)) { super.visitFieldInsn(opcode, owner, name, desc);
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;
} }
return super.visitMethod(access, name, desc, signature, exceptions);
} }
} }
...@@ -135,15 +82,16 @@ class AssertionEnabler { ...@@ -135,15 +82,16 @@ class AssertionEnabler {
while ((numRead = inputStream.read(data, 0, data.length)) != -1) { while ((numRead = inputStream.read(data, 0, data.length)) != -1) {
buffer.write(data, 0, numRead); buffer.write(data, 0, numRead);
} }
return buffer.toByteArray(); return buffer.toByteArray();
} }
static void enableAssertionInJar(String inputJarPath, String outputJarPath) { static void enableAssertionInJar(String inputJarPath, String outputJarPath) {
String tempJarPath = outputJarPath + TEMPORARY_FILE_SUFFIX; String tempJarPath = outputJarPath + TEMPORARY_FILE_SUFFIX;
try (ZipInputStream inputStream = new ZipInputStream( try (ZipInputStream inputStream = new ZipInputStream(
new BufferedInputStream(new FileInputStream(inputJarPath))); new BufferedInputStream(new FileInputStream(inputJarPath)));
ZipOutputStream tempStream = new ZipOutputStream( ZipOutputStream tempStream = new ZipOutputStream(
new BufferedOutputStream(new FileOutputStream(tempJarPath)))) { new BufferedOutputStream(new FileOutputStream(tempJarPath)))) {
ZipEntry entry = null; ZipEntry entry = null;
while ((entry = inputStream.getNextEntry()) != null) { while ((entry = inputStream.getNextEntry()) != null) {
...@@ -182,8 +130,6 @@ class AssertionEnabler { ...@@ -182,8 +130,6 @@ class AssertionEnabler {
System.out.println("Example usage: java_assertion_enabler input.jar output.jar"); System.out.println("Example usage: java_assertion_enabler input.jar output.jar");
System.exit(-1); System.exit(-1);
} }
String inputJarPath = args[0]; enableAssertionInJar(args[0], args[1]);
String outputJarPath = args[1];
enableAssertionInJar(inputJarPath, outputJarPath);
} }
} }
...@@ -1555,6 +1555,7 @@ public class ContextualSearchManager implements ContextualSearchManagementDelega ...@@ -1555,6 +1555,7 @@ public class ContextualSearchManager implements ContextualSearchManagementDelega
if (mContext == null) return; if (mContext == null) return;
// Update the UI to show the resolve is in progress. // Update the UI to show the resolve is in progress.
assert mContext.getTextContentFollowingSelection() != null;
mSearchPanel.setContextDetails( mSearchPanel.setContextDetails(
selection, mContext.getTextContentFollowingSelection()); selection, mContext.getTextContentFollowingSelection());
} }
......
...@@ -109,6 +109,7 @@ public class OfflinePageTabObserver ...@@ -109,6 +109,7 @@ public class OfflinePageTabObserver
*/ */
public static void addObserverForTab(Tab tab) { public static void addObserverForTab(Tab tab) {
OfflinePageTabObserver observer = getObserverForActivity(tab.getActivity()); OfflinePageTabObserver observer = getObserverForActivity(tab.getActivity());
assert observer != null;
observer.startObservingTab(tab); observer.startObservingTab(tab);
observer.maybeShowReloadSnackbar(tab, false); observer.maybeShowReloadSnackbar(tab, false);
} }
......
...@@ -44,6 +44,7 @@ public class WebApkServiceImpl extends IWebApkApi.Stub { ...@@ -44,6 +44,7 @@ public class WebApkServiceImpl extends IWebApkApi.Stub {
mContext = context; mContext = context;
mSmallIconId = bundle.getInt(KEY_SMALL_ICON_ID); mSmallIconId = bundle.getInt(KEY_SMALL_ICON_ID);
mHostUid = bundle.getInt(KEY_HOST_BROWSER_UID); mHostUid = bundle.getInt(KEY_HOST_BROWSER_UID);
assert mHostUid >= 0;
} }
@Override @Override
......
...@@ -6,7 +6,7 @@ ...@@ -6,7 +6,7 @@
# (including AndroidManifest.xml) is updated. This version should be incremented # (including AndroidManifest.xml) is updated. This version should be incremented
# prior to uploading a new ShellAPK to the WebAPK Minting Server. # prior to uploading a new ShellAPK to the WebAPK Minting Server.
# Does not affect Chrome.apk # 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 # 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|. # if the WebAPK's ShellAPK version is less than |expected_shell_apk_version|.
......
...@@ -7,6 +7,7 @@ package org.chromium.webapk.shell_apk; ...@@ -7,6 +7,7 @@ package org.chromium.webapk.shell_apk;
import android.content.Context; import android.content.Context;
import android.content.SharedPreferences; import android.content.SharedPreferences;
import android.content.pm.PackageManager; import android.content.pm.PackageManager;
import android.os.Looper;
import android.util.Log; import android.util.Log;
import org.chromium.webapk.lib.common.WebApkConstants; import org.chromium.webapk.lib.common.WebApkConstants;
...@@ -50,6 +51,7 @@ public class HostBrowserClassLoader { ...@@ -50,6 +51,7 @@ public class HostBrowserClassLoader {
* @return The ClassLoader. * @return The ClassLoader.
*/ */
public static ClassLoader getClassLoaderInstance(Context context, String canaryClassName) { public static ClassLoader getClassLoaderInstance(Context context, String canaryClassName) {
assertRunningOnUiThread();
Context remoteContext = WebApkUtils.getHostBrowserContext(context); Context remoteContext = WebApkUtils.getHostBrowserContext(context);
if (remoteContext == null) { if (remoteContext == null) {
Log.w(TAG, "Failed to get remote context."); Log.w(TAG, "Failed to get remote context.");
...@@ -171,4 +173,11 @@ public class HostBrowserClassLoader { ...@@ -171,4 +173,11 @@ public class HostBrowserClassLoader {
} }
return value; return value;
} }
/**
* Asserts that current thread is the UI thread.
*/
private static void assertRunningOnUiThread() {
assert Looper.getMainLooper().equals(Looper.myLooper());
}
} }
...@@ -62,6 +62,7 @@ public class WebApkSandboxedProcessService extends Service { ...@@ -62,6 +62,7 @@ public class WebApkSandboxedProcessService extends Service {
Method bindMethod = Method bindMethod =
mWebApkChildProcessServiceImplClass.getMethod("bind", Intent.class, int.class); mWebApkChildProcessServiceImplClass.getMethod("bind", Intent.class, int.class);
int hostBrowserUid = WebApkUtils.getHostBrowserUid(this); int hostBrowserUid = WebApkUtils.getHostBrowserUid(this);
assert hostBrowserUid >= 0;
return (IBinder) bindMethod.invoke( return (IBinder) bindMethod.invoke(
mWebApkChildProcessServiceImplInstance, intent, hostBrowserUid); mWebApkChildProcessServiceImplInstance, intent, hostBrowserUid);
} catch (NoSuchMethodException | IllegalAccessException | InvocationTargetException e) { } catch (NoSuchMethodException | IllegalAccessException | InvocationTargetException e) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment