Remove @MainDex from classes within content/browser

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
Committed: https://crrev.com/9e1613bc13a01a88d24f8b8b158618573bd3d795
Cr-Commit-Position: refs/heads/master@{#419462}

[agrieve, 2016-09-08 20:13:30]

Description was changed from

==========
Remove @MainDex from classes within content/browser

BUG=643489
==========

to

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

[agrieve, 2016-09-09 20:14:03]

agrieve@chromium.org changed reviewers:
+ nick@chromium.org, torne@chromium.org, watk@chromium.org

[agrieve, 2016-09-09 20:14:04]

torne@ - please review JNI changes.

watk & nick - please RS (or review)

[agrieve, 2016-09-09 20:14:27]

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

[agrieve, 2016-09-09 20:14:35]

On 2016/09/09 20:14:04, agrieve wrote:
> torne@ - please review JNI changes.
> 
> watk & nick - please RS (or review)

[commit-bot: I haz the power, 2016-09-09 20:15:00]

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, 2016-09-09 22:02:52]

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

[commit-bot: I haz the power, 2016-09-09 22:02:53]

Dry run: This issue passed the CQ dry run.

[ncarter (slow), 2016-09-09 23:13:46]

rs lgtm for content api owners
please make sure you get a real review too; I don't know what the @MainDex
annotation does.

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

torne@chromium.org changed reviewers:
+ michaelbai@chromium.org

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

https://codereview.chromium.org/2316183002/diff/80001/content/app/android/lib...
File content/app/android/library_loader_hooks.cc (right):

https://codereview.chromium.org/2316183002/diff/80001/content/app/android/lib...
content/app/android/library_loader_hooks.cc:72: base::android::PROCESS_BROWSER)
{
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 :)

https://codereview.chromium.org/2316183002/diff/80001/content/public/android/...
File
content/public/android/java/src/org/chromium/content/browser/FileDescriptorInfo.java
(left):

https://codereview.chromium.org/2316183002/diff/80001/content/public/android/...
content/public/android/java/src/org/chromium/content/browser/FileDescriptorInfo.java:2:
// Use of this source code is governed by a BSD-style license that can be
Any idea why this file shows up as modified and not just deleted?

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

https://codereview.chromium.org/2316183002/diff/80001/content/app/android/lib...
File content/app/android/library_loader_hooks.cc (right):

https://codereview.chromium.org/2316183002/diff/80001/content/app/android/lib...
content/app/android/library_loader_hooks.cc:72: base::android::PROCESS_BROWSER)
{
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:23]

https://codereview.chromium.org/2316183002/diff/80001/content/app/android/lib...
File content/app/android/library_loader_hooks.cc (right):

https://codereview.chromium.org/2316183002/diff/80001/content/app/android/lib...
content/app/android/library_loader_hooks.cc:72: base::android::PROCESS_BROWSER)
{
On 2016/09/12 18:54:36, michaelbai wrote:
> 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?
> 

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]

https://codereview.chromium.org/2316183002/diff/80001/content/app/android/lib...
File content/app/android/library_loader_hooks.cc (right):

