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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 8395042: [cros,de-hack] Get rid of singleton for the WebUILoginScreen. (Closed)

Created:
9 years, 1 month ago by altimofeev
Modified:
9 years, 1 month ago
Reviewers:
Nikita (slow)
CC:
chromium-reviews, nkostylev+watch_chromium.org, kkania, arv (Not doing code reviews), robertshield, Paweł Hajdan Jr., stevenjb+watch_chromium.org, davemoore+watch_chromium.org
Base URL:
http://git.chromium.org/chromium/src.git@master
Visibility:
Public.

Description

[cros,de-hack] Get rid of singleton for the WebUILoginScreen. After refactoring WebUILoginScreen is owned by ExistingUserController. Also, all users of GetLoginWindow() switched to use LoginDisplayHost::GetNativeWindow() instead. Views code from BaseLoginDisplayHost was moved to ViewsLoginDisplayHost. BUG=chromium-os:21054, chromium-os:20946 TEST=unit_tests,browser_tests Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=108077

Patch Set 1 #

Patch Set 2 : nits #

Patch Set 3 : also fix 20946 #

Patch Set 4 : clang fix #

Total comments: 6

Patch Set 5 : merged #

Patch Set 6 : code review #

Patch Set 7 : comment nit #

Unified diffs Side-by-side diffs Delta from patch set Stats (+144 lines, -189 lines) Patch
M chrome/browser/automation/testing_automation_provider_chromeos.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/chromeos/login/base_login_display_host.h View 1 2 3 chunks +2 lines, -8 lines 0 comments Download
M chrome/browser/chromeos/login/base_login_display_host.cc View 1 2 3 4 4 chunks +1 line, -22 lines 0 comments Download
M chrome/browser/chromeos/login/existing_user_controller.h View 1 3 chunks +5 lines, -6 lines 0 comments Download
M chrome/browser/chromeos/login/existing_user_controller.cc View 1 2 3 4 3 chunks +2 lines, -5 lines 0 comments Download
M chrome/browser/chromeos/login/existing_user_controller_browsertest.cc View 1 2 3 4 1 chunk +1 line, -2 lines 0 comments Download
M chrome/browser/chromeos/login/login_display.h View 1 chunk +0 lines, -4 lines 0 comments Download
M chrome/browser/chromeos/login/login_display.cc View 1 chunk +0 lines, -4 lines 0 comments Download
M chrome/browser/chromeos/login/login_display_host.h View 1 2 3 4 5 6 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/chromeos/login/screen_locker_webui.cc View 1 2 3 5 chunks +6 lines, -14 lines 0 comments Download
M chrome/browser/chromeos/login/views_login_display_host.h View 1 2 3 chunks +10 lines, -2 lines 0 comments Download
M chrome/browser/chromeos/login/views_login_display_host.cc View 1 2 3 chunks +17 lines, -1 line 0 comments Download
M chrome/browser/chromeos/login/webui_login_display.h View 1 4 chunks +4 lines, -51 lines 0 comments Download
M chrome/browser/chromeos/login/webui_login_display.cc View 2 chunks +14 lines, -27 lines 0 comments Download
M chrome/browser/chromeos/login/webui_login_display_host.h View 3 chunks +6 lines, -2 lines 0 comments Download
M chrome/browser/chromeos/login/webui_login_display_host.cc View 1 2 5 chunks +8 lines, -9 lines 0 comments Download
M chrome/browser/chromeos/login/webui_login_view.h View 1 3 chunks +4 lines, -4 lines 0 comments Download
M chrome/browser/chromeos/login/webui_login_view.cc View 1 2 3 4 5 5 chunks +4 lines, -8 lines 0 comments Download
M chrome/browser/chromeos/setting_level_bubble.cc View 1 2 3 4 5 2 chunks +16 lines, -4 lines 0 comments Download
M chrome/browser/ui/webui/chromeos/login/base_screen_handler.h View 1 2 3 4 5 2 chunks +4 lines, -0 lines 0 comments Download
M chrome/browser/ui/webui/chromeos/login/base_screen_handler.cc View 2 chunks +6 lines, -0 lines 0 comments Download
M chrome/browser/ui/webui/chromeos/login/eula_screen_handler.cc View 1 chunk +1 line, -2 lines 0 comments Download
M chrome/browser/ui/webui/chromeos/login/network_dropdown_handler.cc View 1 chunk +1 line, -2 lines 0 comments Download
M chrome/browser/ui/webui/chromeos/login/oobe_ui.h View 2 chunks +2 lines, -1 line 0 comments Download
M chrome/browser/ui/webui/chromeos/login/oobe_ui.cc View 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/ui/webui/chromeos/login/signin_screen_handler.h View 3 chunks +14 lines, -0 lines 0 comments Download
M chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc View 1 2 3 4 4 chunks +10 lines, -6 lines 0 comments Download
M chrome/browser/ui/webui/chromeos/login/user_image_screen_handler.cc View 1 chunk +1 line, -2 lines 0 comments Download

Messages

Total messages: 10 (0 generated)
altimofeev
9 years, 1 month ago (2011-10-28 14:40:40 UTC) #1
Nikita (slow)
lgtm http://codereview.chromium.org/8395042/diff/2003/chrome/browser/chromeos/login/webui_login_display_host.h File chrome/browser/chromeos/login/webui_login_display_host.h (right): http://codereview.chromium.org/8395042/diff/2003/chrome/browser/chromeos/login/webui_login_display_host.h#newcode24 chrome/browser/chromeos/login/webui_login_display_host.h:24: // WebUILoginDisplay as the login screen UI implementation, ...
9 years, 1 month ago (2011-10-31 11:14:18 UTC) #2
Nikita (slow)
lgtm
9 years, 1 month ago (2011-10-31 11:14:19 UTC) #3
altimofeev
http://codereview.chromium.org/8395042/diff/2003/chrome/browser/chromeos/login/webui_login_display_host.h File chrome/browser/chromeos/login/webui_login_display_host.h (right): http://codereview.chromium.org/8395042/diff/2003/chrome/browser/chromeos/login/webui_login_display_host.h#newcode24 chrome/browser/chromeos/login/webui_login_display_host.h:24: // WebUILoginDisplay as the login screen UI implementation, On ...
9 years, 1 month ago (2011-10-31 16:55:51 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/altimofeev@chromium.org/8395042/17002
9 years, 1 month ago (2011-10-31 18:57:36 UTC) #5
commit-bot: I haz the power
Commit queue rejected this change because the description was changed between the time the change ...
9 years, 1 month ago (2011-10-31 20:05:35 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/altimofeev@chromium.org/8395042/17002
9 years, 1 month ago (2011-11-01 05:31:18 UTC) #7
commit-bot: I haz the power
Change committed as 108077
9 years, 1 month ago (2011-11-01 07:12:25 UTC) #8
cwolfe
On 2011/11/01 07:12:25, I haz the power (commit-bot) wrote: > Change committed as 108077 This ...
9 years, 1 month ago (2011-11-01 16:57:59 UTC) #9
altimofeev
9 years, 1 month ago (2011-11-01 17:12:12 UTC) #10 
Sorry about it. I've already created a fix:
http://codereview.chromium.org/8423008/

On 2011/11/01 16:57:59, cwolfe wrote:
> On 2011/11/01 07:12:25, I haz the power (commit-bot) wrote:
> > Change committed as 108077
> 
> This refactoring missed a CHROME_OS+TOUCH_UI client in
> src/chrome/browser/extensions/extension_input_api.cc. We have several e-mails
in
> flight, so the problem will hopefully be fixed before rolling downstream.

Powered by Google App Engine
This is Rietveld 408576698