Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(169)

Issue 111253003: Remove DeferBackgroundExtensionCreation field trial and supporting code (Closed)

Created:
7 years ago by tapted
Modified:
7 years ago
CC:
chromium-reviews, tim+watch_chromium.org, extensions-reviews_chromium.org, haitaol+watch_chromium.org, rginda+watch_chromium.org, chromium-apps-reviews_chromium.org, rsimha+watch_chromium.org, maniscalco+watch_chromium.org, chrome-apps-syd-reviews_chromium.org
Visibility:
Public.

Description

Remove DeferBackgroundExtensionCreation field trial and supporting code. This is mostly just "Revert 221621 This defers starting background extension page RenderViews..." on trunk after resolving conflicts. r221621 assumes a browser is always created with StartupBrowserCreatorImpl. However, this is not true for incognito browsers, some command line switch options, nor some launch flows via the App Launcher. This meant that background extensions were sometimes never created. The field trial setup also needs to be revised, since it went out to 50x the users that is normal for a field trial on stable channel. Field trial is now disabled server-side in m31-33, but there should be enough data collected in m31 to make a call on whether to attempt this again with fixes. This reverts commit 736f456664be5e47df88bf8f936f40ddee299bd1. > 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 > > Review URL: https://chromiumcodereview.appspot.com/23618014 Some conflicts. Most due to c/b/e/chrome/browser/extensions/extension_process_manager.* moving to extensions/browser/process_manager.*, and some minor ones elsewhere. BUG=320787, 288259, 304453 R=mpcomplete@chromium.org, gspencer@chromium.org TBR=sky@chromium.org, rlp@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=240061

Patch Set 1 #

Patch Set 2 : extension_system.h header still needed for GetDisabledPlatformApp #

Unified diffs Side-by-side diffs Delta from patch set Stats (+19 lines, -95 lines) Patch
M chrome/browser/extensions/extension_system.h View 2 chunks +4 lines, -9 lines 0 comments Download
M chrome/browser/extensions/extension_system.cc View 2 chunks +1 line, -6 lines 0 comments Download
M chrome/browser/extensions/test_extension_system.h View 1 chunk +1 line, -2 lines 0 comments Download
M chrome/browser/profiles/profile_manager.cc View 2 chunks +1 line, -31 lines 0 comments Download
M chrome/browser/sync/glue/extension_data_type_controller.cc View 1 chunk +1 line, -2 lines 0 comments Download
M chrome/browser/sync/glue/extension_setting_data_type_controller.cc View 1 chunk +1 line, -2 lines 0 comments Download
M chrome/browser/sync/glue/theme_data_type_controller.cc View 1 chunk +1 line, -2 lines 0 comments Download
M chrome/browser/sync/test/integration/sync_app_helper.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/sync/test/integration/sync_extension_helper.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/startup/startup_browser_creator_impl.cc View 1 1 chunk +0 lines, -8 lines 0 comments Download
M extensions/browser/process_manager.h View 2 chunks +0 lines, -8 lines 0 comments Download
M extensions/browser/process_manager.cc View 5 chunks +6 lines, -22 lines 0 comments Download

Messages

Total messages: 7 (0 generated)
tapted
Hi Matt, this is just a revert, but it's been a while. Could you take ...
7 years ago (2013-12-10 06:50:13 UTC) #1
Matt Perry
gspencer: fyi lgtm
7 years ago (2013-12-10 18:05:53 UTC) #2
Greg Spencer (Chromium)
lgtm
7 years ago (2013-12-10 18:08:50 UTC) #3
tapted
TBR some owners since this is just a revert (although an old one). + sky ...
7 years ago (2013-12-11 06:43:24 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/tapted@chromium.org/111253003/20001
7 years ago (2013-12-11 06:44:10 UTC) #5
commit-bot: I haz the power
Change committed as 240061
7 years ago (2013-12-11 09:13:52 UTC) #6
achuithb
7 years ago (2013-12-17 19:52:05 UTC) #7
Message was sent while issue was closed.
On 2013/12/11 09:13:52, I haz the power (commit-bot) wrote:
> Change committed as 240061

I believe this also fixes
https://code.google.com/p/chromium/issues/detail?id=304442

Great work, Trent!

Powered by Google App Engine
This is Rietveld 408576698