Upstream: DexOptimizer and DexLoader classes

Upstream: DexOptimizer and DexLoader classes

This CL upstreams DexOptimizer and DexLoader
DexOptimizer extracts a .dex file from an APK
DexLoader creates a ClassLoader from the extracted .dex file

BUG=609122
R=hanxi
TBR=yfriedman (TBR for new DEPS in chrome/android/webapk/shell_apk/javatests)

Committed: https://crrev.com/f1bda2947031793ca8a08ecec4a88fe27d9b14e5
Cr-Commit-Position: refs/heads/master@{#394656}

[pkotwicz, 2016-05-13 18:20:56]

Patchset #1 (id:1) has been deleted

[pkotwicz, 2016-05-13 18:23:43]

pkotwicz@chromium.org changed reviewers:
+ hanxi@chromium.org

[pkotwicz, 2016-05-13 18:23:43]

Xi, can you please take a look?

This CL unfortunately also depends on
https://codereview.chromium.org/1971773002/

[Xi Han, 2016-05-16 18:44:31]

lgtm

[pkotwicz, 2016-05-16 19:12:50]

pkotwicz@chromium.org changed reviewers:
+ yfriedman@chromium.org

[pkotwicz, 2016-05-16 19:12:51]

Yaron, can you please take a look at the changes in chrome/android/BUILD.gn ?

[Yaron, 2016-05-16 20:39:57]

yfriedman@chromium.org changed reviewers:
+ jbudorick@chromium.org

[Yaron, 2016-05-16 20:39:58]

https://codereview.chromium.org/1981473002/diff/20001/chrome/android/BUILD.gn
File chrome/android/BUILD.gn (right):

https://codereview.chromium.org/1981473002/diff/20001/chrome/android/BUILD.gn...
chrome/android/BUILD.gn:553:
"//chrome/android/webapk/shell_apk:shell_apk_javatests",
This is sufficient for the tests to get picked up and run, right?

https://codereview.chromium.org/1981473002/diff/20001/chrome/android/BUILD.gn...
chrome/android/BUILD.gn:556:
"//chrome/android/webapk/shell_apk/javatests/dex_optimizer:dex_optimizer_apk",
I think this is fine and I think we discussed with the approach with John but
+jbudorick in case he has concerns. We could split out shell_apk_javatests as a
separate test target on the waterfall if needed

https://codereview.chromium.org/1981473002/diff/20001/chrome/android/webapk/l...
File
chrome/android/webapk/libs/client/src/org/chromium/webapk/lib/client/DexOptimizer.java
(right):

https://codereview.chromium.org/1981473002/diff/20001/chrome/android/webapk/l...
chrome/android/webapk/libs/client/src/org/chromium/webapk/lib/client/DexOptimizer.java:8:
import android.util.Log;
do you have a CL to disable the presubmit for this folder? Possibly I just
wasn't included? Would prefer we don't bypass those checks

https://codereview.chromium.org/1981473002/diff/20001/chrome/android/webapk/l...
chrome/android/webapk/libs/client/src/org/chromium/webapk/lib/client/DexOptimizer.java:168:
private static class VMRuntime {
Please check if there's an existing feature request and if not, we should file
one

https://codereview.chromium.org/1981473002/diff/20001/chrome/android/webapk/s...
File chrome/android/webapk/shell_apk/javatests/canary_lib/BUILD.gn (right):

https://codereview.chromium.org/1981473002/diff/20001/chrome/android/webapk/s...
chrome/android/webapk/shell_apk/javatests/canary_lib/BUILD.gn:7:
android_library("canary_lib") {
Can you add a brief comment about what this is for.

[jbudorick, 2016-05-16 20:57:41]

https://codereview.chromium.org/1981473002/diff/20001/chrome/android/BUILD.gn
File chrome/android/BUILD.gn (right):

https://codereview.chromium.org/1981473002/diff/20001/chrome/android/BUILD.gn...
chrome/android/BUILD.gn:553:
"//chrome/android/webapk/shell_apk:shell_apk_javatests",
On 2016/05/16 20:39:58, Yaron wrote:
> This is sufficient for the tests to get picked up and run, right?

yep

https://codereview.chromium.org/1981473002/diff/20001/chrome/android/BUILD.gn...
chrome/android/BUILD.gn:556:
"//chrome/android/webapk/shell_apk/javatests/dex_optimizer:dex_optimizer_apk",
On 2016/05/16 20:39:58, Yaron wrote:
> I think this is fine and I think we discussed with the approach with John but
> +jbudorick in case he has concerns. We could split out shell_apk_javatests as
a
> separate test target on the waterfall if needed

Yeah, I think it's fine for us to use chrome_public_test_apk as the default
target for instrumentation tests in //chrome/android. (sync_shell deviates
because it needs a different apk_under_test for some reason.)

[pkotwicz, 2016-05-17 01:34:33]

Yaron, can you please take another looK? 👓

https://codereview.chromium.org/1981473002/diff/20001/chrome/android/webapk/l...
File
chrome/android/webapk/libs/client/src/org/chromium/webapk/lib/client/DexOptimizer.java
(right):

https://codereview.chromium.org/1981473002/diff/20001/chrome/android/webapk/l...
chrome/android/webapk/libs/client/src/org/chromium/webapk/lib/client/DexOptimizer.java:8:
import android.util.Log;
Yep: https://codereview.chromium.org/1980693002/

https://codereview.chromium.org/1981473002/diff/20001/chrome/android/webapk/l...
chrome/android/webapk/libs/client/src/org/chromium/webapk/lib/client/DexOptimizer.java:168:
private static class VMRuntime {
Will do

[Yaron, 2016-05-17 13:38:46]

lgtm

https://codereview.chromium.org/1981473002/diff/20001/chrome/android/webapk/l...
File
chrome/android/webapk/libs/client/src/org/chromium/webapk/lib/client/DexOptimizer.java
(right):

https://codereview.chromium.org/1981473002/diff/20001/chrome/android/webapk/l...
chrome/android/webapk/libs/client/src/org/chromium/webapk/lib/client/DexOptimizer.java:8:
import android.util.Log;
On 2016/05/17 01:34:32, pkotwicz wrote:
> Yep: https://codereview.chromium.org/1980693002/

nice!

[pkotwicz, 2016-05-17 14:46:36]

The CQ bit was checked by pkotwicz@chromium.org

[pkotwicz, 2016-05-17 14:46:37]

The patchset sent to the CQ was uploaded after l-g-t-m from hanxi@chromium.org
Link to the patchset: https://codereview.chromium.org/1981473002/#ps40001
(title: " ")

[commit-bot: I haz the power, 2016-05-17 14:46:57]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1981473002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1981473002/40001

[commit-bot: I haz the power, 2016-05-17 15:47:18]

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

[commit-bot: I haz the power, 2016-05-17 15:47:19]

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

[pkotwicz, 2016-05-17 18:53:56]

Description was changed from

==========
Upstream: DexOptimizer and DexLoader classes

This CL upstreams DexOptimizer and DexLoader
DexOptimizer extracts a .dex file from an APK
DexLoader creates a ClassLoader from the extracted .dex file

BUG=609122
==========

to

==========
Upstream: DexOptimizer and DexLoader classes

This CL upstreams DexOptimizer and DexLoader
DexOptimizer extracts a .dex file from an APK
DexLoader creates a ClassLoader from the extracted .dex file

BUG=609122
R=hanxi
TBR=yfriedman (TBR for new DEPS in chrome/android/webapk/shell_apk/javatests)
==========

[pkotwicz, 2016-05-17 18:54:01]

The CQ bit was checked by pkotwicz@chromium.org

[pkotwicz, 2016-05-17 18:54:02]

The patchset sent to the CQ was uploaded after l-g-t-m from hanxi@chromium.org,
yfriedman@chromium.org
Link to the patchset: https://codereview.chromium.org/1981473002/#ps60001
(title: " ")

[commit-bot: I haz the power, 2016-05-17 18:54:35]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1981473002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1981473002/60001

[commit-bot: I haz the power, 2016-05-17 19:41:16]

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

[commit-bot: I haz the power, 2016-05-17 19:41:17]

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

[pkotwicz, 2016-05-17 22:08:33]

Yaron, how do you suggest addressing this issue:

FindBugs reported the following issues:
DP_CREATE_CLASSLOADER_INSIDE_DO_PRIVILEGED: Classloaders should only be created
inside doPrivileged block
  In class org.chromium.webapk.lib.client.DexOptimizer
  In method org.chromium.webapk.lib.client.DexOptimizer.optimize(File)
  In class dalvik.system.DexClassLoader
  At DexOptimizer.java:[line 64]

[Xi Han, 2016-05-18 13:41:31]

On 2016/05/17 22:08:33, pkotwicz wrote:
> Yaron, how do you suggest addressing this issue:
> 
> FindBugs reported the following issues:
> DP_CREATE_CLASSLOADER_INSIDE_DO_PRIVILEGED: Classloaders should only be
created
> inside doPrivileged block
>   In class org.chromium.webapk.lib.client.DexOptimizer
>   In method org.chromium.webapk.lib.client.DexOptimizer.optimize(File)
>   In class dalvik.system.DexClassLoader
>   At DexOptimizer.java:[line 64]

Will add a flag
"@SuppressFBWarnings("DP_CREATE_CLASSLOADER_INSIDE_DO_PRIVILEGED")" help?
See
https://code.google.com/p/chromium/codesearch#chromium/src/android_webview/ja....

[Yaron, 2016-05-18 14:11:55]

Ya, just add the suppression for now

On Wednesday, May 18, 2016, <hanxi@chromium.org> wrote:

> On 2016/05/17 22:08:33, pkotwicz wrote:
> > Yaron, how do you suggest addressing this issue:
> >
> > FindBugs reported the following issues:
> > DP_CREATE_CLASSLOADER_INSIDE_DO_PRIVILEGED: Classloaders should only be
> created
> > inside doPrivileged block
> > In class org.chromium.webapk.lib.client.DexOptimizer
> > In method org.chromium.webapk.lib.client.DexOptimizer.optimize(File)
> > In class dalvik.system.DexClassLoader
> > At DexOptimizer.java:[line 64]
>
> Will add a flag
> "@SuppressFBWarnings("DP_CREATE_CLASSLOADER_INSIDE_DO_PRIVILEGED")" help?
> See
>
>
https://code.google.com/p/chromium/codesearch#chromium/src/android_webview/ja...
> .
>
>
> https://codereview.chromium.org/1981473002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Yaron, 2016-05-18 14:29:31]

On 2016/05/18 14:11:55, Yaron wrote:
> Ya, just add the suppression for now
> 
> On Wednesday, May 18, 2016, <mailto:hanxi@chromium.org> wrote:
> 
> > On 2016/05/17 22:08:33, pkotwicz wrote:
> > > Yaron, how do you suggest addressing this issue:
> > >
> > > FindBugs reported the following issues:
> > > DP_CREATE_CLASSLOADER_INSIDE_DO_PRIVILEGED: Classloaders should only be
> > created
> > > inside doPrivileged block
> > > In class org.chromium.webapk.lib.client.DexOptimizer
> > > In method org.chromium.webapk.lib.client.DexOptimizer.optimize(File)
> > > In class dalvik.system.DexClassLoader
> > > At DexOptimizer.java:[line 64]
> >
> > Will add a flag
> > "@SuppressFBWarnings("DP_CREATE_CLASSLOADER_INSIDE_DO_PRIVILEGED")" help?
> > See
> >
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/android_webview/ja...
> > .
> >
> >
> > https://codereview.chromium.org/1981473002/
> >
> 
> -- 
> You received this message because you are subscribed to the Google Groups
> "Chromium-reviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

Android docs indicate that it shouldn't be used:
https://developer.android.com/reference/java/security/AccessController.html
I would either whitelist this as Xi suggested or preferably just disable the
findbug error entirely since it's not appropriate. Something like
<Bug pattern="DP_CREATE_CLASSLOADER_INSIDE_DO_PRIVILEGED" />
in build/android/findbugs_filter/findbugs_exclude.xml

[pkotwicz, 2016-05-18 23:41:02]

Patchset #3 (id:60001) has been deleted

[pkotwicz, 2016-05-18 23:44:33]

The CQ bit was checked by pkotwicz@chromium.org

[pkotwicz, 2016-05-18 23:44:34]

The patchset sent to the CQ was uploaded after l-g-t-m from hanxi@chromium.org,
yfriedman@chromium.org
Link to the patchset: https://codereview.chromium.org/1981473002/#ps80001
(title: " ")

[commit-bot: I haz the power, 2016-05-18 23:45:19]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1981473002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1981473002/80001

[commit-bot: I haz the power, 2016-05-19 00:59:58]

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

[commit-bot: I haz the power, 2016-05-19 00:59:59]

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

[pkotwicz, 2016-05-19 01:33:55]

The CQ bit was checked by pkotwicz@chromium.org

[pkotwicz, 2016-05-19 01:33:56]

The patchset sent to the CQ was uploaded after l-g-t-m from hanxi@chromium.org,
yfriedman@chromium.org
Link to the patchset: https://codereview.chromium.org/1981473002/#ps120001
(title: " ")

[commit-bot: I haz the power, 2016-05-19 01:34:33]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1981473002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1981473002/120001

[commit-bot: I haz the power, 2016-05-19 03:38:50]

Description was changed from

==========
Upstream: DexOptimizer and DexLoader classes

This CL upstreams DexOptimizer and DexLoader
DexOptimizer extracts a .dex file from an APK
DexLoader creates a ClassLoader from the extracted .dex file

BUG=609122
R=hanxi
TBR=yfriedman (TBR for new DEPS in chrome/android/webapk/shell_apk/javatests)
==========

to

==========
Upstream: DexOptimizer and DexLoader classes

This CL upstreams DexOptimizer and DexLoader
DexOptimizer extracts a .dex file from an APK
DexLoader creates a ClassLoader from the extracted .dex file

BUG=609122
R=hanxi
TBR=yfriedman (TBR for new DEPS in chrome/android/webapk/shell_apk/javatests)
==========

[commit-bot: I haz the power, 2016-05-19 03:38:51]

Committed patchset #5 (id:120001)

[commit-bot: I haz the power, 2016-05-19 03:40:34]

Description was changed from

==========
Upstream: DexOptimizer and DexLoader classes

This CL upstreams DexOptimizer and DexLoader
DexOptimizer extracts a .dex file from an APK
DexLoader creates a ClassLoader from the extracted .dex file

BUG=609122
R=hanxi
TBR=yfriedman (TBR for new DEPS in chrome/android/webapk/shell_apk/javatests)
==========

to

==========
Upstream: DexOptimizer and DexLoader classes

This CL upstreams DexOptimizer and DexLoader
DexOptimizer extracts a .dex file from an APK
DexLoader creates a ClassLoader from the extracted .dex file

BUG=609122
R=hanxi
TBR=yfriedman (TBR for new DEPS in chrome/android/webapk/shell_apk/javatests)

Committed: https://crrev.com/f1bda2947031793ca8a08ecec4a88fe27d9b14e5
Cr-Commit-Position: refs/heads/master@{#394656}
==========

[commit-bot: I haz the power, 2016-05-19 03:40:35]

Patchset 5 (id:??) landed as
https://crrev.com/f1bda2947031793ca8a08ecec4a88fe27d9b14e5
Cr-Commit-Position: refs/heads/master@{#394656}