Chromium Code Reviews
[email protected] (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(128)

Issue 604052: Use regular message loop for cros login window (Closed)

Created:
10 years, 10 months ago by DaveMoore
Modified:
10 years, 10 months ago
Reviewers:
sky
CC:
chromium-reviews_googlegroups.com, ben+cc_chromium.org, Dmitry Polukhin
Visibility:
Public.

Description

Use regular message loop for cros login window First we have to allow the regular initialization to happen. This will permit the login wizard screens to use the network and rendering capabilities of chrome safely. Once a user logs in and cros mounts their encrypted directory as a profile, we change the default profile to that one and create a new browser window with it as well. This ought to make launching the first browser window even faster.

Patch Set 1 #

Patch Set 2 : Put rewrite_dirs back #

Patch Set 3 : Put rewrite_dirs back #

Patch Set 4 : Cleanup #

Patch Set 5 : Added missing files #

Total comments: 14

Patch Set 6 : review changes #

Patch Set 7 : Removed extra file #

Unified diffs Side-by-side diffs Delta from patch set Stats (+252 lines, -66 lines) Patch
M chrome/browser/browser_init.h View 1 2 3 4 5 1 chunk +9 lines, -0 lines 0 comments Download
M chrome/browser/browser_init.cc View 1 2 3 4 5 4 chunks +39 lines, -32 lines 0 comments Download
M chrome/browser/browser_main.cc View 1 2 3 1 chunk +17 lines, -2 lines 0 comments Download
M chrome/browser/chromeos/login/login_manager_view.cc View 1 2 3 4 5 2 chunks +23 lines, -0 lines 0 comments Download
M chrome/browser/chromeos/login/login_wizard_view.h View 1 2 3 4 5 3 chunks +7 lines, -2 lines 0 comments Download
M chrome/browser/chromeos/login/login_wizard_view.cc View 1 2 3 4 5 8 chunks +18 lines, -16 lines 0 comments Download
A chrome/browser/chromeos/login/test_renderer_screen.h View 5 1 chunk +26 lines, -0 lines 0 comments Download
A chrome/browser/chromeos/login/test_renderer_screen.cc View 1 chunk +34 lines, -0 lines 0 comments Download
M chrome/browser/chromeos/login/user_manager.cc View 2 chunks +7 lines, -0 lines 0 comments Download
M chrome/browser/profile_manager.h View 1 2 3 4 5 4 chunks +16 lines, -1 line 0 comments Download
M chrome/browser/profile_manager.cc View 1 2 3 4 5 5 chunks +38 lines, -10 lines 0 comments Download
M chrome/browser/views/browser_dialogs.h View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
M chrome/chrome_browser.gypi View 1 2 3 4 5 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/common/chrome_switches.h View 1 2 3 4 5 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/common/chrome_switches.cc View 1 2 3 4 5 2 chunks +8 lines, -2 lines 0 comments Download
M chrome/common/notification_type.h View 1 2 3 4 5 1 chunk +5 lines, -0 lines 0 comments Download

Messages

Total messages: 2 (0 generated)
DaveMoore
10 years, 10 months ago (2010-02-15 04:25:18 UTC) #1
sky
10 years, 10 months ago (2010-02-16 19:22:40 UTC) #2
LGTM with the following changes.

http://codereview.chromium.org/604052/diff/1044/1045
File chrome/browser/browser_init.cc (right):

http://codereview.chromium.org/604052/diff/1044/1045#newcode910
chrome/browser/browser_init.cc:910: if
(command_line.HasSwitch(switches::kLoginManager))
Can you add a comment here indicating that when using the login manager we
launch after login.

http://codereview.chromium.org/604052/diff/1044/1046
File chrome/browser/browser_init.h (right):

http://codereview.chromium.org/604052/diff/1044/1046#newcode66
chrome/browser/browser_init.h:66: // empty. process_startup indicates whether
this is the first browser.
nit: in descriptions use pipes around arguments.

http://codereview.chromium.org/604052/diff/1044/1046#newcode66
chrome/browser/browser_init.h:66: // empty. process_startup indicates whether
this is the first browser.
What does an empty cur_dir mean?

http://codereview.chromium.org/604052/diff/1044/1049
File chrome/browser/chromeos/login/login_wizard_view.cc (right):

http://codereview.chromium.org/604052/diff/1044/1049#newcode99
chrome/browser/chromeos/login/login_wizard_view.cc:99: const std::string
start_screen_name, gfx::Size size) {
const gfx::Size& and it should be const std::string& too.

http://codereview.chromium.org/604052/diff/1044/1049#newcode132
chrome/browser/chromeos/login/login_wizard_view.cc:132: void
ShowLoginWizard(const std::string& start_screen_name, gfx::Size size) {
const&

http://codereview.chromium.org/604052/diff/1044/1050
File chrome/browser/chromeos/login/login_wizard_view.h (right):

http://codereview.chromium.org/604052/diff/1044/1050#newcode36
chrome/browser/chromeos/login/login_wizard_view.h:36: void Init(const
std::string& start_view_name, gfx::Size size);
const gfx::Size&

http://codereview.chromium.org/604052/diff/1044/1050#newcode85
chrome/browser/chromeos/login/login_wizard_view.h:85: // todo(davemoore) Remove
this after done.
// TODO(davemoore):

http://codereview.chromium.org/604052/diff/1044/1052
File chrome/browser/chromeos/login/test_renderer_screen.h (right):

http://codereview.chromium.org/604052/diff/1044/1052#newcode14
chrome/browser/chromeos/login/test_renderer_screen.h:14: class
TestRendererScreen : public WizardScreen {
Should this be in the chromeos namespace too?

http://codereview.chromium.org/604052/diff/1044/1052#newcode22
chrome/browser/chromeos/login/test_renderer_screen.h:22: };
DISALLOW_COPY_AND_ASSIGN

http://codereview.chromium.org/604052/diff/1044/1054
File chrome/browser/profile_manager.cc (right):

http://codereview.chromium.org/604052/diff/1044/1054#newcode89
chrome/browser/profile_manager.cc:89: // todo(davemoore) Delete this once
chromium os has started using
// TODO(davemoore):

http://codereview.chromium.org/604052/diff/1044/1055
File chrome/browser/profile_manager.h (right):

http://codereview.chromium.org/604052/diff/1044/1055#newcode155
chrome/browser/profile_manager.h:155: bool logged_in_;
Description?

http://codereview.chromium.org/604052/diff/1044/1056
File chrome/browser/views/browser_dialogs.h (right):

http://codereview.chromium.org/604052/diff/1044/1056#newcode96
chrome/browser/views/browser_dialogs.h:96: void ShowLoginWizard(const
std::string& start_screen, gfx::Size size);
const gfx::Size&

http://codereview.chromium.org/604052/diff/1044/1058
File chrome/common/chrome_switches.cc (right):

http://codereview.chromium.org/604052/diff/1044/1058#newcode741
chrome/common/chrome_switches.cc:741: // todo(davemoore) Delete this once
chromeos has started using
// TODO(davemoore):

http://codereview.chromium.org/604052/diff/1044/1060
File chrome/common/notification_type.h (right):

http://codereview.chromium.org/604052/diff/1044/1060#newcode851
chrome/common/notification_type.h:851: LOGIN_USER_CHANGED,
Description?

Powered by Google App Engine
This is Rietveld 408576698