Upstream BrowserChildProcessHostImpl

Upstream BrowserChildProcessHostImpl

This is also needed to make it possible for content_shell and test_shell
to use mobile as user agent. Some methods and files had to be moved out of chrome
to be able to use them in content_shell

BUG=152807, 154122
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=161933

[Yusuf, 2012-10-09 20:51:21]

yfriedman@ for */android
jam@ for content

[Yaron, 2012-10-09 21:38:52]

http://codereview.chromium.org/11066087/diff/1/content/browser/ppapi_plugin_p...
File content/browser/ppapi_plugin_process_host.cc (right):

http://codereview.chromium.org/11066087/diff/1/content/browser/ppapi_plugin_p...
content/browser/ppapi_plugin_process_host.cc:261: base::EnvironmentVector env;
I think we can just drop this. We don't use plugin proceses.

http://codereview.chromium.org/11066087/diff/1/content/browser/renderer_host/...
File content/browser/renderer_host/render_process_host_impl.cc (right):

http://codereview.chromium.org/11066087/diff/1/content/browser/renderer_host/...
content/browser/renderer_host/render_process_host_impl.cc:512:
base::EnvironmentVector env;
This is unnecessary.

http://codereview.chromium.org/11066087/diff/1/content/public/android/java/sr...
File
content/public/android/java/src/org/chromium/content/browser/DeviceUtils.java
(right):

http://codereview.chromium.org/11066087/diff/1/content/public/android/java/sr...
content/public/android/java/src/org/chromium/content/browser/DeviceUtils.java:23:
public static final String TABLET_UI = "tablet-ui";
We've been putting all of these switches in CommandLine.java

[Yaron, 2012-10-09 21:40:18]

http://codereview.chromium.org/11066087/diff/1/chrome/android/testshell/java/...
File
chrome/android/testshell/java/src/org/chromium/chrome/testshell/ChromiumTestShellActivity.java
(right):

http://codereview.chromium.org/11066087/diff/1/chrome/android/testshell/java/...
chrome/android/testshell/java/src/org/chromium/chrome/testshell/ChromiumTestShellActivity.java:41:
CommandLine.getInstance().appendSwitch(DeviceUtils.TABLET_UI);
Is there no shared place in java where we can do this for content shell/test
shell and chrome app?

[Yusuf, 2012-10-09 21:46:32]

http://codereview.chromium.org/11066087/diff/1/chrome/android/testshell/java/...
File
chrome/android/testshell/java/src/org/chromium/chrome/testshell/ChromiumTestShellActivity.java
(right):

http://codereview.chromium.org/11066087/diff/1/chrome/android/testshell/java/...
chrome/android/testshell/java/src/org/chromium/chrome/testshell/ChromiumTestShellActivity.java:41:
CommandLine.getInstance().appendSwitch(DeviceUtils.TABLET_UI);
On 2012/10/09 21:40:18, Yaron wrote:
> Is there no shared place in java where we can do this for content shell/test
> shell and chrome app?

How about DeviceUtils? Now that it is in content. We can use CommandLine, too,
but I think it may be a bit more out of context there.

[Yusuf, 2012-10-10 00:06:07]

http://codereview.chromium.org/11066087/diff/1/chrome/android/testshell/java/...
File
chrome/android/testshell/java/src/org/chromium/chrome/testshell/ChromiumTestShellActivity.java
(right):

http://codereview.chromium.org/11066087/diff/1/chrome/android/testshell/java/...
chrome/android/testshell/java/src/org/chromium/chrome/testshell/ChromiumTestShellActivity.java:41:
CommandLine.getInstance().appendSwitch(DeviceUtils.TABLET_UI);
Moved to DeviceUtils, calling from there for both activities.
On 2012/10/09 21:46:32, Yusuf wrote:
> On 2012/10/09 21:40:18, Yaron wrote:
> > Is there no shared place in java where we can do this for content shell/test
> > shell and chrome app?
> 
> How about DeviceUtils? Now that it is in content. We can use CommandLine, too,
> but I think it may be a bit more out of context there.

