Added android_browser_process.

As part of the content_shell bring up, some un-upstreamed features
have been disabled in browser process main related c++ and Java file.

The disabled features are guarded by !defined(ANDROID_UPSTREAM_BRINGUP).
For Java file, the code was removed.

Fixed the dependency.

- Moved the content_jni_registrar.{h|cc} and library_loader_hooks.cc content/app/android/
- Moved the android_library_loader_hooks.h to content/public/app/

BUG=
TEST=


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=140386

[Yaron, 2012-06-01 20:20:37]

http://codereview.chromium.org/10444121/diff/2001/content/shell/android/shell...
File content/shell/android/shell_library_loader.cc (right):

http://codereview.chromium.org/10444121/diff/2001/content/shell/android/shell...
content/shell/android/shell_library_loader.cc:10: #include
"content/common/android/process_main.h"
Does this have to in public because you're referencing it in content/shell?

[jam, 2012-06-01 22:04:54]

(some initial comments)

http://codereview.chromium.org/10444121/diff/2001/content/common/android/proc...
File content/common/android/process_main.h (right):

http://codereview.chromium.org/10444121/diff/2001/content/common/android/proc...
content/common/android/process_main.h:15: extern content::ContentMainDelegate*
g_content_main_delegate;
we usually don't expose globals this way, both in normal headers in content, and
especially in the content api which this appears to need to move to since you
need it in content/shell. content_shell is a test program around content, so it
can only access the public API of content

http://codereview.chromium.org/10444121/diff/2001/content/common/android/proc...
content/common/android/process_main.h:17: bool InitProcess(JNIEnv* env, jobject
context);
should this file really be in content/common? isn't the analogous non-android
version of this file ContentMain? that's in content/app

http://codereview.chromium.org/10444121/diff/2001/content/shell/android/shell...
File content/shell/android/shell_library_loader.cc (right):

http://codereview.chromium.org/10444121/diff/2001/content/shell/android/shell...
content/shell/android/shell_library_loader.cc:10: #include
"content/common/android/process_main.h"
On 2012/06/01 20:20:37, Yaron wrote:
> Does this have to in public because you're referencing it in content/shell?

exactly, you'll get a checkdeps failure in the try run because of this

[michaelbai, 2012-06-01 23:27:19]

Thanks for the feedback.

Removed process_main.{h|cc}, moved the logical to
content/app/android/content_main.cc.

PTAL

http://codereview.chromium.org/10444121/diff/2001/content/common/android/proc...
File content/common/android/process_main.h (right):

http://codereview.chromium.org/10444121/diff/2001/content/common/android/proc...
content/common/android/process_main.h:15: extern content::ContentMainDelegate*
g_content_main_delegate;
Added InitContentMainDelegate in content/public/app/content_main.h, and
implement it in content/app/android/content_main.cc

On 2012/06/01 22:04:55, John Abd-El-Malek wrote:
> we usually don't expose globals this way, both in normal headers in content,
and
> especially in the content api which this appears to need to move to since you
> need it in content/shell. content_shell is a test program around content, so
it
> can only access the public API of content

http://codereview.chromium.org/10444121/diff/2001/content/common/android/proc...
content/common/android/process_main.h:17: bool InitProcess(JNIEnv* env, jobject
context);
Right! this doesn't need, and could by merge with content_main.

Removed process_main.{h|cc}, and moved the logical to InitApplicationContext()
in content/android/content_main.cc

On 2012/06/01 22:04:55, John Abd-El-Malek wrote:
> should this file really be in content/common? isn't the analogous non-android
> version of this file ContentMain? that's in content/app

http://codereview.chromium.org/10444121/diff/2001/content/shell/android/shell...
File content/shell/android/shell_library_loader.cc (right):

http://codereview.chromium.org/10444121/diff/2001/content/shell/android/shell...
content/shell/android/shell_library_loader.cc:10: #include
"content/common/android/process_main.h"
On 2012/06/01 22:04:55, John Abd-El-Malek wrote:
> On 2012/06/01 20:20:37, Yaron wrote:
> > Does this have to in public because you're referencing it in content/shell?
> 
> exactly, you'll get a checkdeps failure in the try run because of this

