Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(1)

Issue 2697063004: Fix of "login is not defined" error in OOBE (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 months ago by Wenzhao (Colin) Zang
Modified:
2 months, 3 weeks ago
Reviewers:
Alexander Alekseev
CC:
chromium-reviews, alemate+watch_chromium.org, achuith+watch_chromium.org, arv+watch_chromium.org, oshima+watch_chromium.org, Rahul Chaturvedi
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Fix of "login is not defined" error in OOBE The cause of the error is that by the time OnTouchViewToggled in SigninScreenHandler is called, oobe.js may not have completed intialization yet, so we should wait until the JS message is received. BUG=692751 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2697063004 Cr-Commit-Position: refs/heads/master@{#454111} Committed: https://chromium.googlesource.com/chromium/src/+/3dc4062eddbbe88c416a21bd92986645ddb43b50

Patch Set 1 #

Total comments: 3

Patch Set 2 : Move CallJSOrDefer to OobeUI #

Total comments: 2

Patch Set 3 : Move deferred JS calls storage to a separate class #

Patch Set 4 : Merge #

Unified diffs Side-by-side diffs Delta from patch set Stats (+104 lines, -69 lines) Patch
M chrome/browser/ui/webui/chromeos/login/base_screen_handler.h View 1 2 4 chunks +64 lines, -0 lines 0 comments Download
M chrome/browser/ui/webui/chromeos/login/base_screen_handler.cc View 1 2 2 chunks +15 lines, -0 lines 0 comments Download
M chrome/browser/ui/webui/chromeos/login/core_oobe_handler.h View 1 2 3 chunks +2 lines, -28 lines 0 comments Download
M chrome/browser/ui/webui/chromeos/login/core_oobe_handler.cc View 1 2 3 3 chunks +6 lines, -35 lines 0 comments Download
M chrome/browser/ui/webui/chromeos/login/oobe_ui.h View 1 2 1 chunk +4 lines, -0 lines 0 comments Download
M chrome/browser/ui/webui/chromeos/login/oobe_ui.cc View 1 2 3 2 chunks +5 lines, -2 lines 0 comments Download
M chrome/browser/ui/webui/chromeos/login/signin_screen_handler.h View 1 2 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc View 1 2 3 3 chunks +6 lines, -3 lines 0 comments Download
Commit queue not available (can’t edit this change).

Messages

Total messages: 31 (15 generated)
Wenzhao (Colin) Zang
3 months ago (2017-02-17 03:35:13 UTC) #3
Alexander Alekseev
https://codereview.chromium.org/2697063004/diff/1/chrome/browser/resources/chromeos/login/oobe.js File chrome/browser/resources/chromeos/login/oobe.js (right): https://codereview.chromium.org/2697063004/diff/1/chrome/browser/resources/chromeos/login/oobe.js#newcode144 chrome/browser/resources/chromeos/login/oobe.js:144: chrome.send('screenStateInitializeForSignin'); We try to avoid duplicating events, as this ...
3 months ago (2017-02-17 03:56:38 UTC) #4
Wenzhao (Colin) Zang
https://codereview.chromium.org/2697063004/diff/1/chrome/browser/resources/chromeos/login/oobe.js File chrome/browser/resources/chromeos/login/oobe.js (right): https://codereview.chromium.org/2697063004/diff/1/chrome/browser/resources/chromeos/login/oobe.js#newcode144 chrome/browser/resources/chromeos/login/oobe.js:144: chrome.send('screenStateInitializeForSignin'); In order to replace all CallJS with CallJSOrDefer ...
3 months ago (2017-02-17 19:51:56 UTC) #9
Wenzhao (Colin) Zang
https://codereview.chromium.org/2697063004/diff/1/chrome/browser/resources/chromeos/login/oobe.js File chrome/browser/resources/chromeos/login/oobe.js (right): https://codereview.chromium.org/2697063004/diff/1/chrome/browser/resources/chromeos/login/oobe.js#newcode144 chrome/browser/resources/chromeos/login/oobe.js:144: chrome.send('screenStateInitializeForSignin'); We might need to separate CallJSOrDefer into a ...
3 months ago (2017-02-17 22:58:16 UTC) #10
Wenzhao (Colin) Zang
Move CallJSOrDefer to OobeUI
2 months, 4 weeks ago (2017-02-21 23:35:04 UTC) #12
Alexander Alekseev
https://codereview.chromium.org/2697063004/diff/20001/chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc File chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc (right): https://codereview.chromium.org/2697063004/diff/20001/chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc#newcode1131 chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc:1131: GetOobeUI()->CallJSOrDefer("login.AccountPickerScreen.setTouchViewState", GetOobeUI() is a member of BaseScreenHandler. This means ...
2 months, 4 weeks ago (2017-02-21 23:46:49 UTC) #13
Wenzhao (Colin) Zang
https://codereview.chromium.org/2697063004/diff/20001/chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc File chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc (right): https://codereview.chromium.org/2697063004/diff/20001/chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc#newcode1131 chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc:1131: GetOobeUI()->CallJSOrDefer("login.AccountPickerScreen.setTouchViewState", On 2017/02/21 23:46:49, Alexander Alekseev wrote: > GetOobeUI() ...
2 months, 4 weeks ago (2017-02-22 02:53:34 UTC) #14
Alexander Alekseev
On 2017/02/22 02:53:34, Wenzhao (Colin) Zang wrote: > https://codereview.chromium.org/2697063004/diff/20001/chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc > File chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc (right): > > ...
2 months, 3 weeks ago (2017-03-01 09:01:02 UTC) #15
Wenzhao (Colin) Zang
I put the JS calls storage and the is_initialized bool to a separate helper class. ...
2 months, 3 weeks ago (2017-03-01 21:42:32 UTC) #16
Alexander Alekseev
lgtm Thank you!
2 months, 3 weeks ago (2017-03-01 21:58:15 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2697063004/40001
2 months, 3 weeks ago (2017-03-01 22:07:11 UTC) #19
commit-bot: I haz the power
Failed to apply patch for chrome/browser/ui/webui/chromeos/login/core_oobe_handler.cc: While running git apply --index -p1; error: patch failed: ...
2 months, 3 weeks ago (2017-03-01 22:56:36 UTC) #21
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2697063004/40001
2 months, 3 weeks ago (2017-03-01 22:59:37 UTC) #23
commit-bot: I haz the power
Failed to apply patch for chrome/browser/ui/webui/chromeos/login/core_oobe_handler.cc: While running git apply --index -p1; error: patch failed: ...
2 months, 3 weeks ago (2017-03-01 23:07:27 UTC) #25
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2697063004/60001
2 months, 3 weeks ago (2017-03-01 23:36:56 UTC) #28
commit-bot: I haz the power
2 months, 3 weeks ago (2017-03-02 00:12:35 UTC) #31
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/3dc4062eddbbe88c416a21bd9298...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 650457f06