http://codereview.chromium.org/11066087/diff/1/content/browser/ppapi_plugin_p...
File content/browser/ppapi_plugin_process_host.cc (right):

http://codereview.chromium.org/11066087/diff/1/content/browser/ppapi_plugin_p...
content/browser/ppapi_plugin_process_host.cc:261: base::EnvironmentVector env;
On 2012/10/09 21:38:53, Yaron wrote:
> I think we can just drop this. We don't use plugin proceses.

Done.

http://codereview.chromium.org/11066087/diff/1/content/browser/renderer_host/...
File content/browser/renderer_host/render_process_host_impl.cc (right):

http://codereview.chromium.org/11066087/diff/1/content/browser/renderer_host/...
content/browser/renderer_host/render_process_host_impl.cc:512:
base::EnvironmentVector env;
On 2012/10/09 21:38:53, Yaron wrote:
> This is unnecessary.

Done.

http://codereview.chromium.org/11066087/diff/1/content/public/android/java/sr...
File
content/public/android/java/src/org/chromium/content/browser/DeviceUtils.java
(right):

http://codereview.chromium.org/11066087/diff/1/content/public/android/java/sr...
content/public/android/java/src/org/chromium/content/browser/DeviceUtils.java:23:
public static final String TABLET_UI = "tablet-ui";
Moved to CommandLine.
On 2012/10/09 21:38:53, Yaron wrote:
> We've been putting all of these switches in CommandLine.java

[Yusuf, 2012-10-10 22:21:19]

jam@ friendly ping for taking a look at this. Thanks!

[jam, 2012-10-11 02:09:14]

https://codereview.chromium.org/11066087/diff/14/content/browser/browser_chil...
File content/browser/browser_child_process_host_impl_android.cc (right):

https://codereview.chromium.org/11066087/diff/14/content/browser/browser_chil...
content/browser/browser_child_process_host_impl_android.cc:18:
cmd_line->AppendSwitch(switches::kUseMobileUserAgent);
does this need to be passed to all subprocess types or just renderer? if the
latter, why isn't this in render_process_host_impl.cc? if the former, which
other processes need this?

https://codereview.chromium.org/11066087/diff/14/content/browser/renderer_hos...
File content/browser/renderer_host/render_process_host_impl.cc (right):

https://codereview.chromium.org/11066087/diff/14/content/browser/renderer_hos...
content/browser/renderer_host/render_process_host_impl.cc:126: #if
defined(OS_ANDROID)
nit: since this is a platform-independent file, don't wrap it with ifdef for
readability

[Yusuf, 2012-10-11 18:40:29]

It looks like it was only passed to renderer. I removed the call and added the
lines to Init() without the if-def.

On 2012/10/11 02:09:14, John Abd-El-Malek wrote:
>
https://codereview.chromium.org/11066087/diff/14/content/browser/browser_chil...
> File content/browser/browser_child_process_host_impl_android.cc (right):
> 
>
https://codereview.chromium.org/11066087/diff/14/content/browser/browser_chil...
> content/browser/browser_child_process_host_impl_android.cc:18:
> cmd_line->AppendSwitch(switches::kUseMobileUserAgent);
> does this need to be passed to all subprocess types or just renderer? if the
> latter, why isn't this in render_process_host_impl.cc? if the former, which
> other processes need this?
> 
>
https://codereview.chromium.org/11066087/diff/14/content/browser/renderer_hos...
> File content/browser/renderer_host/render_process_host_impl.cc (right):
> 
>
https://codereview.chromium.org/11066087/diff/14/content/browser/renderer_hos...
> content/browser/renderer_host/render_process_host_impl.cc:126: #if
> defined(OS_ANDROID)
> nit: since this is a platform-independent file, don't wrap it with ifdef for
> readability

[Yusuf, 2012-10-11 18:40:29]

It looks like it was only passed to renderer. I removed the call and added the
lines to Init() without the if-def.

