Handle Java assert using a customized function in base

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

[Ran, 2017-07-05 23:53:22]

Description was changed from

==========
Create java_assertion_handler, a Java binary that takes in a JAR file which
modifies the bytecode of classes to replace the Java ASSERT with a customized
function that report the failure without actual crashing the program
Outputs them to a new JAR file.

BUG=672945
==========

to

==========
Create java_assertion_handler, a Java binary that takes in a JAR file which
modifies the bytecode of classes.

Replace the Java ASSERT with a customized function that report the
failure without actual crashing the program.

Output is written to a new JAR file.

BUG=672945
==========

[Ran, 2017-07-05 23:53:39]

Description was changed from

==========
Create java_assertion_handler, a Java binary that takes in a JAR file which
modifies the bytecode of classes.

Replace the Java ASSERT with a customized function that report the
failure without actual crashing the program.

Output is written to a new JAR file.

BUG=672945
==========

to

==========
Create java_assertion_handler, a Java binary that takes in a JAR file
which modifies the bytecode of classes.

Replace the Java ASSERT with a customized function that report the
failure without actual crashing the program.

Output is written to a new JAR file.

BUG=672945
==========

[Ran, 2017-07-06 00:50:25]

Patchset #1 (id:1) has been deleted

[Ran, 2017-07-06 00:51:32]

Description was changed from

==========
Create java_assertion_handler, a Java binary that takes in a JAR file
which modifies the bytecode of classes.

Replace the Java ASSERT with a customized function that report the
failure without actual crashing the program.

Output is written to a new JAR file.

BUG=672945
==========

to

==========
Create java_assertion_handler, a Java binary that takes in a JAR file
which modifies the bytecode of classes.

Replace the Java ASSERT with JavaExceptionReporter.assertFailureHandler
that report the failure without actual crashing the program.

Output is written to a new JAR file.

BUG=672945
==========

[Ran, 2017-07-06 00:51:54]

Description was changed from

==========
Create java_assertion_handler, a Java binary that takes in a JAR file
which modifies the bytecode of classes.

Replace the Java ASSERT with JavaExceptionReporter.assertFailureHandler
that report the failure without actual crashing the program.

Output is written to a new JAR file.

BUG=672945
==========

to

==========
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 report the failure without actual crashing the program.

Output is written to a new JAR file.

BUG=672945
==========

[Ran, 2017-07-06 00:52:45]

Description was changed from

==========
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 report the failure without actual crashing the program.

Output is written to a new JAR file.

BUG=672945
==========

to

==========
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 report the failure without actual crashing the program.

Output is written to a new JAR file.

We would like to log assert in Canary for a short amount of time.

BUG=672945
==========

[Ran, 2017-07-06 00:53:20]

Description was changed from

==========
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 report the failure without actual crashing the program.

Output is written to a new JAR file.

We would like to log assert in Canary for a short amount of time.

BUG=672945
==========

to

==========
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 report the failure without actual crashing the program.

Output is written to a new JAR file.

We would like to report assert failure in Canary for a short amount
of time without crashing.
BUG=672945
==========

[Ran, 2017-07-06 00:56:47]

Patchset #1 (id:20001) has been deleted

[Ran, 2017-07-06 00:57:22]

Description was changed from

==========
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 report the failure without actual crashing the program.

Output is written to a new JAR file.

We would like to report assert failure in Canary for a short amount
of time without crashing.
BUG=672945
==========

to

==========
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 report the failure without actual crashing the program.

Output is written to a new JAR file.

We would like to report assert failure in Canary for a short amount
of time without crashing.


BUG=672945
==========

[Ran, 2017-07-06 00:59:14]

ranj@chromium.org changed reviewers:
+ agrieve@chromium.org, yfriedman@chromium.org

[Ran, 2017-07-06 00:59:15]

Hi Andrew, Yaron, PTAL! Thanks!

[agrieve, 2017-07-06 02:49:50]

https://codereview.chromium.org/2971063003/diff/40001/base/android/java/src/o...
File base/android/java/src/org/chromium/base/JavaExceptionReporter.java (right):

https://codereview.chromium.org/2971063003/diff/40001/base/android/java/src/o...
base/android/java/src/org/chromium/base/JavaExceptionReporter.java:63:
StackTraceElement[] stack = Thread.currentThread().getStackTrace();
I think it would be simpler to just call through to
nativeReportJavaException(exception);

The passed in exception would be the AssertionError.

https://codereview.chromium.org/2971063003/diff/40001/build/android/java_asse...
File build/android/java_assertion_handler/BUILD.gn (right):

https://codereview.chromium.org/2971063003/diff/40001/build/android/java_asse...
build/android/java_assertion_handler/BUILD.gn:1: # Copyright 2016 The Chromium
Authors. All rights reserved.
It's 2017 now :)

https://codereview.chromium.org/2971063003/diff/40001/build/android/java_asse...
File
build/android/java_assertion_handler/java/org/chromium/javaassertionhandler/AssertionHandler.java
(right):

https://codereview.chromium.org/2971063003/diff/40001/build/android/java_asse...
build/android/java_assertion_handler/java/org/chromium/javaassertionhandler/AssertionHandler.java:1:
// Copyright 2016 The Chromium Authors. All rights reserved.
I think it would make more sense to build this as a feature of the existing
java_assertion_enabler here:
https://cs.chromium.org/chromium/src/build/android/java_assertion_enabler/

