@MainDex required for ContentViewStatics when it shouldn't be |
||
Issue description
For context of how this was discovered, see bug #643327
Some initial code searching make me think that the JNI methods for the class are being registered on the native side when they shouldn't be. I don't see any actual usages of the class from the renderer.
I think the offending JNI path is:
chrome/browser/android/chrome_entry_point.cc:
JNI_OnLoad() calls chrome::android::OnJNIOnLoadRegisterJNI(),
which calls content::android::OnJNIOnLoadRegisterJNI(),
which calls content::EnsureJniRegistered()
which calls content::android::RegisterBrowserJni()
Note that chrome::android::OnJNIOnLoadRegisterJNI()'s registration logic includes:
if (base::android::GetLibraryProcessType(env) ==
base::android::PROCESS_BROWSER) {
return RegisterBrowserJNI(env);
}
Seems likely that content should add the same guard for its browser classes.
,
Sep 2 2016
Seems reasonable. I've never really been sure in which cases we do JNI calls from the renderer, but if we have clearly delimited areas where the JNI classes are for browser-only paths then it should be fine to skip registering them. Just make sure that if you're testing changes like this locally that you use a release, non-component build with the chromium linker enabled, because JNI registration isn't actually mandatory in component builds due to the JNI registration changes for webview (the VM will find the functions even if they aren't registered).
,
Sep 19 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9e1613bc13a01a88d24f8b8b158618573bd3d795 commit 9e1613bc13a01a88d24f8b8b158618573bd3d795 Author: agrieve <agrieve@chromium.org> Date: Mon Sep 19 15:18:41 2016 Remove @MainDex from classes within content/browser Removed annotation from: * ContentViewStatics * WebContentsObserverProxy and stopped hooking up their JNI methods for renderer process. Moved FileDescriptorInfo.java from browser/ to common/ since it is actually required in the renderer. BUG= 643489 Review-Url: https://codereview.chromium.org/2316183002 Cr-Commit-Position: refs/heads/master@{#419462} [modify] https://crrev.com/9e1613bc13a01a88d24f8b8b158618573bd3d795/content/app/android/library_loader_hooks.cc [modify] https://crrev.com/9e1613bc13a01a88d24f8b8b158618573bd3d795/content/browser/android/child_process_launcher_android.cc [modify] https://crrev.com/9e1613bc13a01a88d24f8b8b158618573bd3d795/content/public/android/BUILD.gn [modify] https://crrev.com/9e1613bc13a01a88d24f8b8b158618573bd3d795/content/public/android/java/src/org/chromium/content/app/ChildProcessServiceImpl.java [modify] https://crrev.com/9e1613bc13a01a88d24f8b8b158618573bd3d795/content/public/android/java/src/org/chromium/content/browser/ChildProcessConnection.java [modify] https://crrev.com/9e1613bc13a01a88d24f8b8b158618573bd3d795/content/public/android/java/src/org/chromium/content/browser/ChildProcessConnectionImpl.java [modify] https://crrev.com/9e1613bc13a01a88d24f8b8b158618573bd3d795/content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncher.java [modify] https://crrev.com/9e1613bc13a01a88d24f8b8b158618573bd3d795/content/public/android/java/src/org/chromium/content/browser/ContentViewStatics.java [modify] https://crrev.com/9e1613bc13a01a88d24f8b8b158618573bd3d795/content/public/android/java/src/org/chromium/content/browser/webcontents/WebContentsObserverProxy.java [rename] https://crrev.com/9e1613bc13a01a88d24f8b8b158618573bd3d795/content/public/android/java/src/org/chromium/content/common/FileDescriptorInfo.java [modify] https://crrev.com/9e1613bc13a01a88d24f8b8b158618573bd3d795/content/public/android/javatests/src/org/chromium/content/browser/ChildProcessLauncherTest.java [modify] https://crrev.com/9e1613bc13a01a88d24f8b8b158618573bd3d795/content/public/android/junit/src/org/chromium/content/browser/BindingManagerImplTest.java [modify] https://crrev.com/9e1613bc13a01a88d24f8b8b158618573bd3d795/media/base/android/java/src/org/chromium/media/MediaDrmBridge.java
,
Sep 19 2016
|
||
►
Sign in to add a comment |
||
Comment 1 by agrieve@chromium.org
, Sep 2 2016