Issue 223462 in chromium: Expose more master preferences test cases to first-run in browser_tests
3 views
Skip to first unread message
chro...@googlecode.com
Mar 24, 2013, 6:48:19 PM3/24/13
to chromi...@chromium.org
Status: Assigned
Owner: tap...@chromium.org
CC: g...@chromium.org
Labels: M-28 Pri-3 Cr-Tests OS-All Cr-UI-Browser-FirstRun
Cr-UI-Browser-Import
New issue 223462 by tap...@chromium.org: Expose more master preferences
test cases to first-run in browser_tests
http://code.google.com/p/chromium/issues/detail?id=223462
Version: ~ 26.0.1410.40
OS: All
E.g. from https://codereview.chromium.org/12330073/,
On 2013/03/07 22:57:18, gab wrote:
> On 2013/03/07 22:49:41, gab wrote:
> > What bothers me is that the import process probably needs more services
> if
> other
> > import preferences are present (i.e. import_bookmarks,
> > import_bookmarks_from_file, import_history, import_home_page, and
> > import_search_engine).
> >
> > In fact we should probably add a FirstRunBrowserTest to test the
> different
> > import preferences (currently the only tests are (1) default prefs and
> (2)
all
> > prefs == false).
> Actually, tapted has a TODO to do just that I think
(
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/first_run/first_run_browsertest.cc&q=firstrunbrow&sq=package:chromium&l=94
),
> would be nice to get those tests up before landing this (especially if we
> want
> to merge this in M26...).
> Trent could you look into that please?
Yep - I'll check it out. That TODO has been an itch I've been meaning to
scratch
for a while..
--
You received this message because this project is configured to send all
issue notifications to this address.
You may adjust your notification preferences at:
https://code.google.com/hosting/settings
Owner: tap...@chromium.org
CC: g...@chromium.org
Labels: M-28 Pri-3 Cr-Tests OS-All Cr-UI-Browser-FirstRun
Cr-UI-Browser-Import
New issue 223462 by tap...@chromium.org: Expose more master preferences
test cases to first-run in browser_tests
http://code.google.com/p/chromium/issues/detail?id=223462
Version: ~ 26.0.1410.40
OS: All
E.g. from https://codereview.chromium.org/12330073/,
On 2013/03/07 22:57:18, gab wrote:
> On 2013/03/07 22:49:41, gab wrote:
> > What bothers me is that the import process probably needs more services
> if
> other
> > import preferences are present (i.e. import_bookmarks,
> > import_bookmarks_from_file, import_history, import_home_page, and
> > import_search_engine).
> >
> > In fact we should probably add a FirstRunBrowserTest to test the
> different
> > import preferences (currently the only tests are (1) default prefs and
> (2)
all
> > prefs == false).
> Actually, tapted has a TODO to do just that I think
(
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/first_run/first_run_browsertest.cc&q=firstrunbrow&sq=package:chromium&l=94
),
> would be nice to get those tests up before landing this (especially if we
> want
> to merge this in M26...).
> Trent could you look into that please?
Yep - I'll check it out. That TODO has been an itch I've been meaning to
scratch
for a while..
--
You received this message because this project is configured to send all
issue notifications to this address.
You may adjust your notification preferences at:
https://code.google.com/hosting/settings
chro...@googlecode.com
Apr 25, 2013, 5:41:18 PM4/25/13
to chromi...@chromium.org
Updates:
Status: Started
Owner: g...@chromium.org
Cc: -g...@chromium.org tap...@chromium.org
Comment #1 on issue 223462 by g...@chromium.org: Expose more master
Status: Started
Owner: g...@chromium.org
Cc: -g...@chromium.org tap...@chromium.org
Comment #1 on issue 223462 by g...@chromium.org: Expose more master
preferences test cases to first-run in browser_tests
http://code.google.com/p/chromium/issues/detail?id=223462
https://codereview.chromium.org/12670013/ will take care of this too :)
http://code.google.com/p/chromium/issues/detail?id=223462
chro...@googlecode.com
May 23, 2013, 3:29:10 PM5/23/13
to chromi...@chromium.org
Comment #3 on issue 223462 by bugdro...@chromium.org: Expose more master
preferences test cases to first-run in browser_tests
http://code.google.com/p/chromium/issues/detail?id=223462#c3
------------------------------------------------------------------------
r201837 | g...@chromium.org | 2013-05-23T18:49:45.570857Z
Changed paths:
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/sync/profile_signin_confirmation_helper_browsertest.cc?r1=201837&r2=201836&pathrev=201837
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/profiles/profile_manager.cc?r1=201837&r2=201836&pathrev=201837
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/importer/external_process_importer_bridge.cc?r1=201837&r2=201836&pathrev=201837
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/sync/profile_sync_service_factory.cc?r1=201837&r2=201836&pathrev=201837
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/profiles/profile_manager.h?r1=201837&r2=201836&pathrev=201837
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/extensions/extension_system.cc?r1=201837&r2=201836&pathrev=201837
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/webui/options/import_data_handler.cc?r1=201837&r2=201836&pathrev=201837
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/profiles/profile.cc?r1=201837&r2=201836&pathrev=201837
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/chrome_browser_main.cc?r1=201837&r2=201836&pathrev=201837
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/first_run/first_run_internal.h?r1=201837&r2=201836&pathrev=201837
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/common/switch_utils_unittest.cc?r1=201837&r2=201836&pathrev=201837
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/common/switch_utils.cc?r1=201837&r2=201836&pathrev=201837
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/first_run/first_run.cc?r1=201837&r2=201836&pathrev=201837
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/importer/firefox_importer_browsertest.cc?r1=201837&r2=201836&pathrev=201837
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/first_run/first_run.h?r1=201837&r2=201836&pathrev=201837
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/first_run/first_run_browsertest.cc?r1=201837&r2=201836&pathrev=201837
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/cloud/user_policy_signin_service.cc?r1=201837&r2=201836&pathrev=201837
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/first_run/first_run_mac.mm?r1=201837&r2=201836&pathrev=201837
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/extensions/extension_service.cc?r1=201837&r2=201836&pathrev=201837
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/common/chrome_switches.cc?r1=201837&r2=201836&pathrev=201837
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/common/chrome_switches.h?r1=201837&r2=201836&pathrev=201837
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/importer/ie_importer_browsertest_win.cc?r1=201837&r2=201836&pathrev=201837
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/first_run/first_run_win.cc?r1=201837&r2=201836&pathrev=201837
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/first_run/first_run_linux.cc?r1=201837&r2=201836&pathrev=201837
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/utility/profile_import_handler.cc?r1=201837&r2=201836&pathrev=201837
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/first_run/first_run_posix.cc?r1=201837&r2=201836&pathrev=201837
OOP import on Windows.
Gets rid of the import process on all platforms -- replaced by a utility
process which communicates with a ProfileWriter back in the browser process
to write to the profile (this uses the ExternalProcessImporterHost
machinery written 2+ years ago by mirandac@ for import on Mac and still the
state of the art today).
Gets rid of all issues regarding profile contention and races to profile
creation between the browser and import processes on first run (example of
issues this has previously caused: http://crbug.com/171475,
http://crbug.com/174591, http://crbug.com/180459).
Makes bookmarks file import use the same mechanism on all platforms (this
means bookmarks file import is now in-process on Linux which still uses
ImporterHost (instead of ExternalProcessImporterHost) -- Linux used to use
the import process solely for bookmarks file import -- but the work to
switch Linux to ExternalProcessImporterHost should be very minimal after
this CL and I intend to do it in a follow-up CL to unify the import flow
cross-platform once and for all!).
Do not use the out-of-process import for Google Toolbar (this was already
the case prior to this CL).
To make the Google Toolbar importer work out-of-process, we would have to
augment the import IPC drastically to support the web auth flow required by
this importer (it requires to login to import the google.com/bookmarks
favorites).
This, as a side-effect, brings silent bookmarks file import from
master_preferences to Mac (long standing issue 48880).
Also fixes issue 231710 (or at least removes the condition causing the bug
by making the ImportLockDialog go away on first run on Windows -- as should
already have been the case).
Also addresses issue 178083 since the early message loop spinning was
caused by ImportSettingsWin which was called too early on Windows (actually
resulting in running the full import twice on Windows!) -- via
PreCreateThreadsImpl()-->ProcessMasterPreferences()-->SetImportPreferencesAndLaunchImport()-->ImportSettingsWin()...
This whole flow is removed in this CL :).
This improves first run speed in a debug build from 4901ms to 1477ms, a
332% improvement!!!! (tested by instrumenting a first run browser test, the
delta is between the time the test is constructed and the time the test
case is called (which happens after the browser has been initialized and
import has occurred)).
This supersedes https://codereview.chromium.org/12463030/ (which won't be
committed because this fix is so much better).
BUG=219419, 22142, 56816, 178083, 178051, 48880, 232241, 231710, 223462,
87657, 236225
Review URL: https://chromiumcodereview.appspot.com/12670013
------------------------------------------------------------------------
chro...@googlecode.com
May 23, 2013, 3:45:10 PM5/23/13
to chromi...@chromium.org
Updates:
Status: Fixed
Comment #4 on issue 223462 by g...@chromium.org: Expose more master
Status: Fixed
Comment #4 on issue 223462 by g...@chromium.org: Expose more master
preferences test cases to first-run in browser_tests
http://code.google.com/p/chromium/issues/detail?id=223462
Shall we say that this is fixed now?
http://code.google.com/p/chromium/issues/detail?id=223462
chro...@googlecode.com
May 23, 2013, 7:50:44 PM5/23/13
to chromi...@chromium.org
Comment #5 on issue 223462 by tap...@chromium.org: Expose more master
preferences test cases to first-run in browser_tests
http://code.google.com/p/chromium/issues/detail?id=223462
sgtm. With the better process isolation, the master prefs unit tests become
http://code.google.com/p/chromium/issues/detail?id=223462
more compelling, and you've laid a good framework for adding additional
corner cases to browser tests if they're needed.
chro...@googlecode.com
May 23, 2013, 11:48:50 PM5/23/13
to chromi...@chromium.org
Comment #6 on issue 223462 by bugdro...@chromium.org: Expose more master
preferences test cases to first-run in browser_tests
http://code.google.com/p/chromium/issues/detail?id=223462#c6
------------------------------------------------------------------------
r201968 | db...@chromium.org | 2013-05-24T03:35:40.200987Z
Changed paths:
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/utility/profile_import_handler.cc?r1=201968&r2=201967&pathrev=201968
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/first_run/first_run_posix.cc?r1=201968&r2=201967&pathrev=201968
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/sync/profile_signin_confirmation_helper_browsertest.cc?r1=201968&r2=201967&pathrev=201968
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/profiles/profile_manager.cc?r1=201968&r2=201967&pathrev=201968
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/importer/external_process_importer_bridge.cc?r1=201968&r2=201967&pathrev=201968
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/extensions/extension_system.cc?r1=201968&r2=201967&pathrev=201968
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/profiles/profile_manager.h?r1=201968&r2=201967&pathrev=201968
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/sync/profile_sync_service_factory.cc?r1=201968&r2=201967&pathrev=201968
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/profiles/profile.cc?r1=201968&r2=201967&pathrev=201968
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/webui/options/import_data_handler.cc?r1=201968&r2=201967&pathrev=201968
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/first_run/first_run_internal.h?r1=201968&r2=201967&pathrev=201968
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/chrome_browser_main.cc?r1=201968&r2=201967&pathrev=201968
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/common/switch_utils_unittest.cc?r1=201968&r2=201967&pathrev=201968
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/common/switch_utils.cc?r1=201968&r2=201967&pathrev=201968
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/first_run/first_run.cc?r1=201968&r2=201967&pathrev=201968
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/importer/firefox_importer_browsertest.cc?r1=201968&r2=201967&pathrev=201968
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/first_run/first_run.h?r1=201968&r2=201967&pathrev=201968
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/first_run/first_run_browsertest.cc?r1=201968&r2=201967&pathrev=201968
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/cloud/user_policy_signin_service.cc?r1=201968&r2=201967&pathrev=201968
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/first_run/first_run_mac.mm?r1=201968&r2=201967&pathrev=201968
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/common/chrome_switches.cc?r1=201968&r2=201967&pathrev=201968
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/extensions/extension_service.cc?r1=201968&r2=201967&pathrev=201968
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/common/chrome_switches.h?r1=201968&r2=201967&pathrev=201968
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/importer/ie_importer_browsertest_win.cc?r1=201968&r2=201967&pathrev=201968
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/first_run/first_run_win.cc?r1=201968&r2=201967&pathrev=201968
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/first_run/first_run_linux.cc?r1=201968&r2=201967&pathrev=201968
Revert 201837 "OOP import on Windows."
This broke browser_tests on XP Tests (dbg)(4):
http://build.chromium.org/p/chromium.win/builders/XP%20Tests%20%28dbg%29%284%29/builds/33240
[1148:3528:0523/123340:2643015:WARNING:extension_apitest.cc(169)]
Workaround for 177163, prematurely stopping test
Review URL: https://codereview.chromium.org/15968002
------------------------------------------------------------------------
chro...@googlecode.com
May 24, 2013, 2:21:45 AM5/24/13
to chromi...@chromium.org
Comment #7 on issue 223462 by bugdro...@chromium.org: Expose more master
preferences test cases to first-run in browser_tests
http://code.google.com/p/chromium/issues/detail?id=223462#c7
------------------------------------------------------------------------
r201977 | db...@chromium.org | 2013-05-24T06:14:41.697719Z
Changed paths:
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/webui/options/import_data_handler.cc?r1=201977&r2=201976&pathrev=201977
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/profiles/profile.cc?r1=201977&r2=201976&pathrev=201977
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/chrome_browser_main.cc?r1=201977&r2=201976&pathrev=201977
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/first_run/first_run_internal.h?r1=201977&r2=201976&pathrev=201977
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/common/switch_utils_unittest.cc?r1=201977&r2=201976&pathrev=201977
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/common/switch_utils.cc?r1=201977&r2=201976&pathrev=201977
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/first_run/first_run.cc?r1=201977&r2=201976&pathrev=201977
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/importer/firefox_importer_browsertest.cc?r1=201977&r2=201976&pathrev=201977
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/first_run/first_run.h?r1=201977&r2=201976&pathrev=201977
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/policy/cloud/user_policy_signin_service.cc?r1=201977&r2=201976&pathrev=201977
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/first_run/first_run_browsertest.cc?r1=201977&r2=201976&pathrev=201977
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/first_run/first_run_mac.mm?r1=201977&r2=201976&pathrev=201977
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/extensions/extension_service.cc?r1=201977&r2=201976&pathrev=201977
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/common/chrome_switches.cc?r1=201977&r2=201976&pathrev=201977
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/common/chrome_switches.h?r1=201977&r2=201976&pathrev=201977
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/importer/ie_importer_browsertest_win.cc?r1=201977&r2=201976&pathrev=201977
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/first_run/first_run_win.cc?r1=201977&r2=201976&pathrev=201977
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/first_run/first_run_linux.cc?r1=201977&r2=201976&pathrev=201977
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/utility/profile_import_handler.cc?r1=201977&r2=201976&pathrev=201977
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/first_run/first_run_posix.cc?r1=201977&r2=201976&pathrev=201977
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/sync/profile_signin_confirmation_helper_browsertest.cc?r1=201977&r2=201976&pathrev=201977
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/profiles/profile_manager.cc?r1=201977&r2=201976&pathrev=201977
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/importer/external_process_importer_bridge.cc?r1=201977&r2=201976&pathrev=201977
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/sync/profile_sync_service_factory.cc?r1=201977&r2=201976&pathrev=201977
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/profiles/profile_manager.h?r1=201977&r2=201976&pathrev=201977
M
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/extensions/extension_system.cc?r1=201977&r2=201976&pathrev=201977
Revert 201968 "Revert 201837 "OOP import on Windows.""
Sorry about that, seems to be a different issue - http://crbug.com/177163
Review URL: https://codereview.chromium.org/15876003
Reply all
Reply to author
Forward
0 new messages