https://codereview.chromium.org/2316183002/diff/80001/content/app/android/lib...
content/app/android/library_loader_hooks.cc:72: base::android::PROCESS_BROWSER)
{
On 2016/09/12 19:22:22, agrieve wrote:
> On 2016/09/12 18:54:36, michaelbai wrote:
> > 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?
> > 
> 
> 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.

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

[Torne, 2016-09-13 14:50:22]

On 2016/09/12 18:54:36, michaelbai wrote:
>
https://codereview.chromium.org/2316183002/diff/80001/content/app/android/lib...
> File content/app/android/library_loader_hooks.cc (right):
> 
>
https://codereview.chromium.org/2316183002/diff/80001/content/app/android/lib...
> content/app/android/library_loader_hooks.cc:72:
base::android::PROCESS_BROWSER)
> {
> 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.

I was talking about it being weird that the Monochrome entry point needs to
check the library process type in the first place, which is what introduces the
four possible choices for library process type. If the monochrome entry point
instead worked as I described, we wouldn't really need a webview or
webview_child process type at all.

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

The expectation is that *not* registering it is measurably faster to start up -
it was for WebView. It's possible the difference will be smaller in chrome as
chrome doesn't do so much of its initialisation in a blocking way on the main
thread, but I'd be pretty surprised if it was *worse*. We should check,
obviously..

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

If we're actually sure that we're being consistent about whether we call JNI
registration functions or not, then we shouldn't need
base::android::DisableManualJniRegistration, because none of those codepaths
should be being hit in the first place, and we could just remove that somewhat
weird feature from the JNI generator.

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

Yes, we should really have filed a bug for it during Monochrome development.

[Torne, 2016-09-13 14:54:25]

On 2016/09/12 19:22:23, agrieve wrote:
>
https://codereview.chromium.org/2316183002/diff/80001/content/app/android/lib...
> File content/app/android/library_loader_hooks.cc (right):
> 
>
https://codereview.chromium.org/2316183002/diff/80001/content/app/android/lib...
> content/app/android/library_loader_hooks.cc:72:
base::android::PROCESS_BROWSER)
> {
> On 2016/09/12 18:54:36, michaelbai wrote:
> > 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?
> > 
> 
> 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?

No. The decision about whether the automatic registration is going to work for
the binary or not is made at link time, not at runtime. The version of the
chrome .so in Chrome and ChromeModern has already had all the symbols that the
automatic registration relies on stripped out of the binary, specifically to
*make sure* that people don't accidentally depend on automatic registration
because they didn't happen to test on a configuration where the crazy linker is
used. So, the restriction is that any binary which might get put into an APK
that uses the crazy linker must do manual JNI registration - it doesn't actually
matter what happens at runtime or what OS version you are on.

However, since the webview and monochrome .so are *never* loaded by the crazy
linker no matter what, they are built with the symbols required for JNI lookup
left in, and it works whether you register or not.

> 
> Sounds like maybe what we want is:
>   GetJniRegistrationContext() = {None, Browser, Renderer}
> 
> And leave GetProcessType() as-is.

Sure, but my point is that we don't need GetProcessType *at all* in that case -
if we have something else to control the JNI registration, then we can also just
refactor the monochrome entry point to not check the process type either, and we
can just delete the concept entirely. (or just rename/repurpose it to *be* the
thing that controls JNI registration, same thing I guess).

I think this is desirable, because having this global notion of the process type
is actually pretty confusing to people since it's being used in these fairly
orthogonal ways, and people aren't generally going to remember that WebView has
its own separate process types..

[watk, 2016-09-13 17:50:03]

media/ lgtm

[michaelbai, 2016-09-16 00:55:03]

On 2016/09/13 14:50:22, Torne wrote:
> On 2016/09/12 18:54:36, michaelbai wrote:
> >
>
https://codereview.chromium.org/2316183002/diff/80001/content/app/android/lib...
> > File content/app/android/library_loader_hooks.cc (right):
> > 
> >
>
https://codereview.chromium.org/2316183002/diff/80001/content/app/android/lib...
> > content/app/android/library_loader_hooks.cc:72:
> base::android::PROCESS_BROWSER)
> > {
> > 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.
> 
> I was talking about it being weird that the Monochrome entry point needs to
> check the library process type in the first place, which is what introduces
the
> four possible choices for library process type. If the monochrome entry point
> instead worked as I described, we wouldn't really need a webview or
> webview_child process type at all.
> 

I understood your point, at that time, my judgement is 
1. we didn't know the performance impact
2. the first goal is making Monochrome work
3. it is not change the original behavior for Chrome continually to use static
registration, and the possible improvement you mentioned is the one we could
revisit once monochrome released. If Monochrome can't be shipped, it is wasting
time to address this issue at that time. I filed bug to track it
http://crbug.com/647517

Agree?

> > 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? 
> 
> The expectation is that *not* registering it is measurably faster to start up
-
> it was for WebView. It's possible the difference will be smaller in chrome as
> chrome doesn't do so much of its initialisation in a blocking way on the main
> thread, but I'd be pretty surprised if it was *worse*. We should check,
> obviously..
> 
> > If we find out chrome in Monochrome could skip the JNI registration, we just
> > change Monochrome entry point not to call OnJNIOnLoadRegisterJNI().
> 
> If we're actually sure that we're being consistent about whether we call JNI
> registration functions or not, then we shouldn't need
> base::android::DisableManualJniRegistration, because none of those codepaths
> should be being hit in the first place, and we could just remove that somewhat
> weird feature from the JNI generator.
> 

I think the way to **make sure** about there is no JNI registration is to have
dcheck to see whether there is JNI registration when manual JNI registration is
disabled, not everyone are aware of the difference, we should have code to
enforce it.
Does this feature already exist in base::android::DisableManualJniRegistration?
if not, we may should add dcheck when registration is disabled, it seems that we
still need this method or do something when JNI registration is not expected. 


> > We should file a bug to discuss JNI registration issue, this patch seems not
> > related, agree?
> 
> Yes, we should really have filed a bug for it during Monochrome development.

[michaelbai, 2016-09-16 01:52:47]

On 2016/09/13 14:54:25, Torne wrote:
> On 2016/09/12 19:22:23, agrieve wrote:
> >
>
https://codereview.chromium.org/2316183002/diff/80001/content/app/android/lib...
> > File content/app/android/library_loader_hooks.cc (right):
> > 
> >
>
https://codereview.chromium.org/2316183002/diff/80001/content/app/android/lib...
> > content/app/android/library_loader_hooks.cc:72:
> base::android::PROCESS_BROWSER)
> > {
> > On 2016/09/12 18:54:36, michaelbai wrote:
> > > 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?
> > > 
> > 
> > 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?
> 
> No. The decision about whether the automatic registration is going to work for
> the binary or not is made at link time, not at runtime. The version of the
> chrome .so in Chrome and ChromeModern has already had all the symbols that the
> automatic registration relies on stripped out of the binary, specifically to
> *make sure* that people don't accidentally depend on automatic registration
> because they didn't happen to test on a configuration where the crazy linker
is
> used. So, the restriction is that any binary which might get put into an APK
> that uses the crazy linker must do manual JNI registration - it doesn't
actually
> matter what happens at runtime or what OS version you are on.
> 
> However, since the webview and monochrome .so are *never* loaded by the crazy
> linker no matter what, they are built with the symbols required for JNI lookup
> left in, and it works whether you register or not.
> 
> > 
> > Sounds like maybe what we want is:
> >   GetJniRegistrationContext() = {None, Browser, Renderer}
> > 
> > And leave GetProcessType() as-is.
> 
> Sure, but my point is that we don't need GetProcessType *at all* in that case
-
> if we have something else to control the JNI registration, then we can also
just
> refactor the monochrome entry point to not check the process type either, and
we
> can just delete the concept entirely. (or just rename/repurpose it to *be* the
> thing that controls JNI registration, same thing I guess).
>

The issue is that GetProcessType is so general, people could use it to do other
things, I actually stopped some effort to add one more process type here, we
probably should change it to JNI registration specific, but couldn't get ride of
it at all, hopefully, nobody has used it to do something other than JNI
registration.

 
> I think this is desirable, because having this global notion of the process
type
> is actually pretty confusing to people since it's being used in these fairly
> orthogonal ways, and people aren't generally going to remember that WebView
has
> its own separate process types..

hmm, it seemed we can't avoid this, people have to remember all possible process
types if they really want to do different things for different process. e.g.
some use cases could apply to both Chrome and WebView's browser process, while
other could just apply to Chrome browser process or WebView browser process,
here is real example:
https://chrome-internal-review.googlesource.com/#/c/248008/2/android_webview/...

[Torne, 2016-09-16 10:20:51]

So anyway:

1) This patch is fine as-is because when the process is webview the JNI
registration function doesn't get called at all (probably) and it doesn't matter
whether it registers stuff or not anyway. LGTM, sorry for the digression :)

2) Michael and I will look into whether we can avoid having separate WebView
process types (on the bug he filed or elsewhere) and if so we can consider
simplifying the process type enum to being explicitly about JNI.

[agrieve, 2016-09-19 14:10:02]

The CQ bit was checked by agrieve@chromium.org

[commit-bot: I haz the power, 2016-09-19 14:10:14]

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, 2016-09-19 15:19:05]

Description was changed from

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

to

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

[commit-bot: I haz the power, 2016-09-19 15:19:08]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-09-19 15:20:59]

Description was changed from

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

to

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

Committed: https://crrev.com/9e1613bc13a01a88d24f8b8b158618573bd3d795
Cr-Commit-Position: refs/heads/master@{#419462}
==========

[commit-bot: I haz the power, 2016-09-19 15:21:00]

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