https://codereview.chromium.org/2971063003/diff/40001/build/android/java_asse...
build/android/java_assertion_handler/java/org/chromium/javaassertionhandler/AssertionHandler.java:44:
* We replace new, dup, (ldc), invokespecial, athrow with an instruction that
calls our function.
Rather than stripping the code that creates an AssertionError here, I think it
would make more sense to replace the athrow with invokestatic. That way the
created AssertionError will be passed as a parameter to
JavaExceptionReporter.assertFailureHandler, and it can either throw it (in the
normal case), or silently log it (in the canary case). 

This should have the benefit of being a simpler modification, and also that the
exception will have the correct callstack (as opposed to one that's created from
within the helper function).

I'd guess you'd just need one method that sets a flag from a visitFieldInsn()
for a GETSTATIC of $assertionsDisabled:Z, and then one override to convert the
next athrow.

[Yaron, 2017-07-06 14:20:38]

https://codereview.chromium.org/2971063003/diff/40001/base/android/java/src/o...
File base/android/java/src/org/chromium/base/JavaExceptionReporter.java (right):

https://codereview.chromium.org/2971063003/diff/40001/base/android/java/src/o...
base/android/java/src/org/chromium/base/JavaExceptionReporter.java:63:
StackTraceElement[] stack = Thread.currentThread().getStackTrace();
On 2017/07/06 02:49:50, agrieve wrote:
> I think it would be simpler to just call through to
> nativeReportJavaException(exception);
> 
> The passed in exception would be the AssertionError.

+1

https://codereview.chromium.org/2971063003/diff/40001/build/android/java_asse...
File
build/android/java_assertion_handler/java/org/chromium/javaassertionhandler/AssertionHandler.java
(right):

https://codereview.chromium.org/2971063003/diff/40001/build/android/java_asse...
build/android/java_assertion_handler/java/org/chromium/javaassertionhandler/AssertionHandler.java:47:
class AssertionHandler {
can you add tests for this?

[Ran, 2017-07-06 16:49:51]

Patchset #2 (id:60001) has been deleted

[Ran, 2017-07-06 16:58:16]

Move code to AssertionHandler.java and add some comments.

https://codereview.chromium.org/2971063003/diff/40001/base/android/java/src/o...
File base/android/java/src/org/chromium/base/JavaExceptionReporter.java (right):

https://codereview.chromium.org/2971063003/diff/40001/base/android/java/src/o...
base/android/java/src/org/chromium/base/JavaExceptionReporter.java:63:
StackTraceElement[] stack = Thread.currentThread().getStackTrace();
On 2017/07/06 02:49:50, agrieve wrote:
> I think it would be simpler to just call through to
> nativeReportJavaException(exception);
> 
> The passed in exception would be the AssertionError.

I was thinking to pass an argument to the assertHandler before and I found a
problem doing it.
When we pass a local variable to a function, the bytecode instructions look like
this.
       0: new           #2                  // class java/lang/AssertionError
       3: dup
       4: invokespecial #3                  // Method
java/lang/AssertionError."<init>":()V
       7: astore_1
       8: aload_1
       9: invokestatic  #4                  // Method
packagethree/SomeFunction.handleAssert:(Ljava/lang/AssertionError;)V


The object has to be stored and then loaded from memory (astore_1 and aload_1 do
this job).

However, if astore_1 is already used and the variable is not freed, it will uses
astore_2/aload_2 instead, or astore N/aload N. At this point, I don't know which
N should I use to store. (Maybe we can scan the previous instructions set? but I
guess it will be harder than the current approach).

https://codereview.chromium.org/2971063003/diff/40001/build/android/java_asse...
File build/android/java_assertion_handler/BUILD.gn (right):

https://codereview.chromium.org/2971063003/diff/40001/build/android/java_asse...
build/android/java_assertion_handler/BUILD.gn:1: # Copyright 2016 The Chromium
Authors. All rights reserved.
On 2017/07/06 02:49:50, agrieve wrote:
> It's 2017 now :)

Nice catch :)

https://codereview.chromium.org/2971063003/diff/40001/build/android/java_asse...
File
build/android/java_assertion_handler/java/org/chromium/javaassertionhandler/AssertionHandler.java
(right):

https://codereview.chromium.org/2971063003/diff/40001/build/android/java_asse...
build/android/java_assertion_handler/java/org/chromium/javaassertionhandler/AssertionHandler.java:1:
// Copyright 2016 The Chromium Authors. All rights reserved.
On 2017/07/06 02:49:50, agrieve wrote:
> I think it would make more sense to build this as a feature of the existing
> java_assertion_enabler here:
> https://cs.chromium.org/chromium/src/build/android/java_assertion_enabler/
> 

Done.

https://codereview.chromium.org/2971063003/diff/40001/build/android/java_asse...
build/android/java_assertion_handler/java/org/chromium/javaassertionhandler/AssertionHandler.java:44:
* We replace new, dup, (ldc), invokespecial, athrow with an instruction that
calls our function.
On 2017/07/06 02:49:50, agrieve wrote:
> Rather than stripping the code that creates an AssertionError here, I think it
> would make more sense to replace the athrow with invokestatic. That way the
> created AssertionError will be passed as a parameter to
> JavaExceptionReporter.assertFailureHandler, and it can either throw it (in the
> normal case), or silently log it (in the canary case). 
> 
> This should have the benefit of being a simpler modification, and also that
the
> exception will have the correct callstack (as opposed to one that's created
from
> within the helper function).
> 
> I'd guess you'd just need one method that sets a flag from a visitFieldInsn()
> for a GETSTATIC of $assertionsDisabled:Z, and then one override to convert the
> next athrow.
>  

