Remove @MainDex from classes within content/browser

content/app/android/library_loader_hooks.cc

 // Copyright (c) 2012 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.
 
 #include "content/app/android/library_loader_hooks.h"
 
 #include "base/android/base_jni_registrar.h"
 #include "base/android/command_line_android.h"
 #include "base/android/jni_android.h"
 #include "base/android/jni_registrar.h"
 #include "base/android/jni_string.h"
+#include "base/android/library_loader/library_loader_hooks.h"
 #include "base/base_switches.h"
 #include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/logging.h"
 #include "base/strings/string_util.h"
 #include "base/trace_event/trace_event.h"
 #include "base/tracked_objects.h"
 #include "components/tracing/browser/trace_config_file.h"
 #include "components/tracing/common/trace_to_console.h"
 #include "components/tracing/common/tracing_switches.h"
@@ skipping 38 matching lines @@
 
     if (!ui::gl::android::RegisterJni(env))
       return false;
 
     if (!ui::shell_dialogs::RegisterJni(env))
       return false;
 
     if (!content::android::RegisterCommonJni(env))
       return false;
 
-    if (!content::android::RegisterBrowserJni(env))
-      return false;
+    if (base::android::GetLibraryProcessType(env) ==
+        base::android::PROCESS_BROWSER) {

[Torne, 2016/09/12 13:10:13]

There's four values for process type, not just two - my first thought was that
you should probably also handle PROCESS_WEBVIEW here since this is content layer
code that's shared.

However, there's two weird things there:
1) WebView doesn't actually need JNI registration in the first place
2) It's kinda weird that we have this 4-way enum instead of separate flags.

Maybe we could reinvent the library process type stuff to be better? (adding
michaelbai who did the monochrome work and may have thoughts)

What the library process type is actually used for:

1) The monochrome entry point decides which real entry point to call by checking
to see whether the process type is WEBVIEW/WEBVIEW_CHILD or not. This affects
which JNI registration functions are called, but also changes which actual init
functions are called, so it's not purely about JNI registration.

2) Several places (including this one you're introducing here) want to know
whether the current process is a browser or renderer process so they can avoid
doing some of the JNI registration work (for browser-only code) in renderer
processes.

These two are basically orthogonal, and combining them into a single enum
instead of having two separate flags makes it seem weird to me. I'm not sure
that two separate flags is right either though: what there actually is here is
*three choices* for how much JNI registration needs to be done:

1) The regular pre-N Chrome browser process needs to register everything.

2) The regular pre-N Chrome renderer process needs to register some subset of
things (which we're still working on determining exactly what that is, and
shrinking it to the minimum, as with this CL).

3) WebView and Monochrome *don't need to register anything at all*, no matter
what process type they are. This may come as a surprise, but the only reason we
have explicit JNI registration is to make the crazy linker work (since implicit
JNI method lookup is broken by the crazy linker for reasons that are too hard to
fix), and neither WebView nor Monochrome ever use the crazy linker.

Right now, we do still end up calling a bunch of JNI registration in both
WebView and Monochrome, in various layers, but the registration functions are
shortcircuited for.. just WebView now that I look at it, which is weird, by
having called base::android::DisableManualJniRegistration, which turns all the
actual JNI-generator-generated registration functions into no-ops at runtime.


What I wanted us to have from the beginning was to simply never call any JNI
registration functions in the first place when in WebView/Monochrome - but at
the time, JNI registration was confusingly mixed up with other kinds of library
initialisation that aren't safe to skip in various places. We should probably
look to see if that's still the case and fix it, so that we can just not call
them.

We could *also* avoid needing to distinguish between WebView and not-WebView in
the Monochrome entry point if we just didn't do *either* set of initialisation
in JNI_OnLoad, and instead just exposed both webview and chrome's entry points
as separate JNI functions that the Java code can call after loading the library
but before calling anything else..

So, if all this could be made to work, this enum could get much simpler and
could just *actually* be "browser vs renderer" for the case you have here.

I realise this is pretty long; sorry :)

[michaelbai, 2016/09/12 18:54:36]

The four process types defined in
https://cs.chromium.org/chromium/src/base/android/library_loader/library_load...
need a little clarify, because at the time the code written, we did't have
multiple processes WebView yet. 

PROCESS_BROWSER, // Chrome's browser process
PROCESS_CHILD, // Chrome's child processes including renderer, GPU etc.
PROCESS_WEBVIEW, // WebView process or WebView's browser process for multiple
processes case.
PROCESS_WEBVIEW_CHILD, // WebView's child process like renderer and is only
valid if WebView's multiple processes.

As Torne explained, WebView doesn't need JNI registration, so the code in this
patch seems correct to me.

As to 2 weird things Torne mentioned
Thing 1, I don't think WebView is running into this code. WebView never
registers JNI.

Thing 2, We should follow it up along with the issue that whether Chrome should
register JNI in Monochrome.

There are a number of reasons Chrome register JNI in Monochrome at time I wrote
the code.

1. It is critical to make monochrome work first.
2. Made chrome consistent, i.e Chrome always registers JNI
3. Is there performance difference between 2 approaches? 

If we find out chrome in Monochrome could skip the JNI registration, we just
change Monochrome entry point not to call OnJNIOnLoadRegisterJNI().

We should file a bug to discuss JNI registration issue, this patch seems not
related, agree?

[agrieve, 2016/09/12 19:22:22]

Is it accurate that JNI registration isn't required any time that the crazy
linker isn't used? And is it accurate to say that it's not necessary when
running Chrome.apk / ChromeModern.apk on Android N?

Sounds like maybe what we want is:
  GetJniRegistrationContext() = {None, Browser, Renderer}

And leave GetProcessType() as-is.

[michaelbai, 2016/09/12 19:25:27]

No, as Chrome.apk and ChromeModern.apk still use crazylinker, JNI registration
is needed. Only Monochrome and WebView don't need JNI regstration.

+      if (!content::android::RegisterBrowserJni(env))
+        return false;
+    }
 
     if (!content::android::RegisterAppJni(env))
       return false;
 
     if (!device::android::RegisterBluetoothJni(env))
       return false;
 
     if (!device::android::RegisterGamepadJni(env))
       return false;
 
@@ skipping 66 matching lines @@
                        false,    // Thread ID
                        false,    // Timestamp
                        false);   // Tick count
   VLOG(0) << "Chromium logging enabled: level = " << logging::GetMinLogLevel()
           << ", default verbosity = " << logging::GetVlogVerbosity();
 
   return true;
 }
 
 }  // namespace content