Automatically start the GPU process 5s after browser startup.

Automatically start the GPU process 5s after browser startup.


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

[Ami GONE FROM CHROMIUM, 2012-03-01 00:05:29]

Speculative CL per chrome-gpu thread "RFC: auto-start GPU process".

Corresponding histograms.xml CL is: https://chromereviews.googleplex.com/4046013

[apatrick_chromium, 2012-03-01 00:26:21]

+zmo

I'm not sure if there are any GPU blacklist related reasons not to attempt to
launch the GPU process at all? Like bad configurations where trying to
initialize Direct3D / OpenGL at all will lead to disaster?

https://chromiumcodereview.appspot.com/9545008/diff/1001/content/browser/brow...
File content/browser/browser_main_loop.cc (right):

https://chromiumcodereview.appspot.com/9545008/diff/1001/content/browser/brow...
content/browser/browser_main_loop.cc:438: 1,  // Arbitrary non-0 (to cause
sandboxing) client_id.
I had a plan at some point to have a pool of GPU processes but I think that will
never happen with the direction Aura is going. Not for this CL but GetForClient
should just take an argument indicating whether to return the sandboxed or
unsandboxed GPU process. I'll do it.

[Zhenyao Mo, 2012-03-01 00:31:24]

On 2012/03/01 00:26:21, apatrick_chromium wrote:
> +zmo
> 
> I'm not sure if there are any GPU blacklist related reasons not to attempt to
> launch the GPU process at all? Like bad configurations where trying to
> initialize Direct3D / OpenGL at all will lead to disaster?

I don't see an issue here.

> 
>
https://chromiumcodereview.appspot.com/9545008/diff/1001/content/browser/brow...
> File content/browser/browser_main_loop.cc (right):
> 
>
https://chromiumcodereview.appspot.com/9545008/diff/1001/content/browser/brow...
> content/browser/browser_main_loop.cc:438: 1,  // Arbitrary non-0 (to cause
> sandboxing) client_id.
> I had a plan at some point to have a pool of GPU processes but I think that
will
> never happen with the direction Aura is going. Not for this CL but
GetForClient
> should just take an argument indicating whether to return the sandboxed or
> unsandboxed GPU process. I'll do it.

[apatrick_chromium, 2012-03-01 03:09:18]

LGTM. I'll hold off on 9559013 (GpuProcessHost::GetForClient to Get) until this
is landed. You'll need another reviewer for owners.

[commit-bot: I haz the power, 2012-03-01 03:16:52]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/fischman@chromium.org/9545008/1001

[commit-bot: I haz the power, 2012-03-01 03:16:55]

Presubmit check for 9545008-1001 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit Messages **
If this change has an associated bug, add BUG=[bug number].

If this change requires manual test instructions to QA team, add
TEST=[instructions].

** Presubmit ERRORS **
Missing LGTM from an OWNER for: content/browser/browser_main_loop.cc

Was the presubmit check useful? Please send feedback & hate mail to
maruel@chromium.org!

[Ami GONE FROM CHROMIUM, 2012-03-01 03:18:12]

jam: please OWNERS-review for content/browser

[jam, 2012-03-01 06:13:02]

lgtm

http://codereview.chromium.org/9545008/diff/1001/content/browser/browser_main...
File content/browser/browser_main_loop.cc (right):

http://codereview.chromium.org/9545008/diff/1001/content/browser/browser_main...
content/browser/browser_main_loop.cc:15: #include
"base/threading/sequenced_worker_pool.h"
not needed?

[jam, 2012-03-01 06:13:02]

lgtm

http://codereview.chromium.org/9545008/diff/1001/content/browser/browser_main...
File content/browser/browser_main_loop.cc (right):

http://codereview.chromium.org/9545008/diff/1001/content/browser/browser_main...
content/browser/browser_main_loop.cc:15: #include
"base/threading/sequenced_worker_pool.h"
not needed?

[Ami GONE FROM CHROMIUM, 2012-03-01 16:03:59]

Thanks for the quick review.

http://codereview.chromium.org/9545008/diff/1001/content/browser/browser_main...
File content/browser/browser_main_loop.cc (right):

http://codereview.chromium.org/9545008/diff/1001/content/browser/browser_main...
content/browser/browser_main_loop.cc:15: #include
"base/threading/sequenced_worker_pool.h"
On 2012/03/01 06:13:02, John Abd-El-Malek wrote:
> not needed?

Done.

[commit-bot: I haz the power, 2012-03-01 16:04:19]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/fischman@chromium.org/9545008/8001

[commit-bot: I haz the power, 2012-03-01 17:52:16]

Try job failure for 9545008-8001 (retry) on mac_rel for step "ui_tests".
It's a second try, previously, step "ui_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[Ami GONE FROM CHROMIUM, 2012-03-01 18:10:27]

jam: PTAL.  I had to tweak worker_uitest.cc because it has hard-coded
expectations about process counts, and this CL bumps the count by 1 at some time
semi-non-deterministically as far as the test is concerned.

On the bright side, the name WaitForProcessCountToBe() was always a lie; by the
time it returned the process count could well be higher, so it makes sense to me
to append AtLeast() to its name, at which point the extra process doesn't hurt
anymore.  WDYT?

[jam, 2012-03-02 22:53:39]

slgtm

[commit-bot: I haz the power, 2012-03-02 22:54:37]

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

[commit-bot: I haz the power, 2012-03-03 01:06:53]

Change committed as 124803