Added a comment for passing parameters to outside function. Basically we don't
know which registers are available for store at that point.

https://codereview.chromium.org/2971063003/diff/40001/build/android/java_asse...
build/android/java_assertion_handler/java/org/chromium/javaassertionhandler/AssertionHandler.java:47:
class AssertionHandler {
On 2017/07/06 14:20:38, Yaron wrote:
> can you add tests for this?

Didn't really find a good way to write test other than run in local to see if
assert is handled...
However, JVM checks for instruction validation. If any instruction goes wrong or
is unexpected, it will crash in build time.

[agrieve, 2017-07-06 18:55:36]

https://codereview.chromium.org/2971063003/diff/40001/base/android/java/src/o...
File base/android/java/src/org/chromium/base/JavaExceptionReporter.java (right):

https://codereview.chromium.org/2971063003/diff/40001/base/android/java/src/o...
base/android/java/src/org/chromium/base/JavaExceptionReporter.java:63:
StackTraceElement[] stack = Thread.currentThread().getStackTrace();
On 2017/07/06 16:58:16, Ran wrote:
> On 2017/07/06 02:49:50, agrieve wrote:
> > I think it would be simpler to just call through to
> > nativeReportJavaException(exception);
> > 
> > The passed in exception would be the AssertionError.
> 
> I was thinking to pass an argument to the assertHandler before and I found a
> problem doing it.
> When we pass a local variable to a function, the bytecode instructions look
like
> this.
>        0: new           #2                  // class java/lang/AssertionError
>        3: dup
>        4: invokespecial #3                  // Method
> java/lang/AssertionError."<init>":()V
>        7: astore_1
>        8: aload_1
>        9: invokestatic  #4                  // Method
> packagethree/SomeFunction.handleAssert:(Ljava/lang/AssertionError;)V
> 
> 
> The object has to be stored and then loaded from memory (astore_1 and aload_1
do
> this job).
> 
> However, if astore_1 is already used and the variable is not freed, it will
uses
> astore_2/aload_2 instead, or astore N/aload N. At this point, I don't know
which
> N should I use to store. (Maybe we can scan the previous instructions set? but
I
> guess it will be harder than the current approach).
> 
> 

Went through this offline. Looks like you just need the store & load when
assigning the exception to a local variable. Should "just work" to replace the
athrow with an invokestatic

https://codereview.chromium.org/2971063003/diff/80001/build/config/android/in...
File build/config/android/internal_rules.gni (right):

https://codereview.chromium.org/2971063003/diff/80001/build/config/android/in...
build/config/android/internal_rules.gni:2063: "data",
Not sure why this change is showing up in your review. Maybe need to rebase?

[Ran, 2017-07-06 20:24:51]

Patchset #3 (id:90001) has been deleted

[Ran, 2017-07-06 20:46:26]

Patchset #3 (id:110001) has been deleted

[Ran, 2017-07-06 20:50:18]

Thanks for the suggestion. I was looking at the wrong place. It's much simple
and clear now!

https://codereview.chromium.org/2971063003/diff/80001/build/config/android/in...
File build/config/android/internal_rules.gni (right):

https://codereview.chromium.org/2971063003/diff/80001/build/config/android/in...
build/config/android/internal_rules.gni:2063: "data",
On 2017/07/06 18:55:35, agrieve wrote:
> Not sure why this change is showing up in your review. Maybe need to rebase?

Done.

[agrieve, 2017-07-07 17:25:17]

Almost there!

https://codereview.chromium.org/2971063003/diff/130001/base/android/java/src/...
File base/android/java/src/org/chromium/base/JavaExceptionReporter.java (right):

https://codereview.chromium.org/2971063003/diff/130001/base/android/java/src/...
base/android/java/src/org/chromium/base/JavaExceptionReporter.java:58: * Report
and upload the stack trace without actual crash when an assertion fails.
I think we want to do this only when a certain GN arg is set (aka, map it
through BuildConfig.java)

For this first change, it's probably best to have this method just throw the
given exception so that it's a strict refactoring, then add the GN switch in a
follow-up.

https://codereview.chromium.org/2971063003/diff/130001/base/android/java/src/...
base/android/java/src/org/chromium/base/JavaExceptionReporter.java:62: String
stackTrace = assertionError.toString() + "\n";
In the follow-up, I think you can just replace this all with a call to
nativeReportJavaException(false, assertionError)

https://codereview.chromium.org/2971063003/diff/130001/base/android/java/src/...
base/android/java/src/org/chromium/base/JavaExceptionReporter.java:67:
nativeReportJavaStackTrace(stackTrace);
This is a problem with reportStackTrace() as well, but it's possible that the
native library is not loaded yet. Might have to surround this call in a
try/catch for UnsatisfiedLinkError, or call LibraryLoader.ensureInitialized()
beforehand.

https://codereview.chromium.org/2971063003/diff/130001/build/android/java_ass...
File
build/android/java_assertion_enabler/java/org/chromium/javaassertionenabler/AssertionEnabler.java
(left):

https://codereview.chromium.org/2971063003/diff/130001/build/android/java_ass...
build/android/java_assertion_enabler/java/org/chromium/javaassertionenabler/AssertionEnabler.java:52:
// The following bytecode is generated for each class with ASSERT statements:
Don't want to lose this comment since it explains what this transformation does.

https://codereview.chromium.org/2971063003/diff/130001/build/android/java_ass...
File
build/android/java_assertion_enabler/java/org/chromium/javaassertionenabler/AssertionEnabler.java
(right):

https://codereview.chromium.org/2971063003/diff/130001/build/android/java_ass...
build/android/java_assertion_enabler/java/org/chromium/javaassertionenabler/AssertionEnabler.java:41:
* We replace athrow with a call to a function than handles the AssertionError.
than -> that

https://codereview.chromium.org/2971063003/diff/130001/build/android/java_ass...
build/android/java_assertion_enabler/java/org/chromium/javaassertionenabler/AssertionEnabler.java:43:

delete line

https://codereview.chromium.org/2971063003/diff/130001/build/android/java_ass...
build/android/java_assertion_enabler/java/org/chromium/javaassertionenabler/AssertionEnabler.java:86:
static final String ASSERTION_DISABLED_NAME = "$assertionsDisabled";
This already exists in the outer class.

https://codereview.chromium.org/2971063003/diff/130001/build/android/java_ass...
build/android/java_assertion_enabler/java/org/chromium/javaassertionenabler/AssertionEnabler.java:87:
static final int INSERT_INSTRUCTION_CODE = Opcodes.INVOKESTATIC;
nit: I don't think there's much value in creating a constant that holds another
one. Just use Opcodes.INVOKESTATIC below.

https://codereview.chromium.org/2971063003/diff/130001/build/android/java_ass...
build/android/java_assertion_enabler/java/org/chromium/javaassertionenabler/AssertionEnabler.java:173:
boolean isRewriteAssert = false;
Sorry, I know I asked you to make this a flag, but I think actually it's fine
(and simpler) to run this logic unconditionally and put the crash vs no crash
logic into JavaExceptionReporter

[Ran, 2017-07-07 18:16:24]

Just throw AssertionError in assertFailureHandler, report failure and add gn arg
in follow-up cl.

https://codereview.chromium.org/2971063003/diff/130001/base/android/java/src/...
File base/android/java/src/org/chromium/base/JavaExceptionReporter.java (right):

https://codereview.chromium.org/2971063003/diff/130001/base/android/java/src/...
base/android/java/src/org/chromium/base/JavaExceptionReporter.java:58: * Report
and upload the stack trace without actual crash when an assertion fails.
On 2017/07/07 17:25:16, agrieve wrote:
> I think we want to do this only when a certain GN arg is set (aka, map it
> through BuildConfig.java)
> 
> For this first change, it's probably best to have this method just throw the
> given exception so that it's a strict refactoring, then add the GN switch in a
> follow-up.
> 

Acknowledged.

https://codereview.chromium.org/2971063003/diff/130001/base/android/java/src/...
base/android/java/src/org/chromium/base/JavaExceptionReporter.java:62: String
stackTrace = assertionError.toString() + "\n";
On 2017/07/07 17:25:16, agrieve wrote:
> In the follow-up, I think you can just replace this all with a call to
> nativeReportJavaException(false, assertionError)

Acknowledged.

https://codereview.chromium.org/2971063003/diff/130001/base/android/java/src/...
base/android/java/src/org/chromium/base/JavaExceptionReporter.java:67:
nativeReportJavaStackTrace(stackTrace);
On 2017/07/07 17:25:16, agrieve wrote:
> This is a problem with reportStackTrace() as well, but it's possible that the
> native library is not loaded yet. Might have to surround this call in a
> try/catch for UnsatisfiedLinkError, or call LibraryLoader.ensureInitialized()
> beforehand.

Acknowledged.

https://codereview.chromium.org/2971063003/diff/130001/build/android/java_ass...
File
build/android/java_assertion_enabler/java/org/chromium/javaassertionenabler/AssertionEnabler.java
(left):

https://codereview.chromium.org/2971063003/diff/130001/build/android/java_ass...
build/android/java_assertion_enabler/java/org/chromium/javaassertionenabler/AssertionEnabler.java:52:
// The following bytecode is generated for each class with ASSERT statements:
On 2017/07/07 17:25:17, agrieve wrote:
> Don't want to lose this comment since it explains what this transformation
does.

Done.

https://codereview.chromium.org/2971063003/diff/130001/build/android/java_ass...
File
build/android/java_assertion_enabler/java/org/chromium/javaassertionenabler/AssertionEnabler.java
(right):

https://codereview.chromium.org/2971063003/diff/130001/build/android/java_ass...
build/android/java_assertion_enabler/java/org/chromium/javaassertionenabler/AssertionEnabler.java:41:
* We replace athrow with a call to a function than handles the AssertionError.
On 2017/07/07 17:25:16, agrieve wrote:
> than -> that

Done.

https://codereview.chromium.org/2971063003/diff/130001/build/android/java_ass...
build/android/java_assertion_enabler/java/org/chromium/javaassertionenabler/AssertionEnabler.java:43:

On 2017/07/07 17:25:17, agrieve wrote:
> delete line

Done.

https://codereview.chromium.org/2971063003/diff/130001/build/android/java_ass...
build/android/java_assertion_enabler/java/org/chromium/javaassertionenabler/AssertionEnabler.java:86:
static final String ASSERTION_DISABLED_NAME = "$assertionsDisabled";
On 2017/07/07 17:25:17, agrieve wrote:
> This already exists in the outer class.

Done.

https://codereview.chromium.org/2971063003/diff/130001/build/android/java_ass...
build/android/java_assertion_enabler/java/org/chromium/javaassertionenabler/AssertionEnabler.java:87:
static final int INSERT_INSTRUCTION_CODE = Opcodes.INVOKESTATIC;
On 2017/07/07 17:25:16, agrieve wrote:
> nit: I don't think there's much value in creating a constant that holds
another
> one. Just use Opcodes.INVOKESTATIC below.

Done.

https://codereview.chromium.org/2971063003/diff/130001/build/android/java_ass...
build/android/java_assertion_enabler/java/org/chromium/javaassertionenabler/AssertionEnabler.java:173:
boolean isRewriteAssert = false;
On 2017/07/07 17:25:17, agrieve wrote:
> Sorry, I know I asked you to make this a flag, but I think actually it's fine
> (and simpler) to run this logic unconditionally and put the crash vs no crash
> logic into JavaExceptionReporter

Done.

[agrieve, 2017-07-07 18:42:42]

lgtm. Please first update the commit description.

Note: there is some risk of this breaking java targets if any exist which do not
have base_java included in them. Not sure how to check this though, so we'll
have to hope for the best.

https://codereview.chromium.org/2971063003/diff/150001/base/android/java/src/...
File base/android/java/src/org/chromium/base/JavaExceptionReporter.java (right):

https://codereview.chromium.org/2971063003/diff/150001/base/android/java/src/...
base/android/java/src/org/chromium/base/JavaExceptionReporter.java:59: *
nit: delete blank line

[Ran, 2017-07-07 19:06:20]

Description was changed from

==========
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 report the failure without actual crashing the program.

Output is written to a new JAR file.

We would like to report assert failure in Canary for a short amount
of time without crashing.


BUG=672945
==========

to

==========
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 throw Exception or report the failure without actual crashing the program.

Output is written to a new JAR file.

We would like to report assert failure in Canary for a short amount
of time without crashing.


BUG=672945
==========

[Ran, 2017-07-07 19:12:00]

Description was changed from

==========
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 throw Exception or report the failure without actual crashing the program.

Output is written to a new JAR file.

We would like to report assert failure in Canary for a short amount
of time without crashing.


BUG=672945
==========

to

==========
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
==========

[Yaron, 2017-07-07 21:03:45]

lgtm

https://codereview.chromium.org/2971063003/diff/150001/build/android/java_ass...
File build/android/java_assertion_enabler/OWNERS (right):

https://codereview.chromium.org/2971063003/diff/150001/build/android/java_ass...
build/android/java_assertion_enabler/OWNERS:3: ranj@chromium.org
are you a committer? (nit: also sort)

[Ran, 2017-07-08 00:05:31]

The CQ bit was checked by ranj@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-07-08 00:05:52]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-07-08 01:24:49]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-07-08 01:24:50]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Ran, 2017-07-10 16:07:39]