Done.

[jam, 2012-06-01 23:40:00]

http://codereview.chromium.org/10444121/diff/13001/content/app/android/browse...
File content/app/android/browser_process_main.cc (right):

http://codereview.chromium.org/10444121/diff/13001/content/app/android/browse...
content/app/android/browser_process_main.cc:1: // Copyright (c) 2012 The
Chromium Authors. All rights reserved.
it's not clear to me why this file is in content/app instead of content/browser?

http://codereview.chromium.org/10444121/diff/13001/content/public/android/jav...
File
content/public/android/java/org/chromium/content/app/BrowserProcessMain.java
(right):

http://codereview.chromium.org/10444121/diff/13001/content/public/android/jav...
content/public/android/java/org/chromium/content/app/BrowserProcessMain.java:1:
// Copyright (c) 2012 The Chromium Authors. All rights reserved.
the name of this file suggest it should be in
content/public/android/java/org/chromium/content/browser/BrowserProcessMain.java?
there's also a bunch of browser-process specific stuff.

http://codereview.chromium.org/10444121/diff/13001/content/public/app/content...
File content/public/app/content_main.h (right):

http://codereview.chromium.org/10444121/diff/13001/content/public/app/content...
content/public/app/content_main.h:24: #if !defined(OS_ANDROID)
nit: i think it would be more readable if you change line 36 to be

#else defined(OS_ANDROID)

and move the code there

http://codereview.chromium.org/10444121/diff/13001/content/public/app/content...
content/public/app/content_main.h:47: void
InitContentMainDelegate(ContentMainDelegate* delegate);
nit: SetContentMainDelegate

[jam, 2012-06-01 23:40:51]

http://codereview.chromium.org/10444121/diff/13001/content/app/android/browse...
File content/app/android/browser_process_main.cc (right):

http://codereview.chromium.org/10444121/diff/13001/content/app/android/browse...
content/app/android/browser_process_main.cc:1: // Copyright (c) 2012 The
Chromium Authors. All rights reserved.
On 2012/06/01 23:40:00, John Abd-El-Malek wrote:
> it's not clear to me why this file is in content/app instead of
content/browser?

to be clear: this seems analogous to BrowserMain, which is in
src/content/browser

[michaelbai, 2012-06-04 17:39:36]

Hi John,

In Android, the process is started from Java, and the different type of
processes are started from different Java files.

We can not have the same entry like content_main as other platform.

Furthermore, some Java side initialization need to be done before
content_main_runner.

So, we need AndroidBrowserProcess in app directory and do some process specific
initialization in Java side. Actually,  in downstream, we also have
SandboxedProcessService in app. They were moved to app directory, because they
need to call content_main_runner.


PTAL

http://codereview.chromium.org/10444121/diff/13001/content/app/android/browse...
File content/app/android/browser_process_main.cc (right):

http://codereview.chromium.org/10444121/diff/13001/content/app/android/browse...
content/app/android/browser_process_main.cc:1: // Copyright (c) 2012 The
Chromium Authors. All rights reserved.
The code in this file can not move to content/BrowserMain. I changed this to
android_browser_process.

For Android, we can not have a single entry point for all types of processes,
the browser process is Activity, other processes are Services.

The methods in this file are JNI methods, and they have to be called by Java
before the content_main_runner.Initialize() method.


On 2012/06/01 23:40:51, John Abd-El-Malek wrote:
> On 2012/06/01 23:40:00, John Abd-El-Malek wrote:
> > it's not clear to me why this file is in content/app instead of
> content/browser?
> 
> to be clear: this seems analogous to BrowserMain, which is in
> src/content/browser

http://codereview.chromium.org/10444121/diff/13001/content/public/android/jav...
File
content/public/android/java/org/chromium/content/app/BrowserProcessMain.java
(right):

http://codereview.chromium.org/10444121/diff/13001/content/public/android/jav...
content/public/android/java/org/chromium/content/app/BrowserProcessMain.java:1:
// Copyright (c) 2012 The Chromium Authors. All rights reserved.
The code in this file need run before the content_main_runner.

