This defers starting background extension page RenderViews

This defers starting background extension page RenderViews
until after session restore has completed.

This is mainly to help login/startup times so that Chrome is useful to
the user earlier.  To make sure that this is actually helping, this CL
includes a Finch experiment that will only enable the deferral on 50%
of the clients.

It is also expected that deferring these will help with some problems
we've seen when extensions attempt to do GAIA authentication.

BUG=279427, 259791
TEST=Ran performance tests on ChromeOS and Linux

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=221621

[Greg Spencer (Chromium), 2013-08-30 22:29:20]

Matt, could you please take a look at this and see if this makes sense?

My goal here was to do what you suggested, and just defer the creation of the
RenderViews (and only then if they didn't already have processes created).

Once you've reviewed, I'll bug sky@ for an OWNERs review.

[Matt Perry, 2013-08-31 00:22:23]

https://chromiumcodereview.appspot.com/23618014/diff/5001/chrome/browser/exte...
File chrome/browser/extensions/extension_process_manager.cc (right):

https://chromiumcodereview.appspot.com/23618014/diff/5001/chrome/browser/exte...
chrome/browser/extensions/extension_process_manager.cc:307: if
(defer_background_host_creation_)
Since you're doing this for all background hosts, I think it'd be better to use
ExtensionProcessManager::DeferLoadingBackgroundHosts. Add some logic to call
CreateBackgroundHostsForProfileStartup when that switches from true to false.
Then you don't need to change ExtensionHost at all.

[Greg Spencer (Chromium), 2013-09-03 19:09:31]

https://chromiumcodereview.appspot.com/23618014/diff/5001/chrome/browser/exte...
File chrome/browser/extensions/extension_process_manager.cc (right):

https://chromiumcodereview.appspot.com/23618014/diff/5001/chrome/browser/exte...
chrome/browser/extensions/extension_process_manager.cc:307: if
(defer_background_host_creation_)
On 2013/08/31 00:22:23, Matt Perry wrote:
> Since you're doing this for all background hosts, I think it'd be better to
use
> ExtensionProcessManager::DeferLoadingBackgroundHosts. Add some logic to call
> CreateBackgroundHostsForProfileStartup when that switches from true to false.
> Then you don't need to change ExtensionHost at all.

Good point, that does simplify things. Done.

[Matt Perry, 2013-09-03 19:26:32]

https://chromiumcodereview.appspot.com/23618014/diff/11001/chrome/browser/ext...
File chrome/browser/extensions/extension_process_manager.cc (right):

https://chromiumcodereview.appspot.com/23618014/diff/11001/chrome/browser/ext...
chrome/browser/extensions/extension_process_manager.cc:792: bool
ExtensionProcessManager::DeferLoadingBackgroundHosts() const {
could you integrate this method with your new method? there should be a common
switch that is gated on all the various conditions.

[Greg Spencer (Chromium), 2013-09-04 21:17:24]

https://chromiumcodereview.appspot.com/23618014/diff/11001/chrome/browser/ext...
File chrome/browser/extensions/extension_process_manager.cc (right):

https://chromiumcodereview.appspot.com/23618014/diff/11001/chrome/browser/ext...
chrome/browser/extensions/extension_process_manager.cc:792: bool
ExtensionProcessManager::DeferLoadingBackgroundHosts() const {
On 2013/09/03 19:26:32, Matt Perry wrote:
> could you integrate this method with your new method? there should be a common
> switch that is gated on all the various conditions.

Done.  I thought of this too, but was a little hesitant to do that before
because I didn't want to mess with the logic that was already there.  I think
you're right that it makes sense to integrate these, however.  The new version
will check for a valid profile and some flags at times when it didn't before,
but I think that shouldn't affect how the logic actually works, because the
profile should be valid already in those new cases.

[Matt Perry, 2013-09-04 21:29:31]

lgtm

[Greg Spencer (Chromium), 2013-09-04 21:45:15]

Scott, could I have an OWNERS review for
chrome/browser/ui/startup/startup_browser_creator_impl.cc please?

Miranda, could I have an OWNERS review for
chrome/browser/profiles/profile_manager.cc
please?

(I'll probably TBR the reviews for chrome/browser/sync/..., since they are all
trivial).

[sky, 2013-09-04 21:55:34]

https://chromiumcodereview.appspot.com/23618014/diff/15001/chrome/browser/ui/...
File chrome/browser/ui/startup/startup_browser_creator_impl.cc (right):

https://chromiumcodereview.appspot.com/23618014/diff/15001/chrome/browser/ui/...
chrome/browser/ui/startup/startup_browser_creator_impl.cc:396:
extensions::ExtensionSystem::Get(profile)->process_manager();
Will this force creation of any extension bits that might not have been running?

[Greg Spencer (Chromium), 2013-09-04 22:17:34]

https://chromiumcodereview.appspot.com/23618014/diff/15001/chrome/browser/ui/...
File chrome/browser/ui/startup/startup_browser_creator_impl.cc (right):

https://chromiumcodereview.appspot.com/23618014/diff/15001/chrome/browser/ui/...
chrome/browser/ui/startup/startup_browser_creator_impl.cc:396:
extensions::ExtensionSystem::Get(profile)->process_manager();
On 2013/09/04 21:55:34, sky wrote:
> Will this force creation of any extension bits that might not have been
running?

It shouldn't on systems where extensions are enabled: the profile manager will
have already been initialized by this point (which is what initializes the
ExtensionSystem by calling InitForRegularProfile currently), so the Get() won't
create anything new, and the process_manager() call just returns a member of
ExtensionService that should already have been created when the ExtensionService
was.

Your comment made me realize that I probably needed to surround this new piece
with "#if defined(EXTENSIONS_ENABLED)", however, so I did that.

[Greg Spencer (Chromium), 2013-09-04 22:20:24]

https://chromiumcodereview.appspot.com/23618014/diff/15001/chrome/browser/ui/...
File chrome/browser/ui/startup/startup_browser_creator_impl.cc (right):

https://chromiumcodereview.appspot.com/23618014/diff/15001/chrome/browser/ui/...
chrome/browser/ui/startup/startup_browser_creator_impl.cc:396:
extensions::ExtensionSystem::Get(profile)->process_manager();
On 2013/09/04 22:17:35, Greg Spencer (Chromium) wrote:
> Your comment made me realize that I probably needed to surround this new piece
> with "#if defined(EXTENSIONS_ENABLED)", however, so I did that.

Err, I mean ENABLE_EXTENSIONS (which is what I really put in the code).

[Miranda Callahan, 2013-09-05 12:02:03]

profile_manager LGTM with spelling nit.

https://chromiumcodereview.appspot.com/23618014/diff/27001/chrome/browser/pro...
File chrome/browser/profiles/profile_manager.cc (right):

https://chromiumcodereview.appspot.com/23618014/diff/27001/chrome/browser/pro...
chrome/browser/profiles/profile_manager.cc:751:
"DeferBackgoundExtensionCreation",
nit: s/ackgo/ackgro

[Greg Spencer (Chromium), 2013-09-05 18:33:25]

https://chromiumcodereview.appspot.com/23618014/diff/27001/chrome/browser/pro...
File chrome/browser/profiles/profile_manager.cc (right):

https://chromiumcodereview.appspot.com/23618014/diff/27001/chrome/browser/pro...
chrome/browser/profiles/profile_manager.cc:751:
"DeferBackgoundExtensionCreation",
On 2013/09/05 12:02:03, Miranda Callahan wrote:
> nit: s/ackgo/ackgro

Done.

[sky, 2013-09-05 20:36:54]

LGTM

[commit-bot: I haz the power, 2013-09-05 20:39:20]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gspencer@chromium.org/23618014/32001

[commit-bot: I haz the power, 2013-09-05 21:16:14]

Sorry for I got bad news for ya.
Compile failed with a clobber build on linux_rel.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...
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, 2013-09-05 21:20:20]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gspencer@chromium.org/23618014/56001

[commit-bot: I haz the power, 2013-09-06 07:22:01]

Change committed as 221621

[James Cook, 2013-12-12 19:16:22]

On 2013/09/06 07:22:01, I haz the power (commit-bot) wrote:
> Change committed as 221621

FYI to future readers - reverted in https://codereview.chromium.org/111253003/