The CQ bit was checked by ranj@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-07-10 16:07:54]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-07-10 17:12:41]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-07-10 17:12:43]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Ran, 2017-07-10 20:17:09]

The CQ bit was checked by ranj@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-07-10 20:17:27]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Ran, 2017-07-10 20:23:10]

The CQ bit was checked by ranj@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-07-10 20:23:27]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Ran, 2017-07-10 20:23:51]

Patchset #8 (id:230001) has been deleted

[commit-bot: I haz the power, 2017-07-10 23:11:12]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-07-10 23:11:13]

Dry run: Try jobs failed on following builders:
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)

[Ran, 2017-07-17 17:52:36]

Patchset #5 (id:170001) has been deleted

[Ran, 2017-07-17 17:52:46]

Patchset #5 (id:190001) has been deleted

[Ran, 2017-07-17 17:52:54]

Patchset #5 (id:210001) has been deleted

[Ran, 2017-07-17 17:53:04]

Patchset #5 (id:250001) has been deleted

[Ran, 2017-07-17 17:53:15]

Patchset #5 (id:270001) has been deleted

[Ran, 2017-07-17 18:07:52]

Patchset #5 (id:290001) has been deleted

[Ran, 2017-07-17 18:08:16]

The CQ bit was checked by ranj@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-07-17 18:08:24]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Ran, 2017-07-17 18:13:47]