On 2012/10/11 02:09:14, John Abd-El-Malek wrote:
>
https://codereview.chromium.org/11066087/diff/14/content/browser/browser_chil...
> File content/browser/browser_child_process_host_impl_android.cc (right):
> 
>
https://codereview.chromium.org/11066087/diff/14/content/browser/browser_chil...
> content/browser/browser_child_process_host_impl_android.cc:18:
> cmd_line->AppendSwitch(switches::kUseMobileUserAgent);
> does this need to be passed to all subprocess types or just renderer? if the
> latter, why isn't this in render_process_host_impl.cc? if the former, which
> other processes need this?
> 
>
https://codereview.chromium.org/11066087/diff/14/content/browser/renderer_hos...
> File content/browser/renderer_host/render_process_host_impl.cc (right):
> 
>
https://codereview.chromium.org/11066087/diff/14/content/browser/renderer_hos...
> content/browser/renderer_host/render_process_host_impl.cc:126: #if
> defined(OS_ANDROID)
> nit: since this is a platform-independent file, don't wrap it with ifdef for
> readability

[jam, 2012-10-11 19:38:17]

https://codereview.chromium.org/11066087/diff/7011/content/browser/renderer_h...
File content/browser/renderer_host/render_process_host_impl.cc (right):

https://codereview.chromium.org/11066087/diff/7011/content/browser/renderer_h...
content/browser/renderer_host/render_process_host_impl.cc:127: #include
"content/browser/browser_child_process_host_impl.h"
not needed now

https://codereview.chromium.org/11066087/diff/7011/content/browser/renderer_h...
content/browser/renderer_host/render_process_host_impl.cc:514:
cmd_line->AppendSwitch(switches::kUseMobileUserAgent);
put this in kSwitchNames below

[Yusuf, 2012-10-11 20:22:37]

http://codereview.chromium.org/11066087/diff/7011/content/browser/renderer_ho...
File content/browser/renderer_host/render_process_host_impl.cc (right):

http://codereview.chromium.org/11066087/diff/7011/content/browser/renderer_ho...
content/browser/renderer_host/render_process_host_impl.cc:127: #include
"content/browser/browser_child_process_host_impl.h"
On 2012/10/11 19:38:17, John Abd-El-Malek wrote:
> not needed now

Done.

http://codereview.chromium.org/11066087/diff/7011/content/browser/renderer_ho...
content/browser/renderer_host/render_process_host_impl.cc:514:
cmd_line->AppendSwitch(switches::kUseMobileUserAgent);
On 2012/10/11 19:38:17, John Abd-El-Malek wrote:
> put this in kSwitchNames below

Done.

[Yaron, 2012-10-11 21:21:28]

lgtm

[jam, 2012-10-12 17:10:06]

lgtm

[commit-bot: I haz the power, 2012-10-12 17:18:00]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yusufo@chromium.org/11066087/12001

[commit-bot: I haz the power, 2012-10-12 17:42:08]

Sorry for I got bad news for ya.
Compile failed with a clobber build.
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2012-10-12 17:58:13]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yusufo@chromium.org/11066087/12001

[commit-bot: I haz the power, 2012-10-12 18:12:37]

Sorry for I got bad news for ya.
Compile failed with a clobber build.
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2012-10-12 18:45:27]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yusufo@chromium.org/11066087/24003

[commit-bot: I haz the power, 2012-10-12 19:12:29]

Sorry for I got bad news for ya.
Compile failed with a clobber build.
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2012-10-12 21:07:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yusufo@chromium.org/11066087/31001

[Yusuf, 2012-10-12 21:09:39]

I just realized the switch definition which was upstreamed before was if-def'ed
out. Moved it out as well.

[commit-bot: I haz the power, 2012-10-12 21:30:08]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yusufo@chromium.org/11066087/41002

[commit-bot: I haz the power, 2012-10-12 21:55:52]

Sorry for I got bad news for ya.
Compile failed with a clobber build.
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2012-10-12 22:34:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yusufo@chromium.org/11066087/43003

[commit-bot: I haz the power, 2012-10-13 02:51:57]

Step "update" is always a major failure.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2012-10-15 17:22:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yusufo@chromium.org/11066087/34006

[commit-bot: I haz the power, 2012-10-15 20:06:26]

Change committed as 161933