New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 844020 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 9
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug

Blocked on:
issue 828058


Participants' hotlists:
LoginRefresh


Sign in to add a comment

Chrome crashes on login when migration is needed.

Project Member Reported by fukino@chromium.org, May 17 2018

Issue description

Chrome Version: ToT
OS: Chrome

What steps will reproduce the problem?
(1) Prepare a profile whose user directory is created with ecryptfs.
(2) Upgrade the device which supports ext4 crypto.
(3) Try to sign in with the profile created in step 1.

What is the expected result?
Migration UI is presented.

What happens instead?
Chrome crashes and sign-in UI falls back to WebUI version
 

Comment 1 by fukino@chromium.org, May 17 2018

Labels: OS-Chrome
When old encryption is detected, ExistingUserController tries to get WizardController instance to set up migration screen.
https://codesearch.chromium.org/chromium/src/chrome/browser/chromeos/login/existing_user_controller.cc?rcl=211e27c281bdd7c94d235011b07421904e6d48be&l=791

With the view-based login UI, GetWizardController() is not implemented.
An alternative way to setup the screen is needed.

Comment 2 by xiy...@chromium.org, May 17 2018

Check out the Gaia dialog impl that shows Gaia screen in a dialog. Maybe we can generalize it to show any WebUI login screen.

https://cs.chromium.org/chromium/src/chrome/browser/chromeos/login/ui/gaia_dialog_delegate.cc

Comment 3 by fukino@chromium.org, May 18 2018

Blockedon: 828058

Comment 4 by fukino@chromium.org, May 18 2018

Hi xiyuan@, jdufault@,

Is it possible to ask you to handle this issue?
I'm not sure if I can make it for M-68 branch point since I'm not familiar with the overall architecture of the new views-based login and I'm still not sure how this issue should be fixed. (e.g. I guess LoginDisplayHostMojo::GetWizardController() is not implemented intentionally, but I'm still not sure why...) In addition, I have another item which need to land on M68.
I'd appreciate it very much if you can take over this issue.
Labels: -M-68 M-69
We pushed login to 69 - sorry for not updating the bug. You can reassign the bug to me if you will not have time to take a look - thanks for investigating thus far.

Comment 6 by fukino@chromium.org, May 21 2018

Thank you for updating the target milestone!
I'll find a chance to look into this issue after M68 branch point.
In the meantime, if you have cycles, please feel free to take this one. Thanks!
fukino@, could you help to verify if the migration UI shows up correctly?

Think this CL should implement GetWizardController() now:
https://chromium-review.googlesource.com/c/chromium/src/+/1087772

Cc: xiaoyinh@chromium.org

Comment 9 by fukino@chromium.org, Jun 14 2018

Re:#7
Thank you for taking care of this screen!
I tested the migration screen on ToT.

The layout seems a bit broken (the footer is misplaced), but I can see the migration screen without Chrome crash.
IMG_20180614_173134.jpg
4.9 MB View Download
Think the spacing in the bottom is reserved for the shelf, however in views login
we moved the shelf into ash, so this additional space is not needed. Can we maybe
modify this for views login case?

https://cs.chromium.org/chromium/src/chrome/browser/resources/chromeos/login/encryption_migration.css?rcl=1ce4571e5093173786966ad4dd2357295094d59f&l=7
Cc: qnnguyen@chromium.org
FYI xiaoyinh@ I believe qnnguyen@ has a CL that eliminates the grey box.
fukino@, outside of the grey box is there any remaining work?
Hi jdufault@,
Except for the gray box, the migration dialog worked properly.

On sign-in screen, I still have a remaining task.  crbug.com/831099 .
I guess this task is to implement UserBoardViewMojo::ShowBannerMessage().
https://codesearch.chromium.org/chromium/src/chrome/browser/chromeos/login/user_board_view_mojo.h?rcl=c9dcd5e903db6fd1a4943b9e5e2eb3afdaca6ad7&l=27

I'll be able to investigate how to implement it tomorrow. (I'm sorry for the delay: I overlooked it)
Of course, it'd be great if you have cycles to help the task.
Labels: -M-69 M-70
Did you have a chance to take a look?
I'm working on a CL for  issue 831099 .

qnnguyen@, are you going to eliminate the gray box? (Re: comment #11)
I saw the gray box on ToT two days ago.
Status: Fixed (was: Assigned)
The grey box has been removed

Sign in to add a comment