The CQ bit was checked by ranj@chromium.org to run a CQ dry run

[Ran, 2017-07-17 18:14:04]

Patchset #5 (id:310001) has been deleted

[commit-bot: I haz the power, 2017-07-17 18:14:05]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-07-17 19:20:31]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-07-17 19:20:32]

Dry run: This issue passed the CQ dry run.

[Ran, 2017-07-17 21:46:51]

ranj@chromium.org changed reviewers:
+ zpeng@chromium.org

[Ran, 2017-07-17 21:46:52]

Thanks's for Andrew & Felix's suggestion, goto the instruction that will be
called as if assertion is disable after calling each handler function. This
seems to fix the problem.

https://codereview.chromium.org/2971063003/diff/150001/base/android/java/src/...
File base/android/java/src/org/chromium/base/JavaExceptionReporter.java (right):

https://codereview.chromium.org/2971063003/diff/150001/base/android/java/src/...
base/android/java/src/org/chromium/base/JavaExceptionReporter.java:59: *
On 2017/07/07 18:42:42, agrieve wrote:
> nit: delete blank line

Done.

https://codereview.chromium.org/2971063003/diff/150001/build/android/java_ass...
File build/android/java_assertion_enabler/OWNERS (right):

https://codereview.chromium.org/2971063003/diff/150001/build/android/java_ass...
build/android/java_assertion_enabler/OWNERS:3: ranj@chromium.org
On 2017/07/07 21:03:45, Yaron wrote:
> are you a committer? (nit: also sort)