I changed the file name to AndroidBrowserProcess.


On 2012/06/01 23:40:00, John Abd-El-Malek wrote:
> the name of this file suggest it should be in
>
content/public/android/java/org/chromium/content/browser/BrowserProcessMain.java?
> there's also a bunch of browser-process specific stuff.

http://codereview.chromium.org/10444121/diff/13001/content/public/app/content...
File content/public/app/content_main.h (right):

http://codereview.chromium.org/10444121/diff/13001/content/public/app/content...
content/public/app/content_main.h:24: #if !defined(OS_ANDROID)
On 2012/06/01 23:40:00, John Abd-El-Malek wrote:
> nit: i think it would be more readable if you change line 36 to be
> 
> #else defined(OS_ANDROID)
> 
> and move the code there

Done.

http://codereview.chromium.org/10444121/diff/13001/content/public/app/content...
content/public/app/content_main.h:47: void
InitContentMainDelegate(ContentMainDelegate* delegate);
I used 'Init' instead of 'Set' because it can only be set once. 
Do we have this convention in chromium?

On 2012/06/01 23:40:00, John Abd-El-Malek wrote:
> nit: SetContentMainDelegate

[jam, 2012-06-04 17:53:27]

On 2012/06/04 17:39:36, michaelbai wrote:
> Hi John,
> 
> In Android, the process is started from Java, and the different type of
> processes are started from different Java files.
> 
> We can not have the same entry like content_main as other platform.
> 
> Furthermore, some Java side initialization need to be done before
> content_main_runner.
> 
> So, we need AndroidBrowserProcess in app directory and do some process
specific
> initialization in Java side. Actually,  in downstream, we also have
> SandboxedProcessService in app. They were moved to app directory, because they
> need to call content_main_runner.


What I meant is why isn't AndroidBrowserProcess located in content/browser
instead of content/app?

[michaelbai, 2012-06-04 18:10:50]

Because it needs to call ContentMainRunner, this is the previous CL which cann't
be submitted because it call ContentMainRunner from browser directory.

http://codereview.chromium.org/10399129/

On 2012/06/04 17:53:27, John Abd-El-Malek wrote:
> On 2012/06/04 17:39:36, michaelbai wrote:
> > Hi John,
> > 
> > In Android, the process is started from Java, and the different type of
> > processes are started from different Java files.
> > 
> > We can not have the same entry like content_main as other platform.
> > 
> > Furthermore, some Java side initialization need to be done before
> > content_main_runner.
> > 
> > So, we need AndroidBrowserProcess in app directory and do some process
> specific
> > initialization in Java side. Actually,  in downstream, we also have
> > SandboxedProcessService in app. They were moved to app directory, because
they
> > need to call content_main_runner.
> 
> 
> What I meant is why isn't AndroidBrowserProcess located in content/browser
> instead of content/app?

[michaelbai, 2012-06-04 19:04:18]

Hi John,

As our discussion, I moved android_browser_process to content/browser.

PTAL

[jam, 2012-06-04 19:26:43]

lgtm

http://codereview.chromium.org/10444121/diff/13001/content/public/app/content...
File content/public/app/content_main.h (right):

http://codereview.chromium.org/10444121/diff/13001/content/public/app/content...
content/public/app/content_main.h:47: void
InitContentMainDelegate(ContentMainDelegate* delegate);
Init sounds like this function will initialize the delegate somehow, but it's
just a setter, which is why I recommended Set

On 2012/06/04 17:39:36, michaelbai wrote:
> I used 'Init' instead of 'Set' because it can only be set once. 
> Do we have this convention in chromium?
> 
> On 2012/06/01 23:40:00, John Abd-El-Malek wrote:
> > nit: SetContentMainDelegate
>

[commit-bot: I haz the power, 2012-06-04 20:58:27]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/michaelbai@chromium.org/10444121/36001

[commit-bot: I haz the power, 2012-06-04 21:48:18]

Change committed as 140386