Not yet :(

[agrieve, 2017-07-18 00:16:51]

opcode changes look good.

https://codereview.chromium.org/2971063003/diff/330001/build/android/java_ass...
File
build/android/java_assertion_enabler/java/org/chromium/javaassertionenabler/AssertionEnabler.java
(right):

https://codereview.chromium.org/2971063003/diff/330001/build/android/java_ass...
build/android/java_assertion_enabler/java/org/chromium/javaassertionenabler/AssertionEnabler.java:47:
* The followed instructions are generated by a java assert statement:
nit: can you update this comment with mention of the goto statement that it
added?

Also - it's not super clear that the "First enable assert" part is code that's
within a class' static initializer method, and that these instructions are
per-assert. Might be good to make that a bit more explicit.

https://codereview.chromium.org/2971063003/diff/330001/build/android/java_ass...
build/android/java_assertion_enabler/java/org/chromium/javaassertionenabler/AssertionEnabler.java:63:
static final String[] EXCLUDED_JAR_NAME = {"webapk"}; // Jars that do not depend
on base/java
This might be pretty hard to find here. Probably better to pass this through as
a parameter. If you don't want to bother with writing an argument parser, you
can pass args using Java's property system, or through an environment variable.

https://codereview.chromium.org/2971063003/diff/330001/build/android/java_ass...
build/android/java_assertion_enabler/java/org/chromium/javaassertionenabler/AssertionEnabler.java:195:
return;
Looks like you're disabling asserts entirely for webapk. Probably still want to
enable them, but not transform to call the method in base.

[Ran, 2017-07-18 01:35:54]

The CQ bit was checked by ranj@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-07-18 01:36:06]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-07-18 03:55:11]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-07-18 03:55:12]

Dry run: This issue passed the CQ dry run.

[Ran, 2017-07-18 04:00:25]

Fix issues.

https://codereview.chromium.org/2971063003/diff/330001/build/android/java_ass...
File
build/android/java_assertion_enabler/java/org/chromium/javaassertionenabler/AssertionEnabler.java
(right):

https://codereview.chromium.org/2971063003/diff/330001/build/android/java_ass...
build/android/java_assertion_enabler/java/org/chromium/javaassertionenabler/AssertionEnabler.java:47:
* The followed instructions are generated by a java assert statement:
On 2017/07/18 00:16:50, agrieve wrote:
> nit: can you update this comment with mention of the goto statement that it
> added?
> 
> Also - it's not super clear that the "First enable assert" part is code that's
> within a class' static initializer method, and that these instructions are
> per-assert. Might be good to make that a bit more explicit.

Done.

https://codereview.chromium.org/2971063003/diff/330001/build/android/java_ass...
build/android/java_assertion_enabler/java/org/chromium/javaassertionenabler/AssertionEnabler.java:63:
static final String[] EXCLUDED_JAR_NAME = {"webapk"}; // Jars that do not depend
on base/java
On 2017/07/18 00:16:50, agrieve wrote:
> This might be pretty hard to find here. Probably better to pass this through
as
> a parameter. If you don't want to bother with writing an argument parser, you
> can pass args using Java's property system, or through an environment
variable.

Yaron and Peter are OK with removing assert in Webapk (there are just three
assertions). I just removed them along with this check.

https://codereview.chromium.org/2971063003/diff/330001/build/android/java_ass...
build/android/java_assertion_enabler/java/org/chromium/javaassertionenabler/AssertionEnabler.java:195:
return;
On 2017/07/18 00:16:51, agrieve wrote:
> Looks like you're disabling asserts entirely for webapk. Probably still want
to
> enable them, but not transform to call the method in base.

Same as above. Just too trivial to complex the code.

[Yaron, 2017-07-18 19:53:19]

not lgtm per issue that was exposed and subsequent updates. I'd be happy to be
convinced this is safe but I currently have concerns

https://codereview.chromium.org/2971063003/diff/150001/build/android/java_ass...
File build/android/java_assertion_enabler/OWNERS (right):

https://codereview.chromium.org/2971063003/diff/150001/build/android/java_ass...
build/android/java_assertion_enabler/OWNERS:3: ranj@chromium.org
On 2017/07/17 21:46:52, Ran wrote:
> On 2017/07/07 21:03:45, Yaron wrote:
> > are you a committer? (nit: also sort)
> 
> Not yet :(

so unfortunately you have to remove yourself and ask Andrew or Felix for reviews

https://codereview.chromium.org/2971063003/diff/370001/base/android/java/src/...
File
base/android/java/src/org/chromium/base/process_launcher/ChildProcessLauncher.java
(left):

https://codereview.chromium.org/2971063003/diff/370001/base/android/java/src/...
base/android/java/src/org/chromium/base/process_launcher/ChildProcessLauncher.java:227:
assert mConnection != null;
why are you removing non-webapk ones?

https://codereview.chromium.org/2971063003/diff/370001/build/android/java_ass...
File
build/android/java_assertion_enabler/java/org/chromium/javaassertionenabler/AssertionEnabler.java
(right):

https://codereview.chromium.org/2971063003/diff/370001/build/android/java_ass...
build/android/java_assertion_enabler/java/org/chromium/javaassertionenabler/AssertionEnabler.java:112:
mGotoLabel = label;
Is it possible this opcode could be there for other reasons? I imagine a
different sequence of code could result in other conditionals with other jump
labels that confuse this.

Not knowing how this code really works, I have no confidence that it's the case
so would appreciate it if you can convince me of that. Like is
mStartLoadingAssert being true mean that we just read the assert so we know taht
if assertions are disabled we'd skip this section and that's why the label is
ok? However, you don't unset mStartLoadingAssert until we've done the jump so
what if there was conditional logic in between and mStartLoadingAssert is still
true and you update the label to something new? Obviously in that case you could
check whether mGotoLabel is still null before assigning it, but either way, I
need more convincing that this is safe. Unit tests could be one approach but
it's also hard to exhaustively test this ;)

[Ran, 2017-07-18 21:43:11]

Fix.

https://codereview.chromium.org/2971063003/diff/150001/build/android/java_ass...
File build/android/java_assertion_enabler/OWNERS (right):

https://codereview.chromium.org/2971063003/diff/150001/build/android/java_ass...
build/android/java_assertion_enabler/OWNERS:3: ranj@chromium.org
On 2017/07/18 19:53:18, Yaron wrote:
> On 2017/07/17 21:46:52, Ran wrote:
> > On 2017/07/07 21:03:45, Yaron wrote:
> > > are you a committer? (nit: also sort)
> > 
> > Not yet :(
> 
> so unfortunately you have to remove yourself and ask Andrew or Felix for
reviews

Done.

https://codereview.chromium.org/2971063003/diff/370001/base/android/java/src/...
File
base/android/java/src/org/chromium/base/process_launcher/ChildProcessLauncher.java
(left):

https://codereview.chromium.org/2971063003/diff/370001/base/android/java/src/...
base/android/java/src/org/chromium/base/process_launcher/ChildProcessLauncher.java:227:
assert mConnection != null;
On 2017/07/18 19:53:18, Yaron wrote:
> why are you removing non-webapk ones?

The compiler gives error saying redundant null check. When I look at the code,
it's guaranteed to be not null.

https://codereview.chromium.org/2971063003/diff/370001/build/android/java_ass...
File
build/android/java_assertion_enabler/java/org/chromium/javaassertionenabler/AssertionEnabler.java
(right):

https://codereview.chromium.org/2971063003/diff/370001/build/android/java_ass...
build/android/java_assertion_enabler/java/org/chromium/javaassertionenabler/AssertionEnabler.java:112:
mGotoLabel = label;
On 2017/07/18 19:53:18, Yaron wrote:
> Is it possible this opcode could be there for other reasons? I imagine a
> different sequence of code could result in other conditionals with other jump
> labels that confuse this.
> 
> Not knowing how this code really works, I have no confidence that it's the
case
> so would appreciate it if you can convince me of that. Like is
> mStartLoadingAssert being true mean that we just read the assert so we know
taht
> if assertions are disabled we'd skip this section and that's why the label is
> ok? However, you don't unset mStartLoadingAssert until we've done the jump so
> what if there was conditional logic in between and mStartLoadingAssert is
still
> true and you update the label to something new? Obviously in that case you
could
> check whether mGotoLabel is still null before assigning it, but either way, I
> need more convincing that this is safe. Unit tests could be one approach but
> it's also hard to exhaustively test this ;)

Done. Discussed offline.

[Yaron, 2017-07-18 21:45:59]

lgtm

[Ran, 2017-07-18 21:47:06]

The CQ bit was checked by ranj@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-07-18 21:47:22]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-07-18 22:46:53]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-07-18 22:46:54]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[agrieve, 2017-07-19 01:45:10]

The CQ bit was checked by agrieve@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-07-19 01:45:32]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-07-19 02:44:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-07-19 02:44:56]

Dry run: This issue passed the CQ dry run.

[Ran, 2017-07-19 17:25:19]

The CQ bit was checked by ranj@chromium.org

[Ran, 2017-07-19 17:25:20]

The patchset sent to the CQ was uploaded after l-g-t-m from agrieve@chromium.org
Link to the patchset: https://codereview.chromium.org/2971063003/#ps390001
(title: "fix goto")

[commit-bot: I haz the power, 2017-07-19 17:25:32]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-07-19 17:31:26]

CQ is committing da patch.

Bot data: {"patchset_id": 390001, "attempt_start_ts": 1500485119309610,
"parent_rev": "1667a8fb7c2697a4e394338d029f0a311c85a00d", "commit_rev":
"578183053d672cf3502045e687844855ad29fcd8"}

[commit-bot: I haz the power, 2017-07-19 17:31:46]

CQ is committing da patch.

Bot data: {"patchset_id": 390001, "attempt_start_ts": 1500485119309610,
"parent_rev": "cb63401fd11ea382833da3fe016b9b989937cc57", "commit_rev":
"33133e5767a9c1ca1f7e9defad2d7b3347065589"}

[commit-bot: I haz the power, 2017-07-19 17:32:03]

Description was changed from

==========
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
==========

to

==========
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/+/33133e5767a9c1ca1f7e9defad2d...
==========

[commit-bot: I haz the power, 2017-07-19 17:32:05]

Committed patchset #8 (id:390001) as
https://chromium.googlesource.com/chromium/src/+/33133e5767a9c1ca1f7e9defad2d...

[mgersh, 2017-07-19 17:57:07]

This CL caused a Cronet bot
(https://build.chromium.org/p/chromium.android/builders/Android%20Cronet%20x86...)
to start failing with:

Warning: org.chromium.net.impl.UrlRequestBase: can't find referenced class
org.chromium.base.JavaExceptionReporter
Warning: org.chromium.net.impl.UrlRequestBase: can't find referenced class
org.chromium.base.JavaExceptionReporter
Warning: there were 2 unresolved references to classes or interfaces.
         You may need to add missing library jars or update their versions.
         If your code works fine without the missing classes, you can suppress
         the warnings with '-dontwarn' options.
        
(http://proguard.sourceforge.net/manual/troubleshooting.html#unresolvedclass)
Error: Please correct the above warnings first.

[mgersh, 2017-07-19 18:07:04]

A revert of this CL (patchset #8 id:390001) has been created in
https://codereview.chromium.org/2981273003/ by mgersh@chromium.org.

The reason for reverting is: 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....

[Ran, 2017-07-19 21:50:45]

Description was changed from

==========
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/+/33133e5767a9c1ca1f7e9defad2d...
==========

to

==========
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/+/33133e5767a9c1ca1f7e9defad2d...
==========

[Ran, 2017-07-19 21:51:07]

The CQ bit was checked by ranj@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-07-19 21:51:24]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-07-19 22:09:40]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-07-19 22:09:41]

Dry run: Try jobs failed on following builders:
  android_cronet on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...)

[Ran, 2017-07-19 22:40:21]

The CQ bit was checked by ranj@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-07-19 22:40:41]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-07-20 00:16:53]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-07-20 00:16:55]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[Ran, 2017-07-20 14:51:28]

Patchset #9 (id:410001) has been deleted

[Ran, 2017-07-20 18:42:43]

The CQ bit was checked by ranj@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-07-20 18:43:12]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-07-20 20:08:43]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-07-20 20:08:45]

Dry run: This issue passed the CQ dry run.

[Ran, 2017-07-20 20:09:02]

The CQ bit was checked by ranj@chromium.org

[Ran, 2017-07-20 20:09:02]

The patchset sent to the CQ was uploaded after l-g-t-m from
yfriedman@chromium.org, agrieve@chromium.org
Link to the patchset: https://codereview.chromium.org/2971063003/#ps470001
(title: "fix tests")

[commit-bot: I haz the power, 2017-07-20 20:09:25]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Ran, 2017-07-20 20:09:29]

The CQ bit was unchecked by ranj@chromium.org

[Ran, 2017-07-20 20:10:05]

The CQ bit was checked by ranj@chromium.org

[commit-bot: I haz the power, 2017-07-20 20:10:25]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-07-20 20:20:54]

CQ is committing da patch.

Bot data: {"patchset_id": 470001, "attempt_start_ts": 1500581405272510,
"parent_rev": "94fc8d933c7a1c6d3414a752615c93bd3df8ec8a", "commit_rev":
"bef3daf02ae6035f166782025fd7e4e54d465271"}

[commit-bot: I haz the power, 2017-07-20 20:21:32]

Description was changed from

==========
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/+/33133e5767a9c1ca1f7e9defad2d...
==========

to

==========
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/+/33133e5767a9c1ca1f7e9defad2d...
Review-Url: https://codereview.chromium.org/2971063003
Cr-Commit-Position: refs/heads/master@{#488381}
Committed:
https://chromium.googlesource.com/chromium/src/+/bef3daf02ae6035f166782025fd7...
==========

[commit-bot: I haz the power, 2017-07-20 20:21:34]

Committed patchset #11 (id:470001) as
https://chromium.googlesource.com/chromium/src/+/bef3daf02ae6035f166782025fd7...

[estevenson, 2017-07-20 20:35:18]

A revert of this CL (patchset #11 id:470001) has been created in
https://codereview.chromium.org/2985613002/ by estevenson@chromium.org.

The reason for reverting is: Reland changes weren't reviewed.

[sgurun-gerrit only, 2017-07-20 20:57:38]

On 2017/07/20 20:35:18, estevenson wrote:
> A revert of this CL (patchset #11 id:470001) has been created in
> https://codereview.chromium.org/2985613002/ by mailto:estevenson@chromium.org.
> 
> The reason for reverting is: Reland changes weren't reviewed.

 This broke cronet builder. Next time relanding can you please test cronet, as
this is the second break of cronet by this CL.

Automatically closing tree for "Failure reason,compile,steps" on "Android Cronet
x86 Builder (dbg)"

https://build.chromium.org/p/chromium.android/builders/Android%20Cronet%20x86...


sage -printmapping
gen/components/cronet/android/cronet_smoketests_missing_native_library_apk/cronet_smoketests_missing_native_library_apk.proguard.jar.mapping
)
Warning: org.chromium.net.impl.UrlRequestBase: can't find referenced class
org.chromium.buildhooks.BuildHooks

[Ran, 2017-07-20 21:07:27]

Description was changed from

==========
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/+/33133e5767a9c1ca1f7e9defad2d...
Review-Url: https://codereview.chromium.org/2971063003
Cr-Commit-Position: refs/heads/master@{#488381}
Committed:
https://chromium.googlesource.com/chromium/src/+/bef3daf02ae6035f166782025fd7...
==========

to

==========
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/+/33133e5767a9c1ca1f7e9defad2d...
Review-Url: https://codereview.chromium.org/2971063003
Cr-Commit-Position: refs/heads/master@{#488381}
Committed:
https://chromium.googlesource.com/chromium/src/+/bef3daf02ae6035f166782025fd7...
==========