White wallpaper in OOBE should be rendered by wallpaper controller, not by OOBE HTML |
||||||||||
Issue descriptionRight now the white OOBE background is being done in HTML. This has a couple of issues: - on boot, the white background is not applied until after OOBE is loaded - if oobe crashes/etc the white background will go away - the status area renders assuming the white background The biggest issue here is the boot experience, which looks very janky/unpolished. Either we need to hide native UI until webui is ready, or we move the white background rendering to native UI. I think the right solution here is to move background rendering to native UI. It works in both webui and views and has less overdraw.
,
Aug 9
We really should try to merge this to 69 as well.
,
Aug 9
+elizabethchiu@ for thoughts Deciding when to show the white background is a bit tricky; there are a number of cases to consider. 1 Going through OOBE normally 2 Partially completing OOBE, never adding a user, coming back to the device a week later where the device boots directly into add user 3 Device is enterprise enrolled but doesn't have any users 4 Device is enterprise enrolled with users, but then the user removes all users from login 5 Add user when there are already users 6 Special screens like powerwash Some of these scenarios have an add user dialog that cannot be dismissed (cases 1,2,3,4).
,
Aug 9
> Some of these scenarios have an add user dialog that cannot be dismissed (cases 1,2,3,4). We should consider using white background to subtly inform the user that the gaia/add user dialog cannot be dismissed.
,
Aug 9
Two options: 1) Create a |ShowOOBEWallpaper| mojo call in wallpaper.mojom (Or more generally, |ShowSolidColorWallpaper(SkColor color)|). |wallpaper_controller_client| in //chrome may call this method when session state is OOBE, but there should be another caller responsible for calling |ShowUserWallpaper(AccountId id)| to update the wallpaper when OOBE completes (or whenever white background is no longer desired.) It can also call |ShowSigninWallpaper| if there's no user logged in yet, in which case it will display the default wallpaper. 2) Put the entire logic in wallpaper controller. It will observe the session state and show white wallpaper if it' SessionState::OOBE, and a user wallpaper otherwise. I prefer 1) because it's more flexible.
,
Aug 9
Or, option 3): If the primary reason for doing this is to ensure a smooth boot experience, I think it's better to: apply the white wallpaper before OOBE dialog is shown, but as soon as the dialog (which still keeps the full screen html white background) is ready, notify the wallpaper controller to revert to the default wallpaper behind the scene. It makes sense to let the html dialog maintain full-screen, since it's a little weird if the full screen actually consists partially of the OOBE dialog and partially of wallpaper. (If OOBE crashes, user may see the default wallpaper behind, but it may not be a big deal.)
,
Aug 10
Per offline discussion with wzang@, I also think that option 3) from #6 is reasonable.
,
Aug 10
If the requirements are simple enough that we can just observe session state to determine wallpaper color, I think we should go with solution 2; however, I doubt this will be the case. I think a slight variation on solution 1 is the best, perhaps |SetWallpaperOverride(optional<Color> override_color)|. This way, login and OOBE can separately control wallpaper color as needed. Separate control here is good, because OOBE does not have all of the context it needs to make the decision w.r.t. wallpaper. For example, login instantiates OOBE for the power-wash dialog, which should most likely not have a white background (whereas power-wash in OOBE likely will have a white background). As a note, one of the things login has been doing is removing chrome/'s ability to control what the UI looks like (ie, exactly this). Here are the issues I see with option 3: - complexity: we now have two places that render a white wallpaper, which are used at differing times - new class of bugs: it's now possible for wallpaper render state to get out of sync, ie, if ash/ thinks OOBE is rendering wallpaper but OOBE is not for whatever reason - challenges with login: OOBE dialog is not always full-screen when used in login - works around the real bug (see below) > since it's a little weird if the full screen actually consists partially of the OOBE dialog and partially of wallpaper. I don't think this is as strange as it may seem; there are already other elements which are rendered separately, ie, the shelf, the status area, etc, but which look like they're part of the same UI in OOBE (technically the shelf in OOBE is still HTML, but that is a bug and needs get fixed/moved to native shelf). The crux of the issue on boot is that these other elements are ready much faster than OOBE and render assuming OOBE is already rendered. Fixing the render order such that all appear at the same time would also fix the boot issue here, but I think it is still worth rendering the white background in the wallpaper controller to avoid overdraw and also due to making login simpler (now login does need to dynamically resize the OOBE widget to full-screen when it wants a white background). If we fix the render order issue, then solution 3 is not needed - which implies it is a work-around for the real issue.
,
Aug 10
We'll need something like a "white background controller" to serve as the source of truth and inform all UI elements to update their states. This is necessary because the criteria for white background should be more complex than a simple session state. Note there's already a bug now: in case 2) of comment #3, the Add user screen is NOT full screen but the status area thinks it is. > new class of bugs: it's now possible for wallpaper render state to get out of sync, ie, if ash/ thinks OOBE is rendering wallpaper but OOBE is not for whatever reason This issue is not specific to option 3), as mentioned above, it's always possible to have the out of sync problem if we don't have a source of truth. > challenges with login: OOBE dialog is not always full-screen when used in login This is not an issue for option 3), note that if we go with 3), we'll still need to create a ShowSolidColorWallpaper mojo call, and login screen is able to use it. But that means that login full-screen is partly wallpaper and partly OOBE dialog, yet the OOBE full-screen is all dialog (except for using wallpaper as a buffer right after boot). If we want to unify the implementation here, I still prefer to use all html (ie. how hard is it to fix the full-screen dialog issue in login?) > I don't think this is as strange as it may seem; there are already other elements which are rendered separately, ie, the shelf, the status area, etc The shelf and status area are indeed separate UI elements, but wallpaper is not. Essentially the use of a white wallpaper relies on the fact that it blends into the OOBE dialog, so that the dialog "looks like" it's full-screen. Using a white wallpaper is not robust: if UX decides to add more elements near the edge of the screen, we'll still need to make the dialog full-screen. I think we should: 1) Create a source truth for all UI elements to query. 2) To fix the boot experience: if possible, make the white splash screen stay until OOBE dialog is ready, or use a white wallpaper as a buffer. 3) When OOBE is shown, the full screen should come from the html background, and fix the full-screen dialog issue in login.
,
Aug 10
Talking offline, we should investigate delaying startup so that the splash screen does not go away until we're actually ready. That will deal with the biggest polish issue. Beyond that, I'm fine with rendering the background in either the dialog or wallpaper view, but the better approach may depend on what the UX ends up being.
,
Aug 11
Talking offline with elizabethchiu@, it's confirmed that we want to show a full white background as long as no user is added yet (which covers cases 1-3 in #3), for cases 4-6, it's undecided but preferred not to have full screen at this point. So it's okay to use session state OOBE as the criteria to show full screen background, but I'll still go with option 1) and create a mojo API, in case we need full screen for login later.
,
Aug 11
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/408f12cadfb0abebf664e74809706144b3a60590 commit 408f12cadfb0abebf664e74809706144b3a60590 Author: Wenzhao Zang <wzang@chromium.org> Date: Sat Aug 11 07:32:51 2018 cros: Dialogs should always be full-screen during OOBE The dialogs should always be full-screen as long as there's no user added yet. This covers the corner cases that user stops in the middle of OOBE and returns later. Do not show the dialog as full-screen (at least for now, until there's UX update) for cases that the OOBE is completed, but user pods are not shown (due to policy, or all users are removed manually on an enrolled device). Bug: 872936 Change-Id: I02a648585a7cbebd1b570096ab3608bff000e96d Reviewed-on: https://chromium-review.googlesource.com/1171916 Reviewed-by: Alexander Alekseev <alemate@chromium.org> Commit-Queue: Wenzhao (Colin) Zang <wzang@chromium.org> Cr-Commit-Position: refs/heads/master@{#582437} [modify] https://crrev.com/408f12cadfb0abebf664e74809706144b3a60590/chrome/browser/resources/chromeos/login/oobe_dialog.js
,
Aug 15
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7f67c48fca5aba83717fe9c99b2d92a2049c9f6a commit 7f67c48fca5aba83717fe9c99b2d92a2049c9f6a Author: Wenzhao Zang <wzang@chromium.org> Date: Wed Aug 15 03:51:53 2018 cros: Smoothen the transition between boot splash and full-screen OOBE 1) The full-screen white background in OOBE is rendered in web-ui, so there's a delay showing it. Users will see a glimpse of the default wallpaper (it's blue on an eve) between the boot splash screen and the OOBE screen (both white). To avoid this, we can show a white wallpaper during the entire OOBE screen, which can show right after the boot splash seamlessly. 2) The original idea was to delay the dismissal of the boot splash screen until OOBE dialog is ready. However, boot splash screen will not always show (e.g. when a guest user exits, or browser crash and restart). In such cases, we also want to avoid showing the blue default wallpaper before the white OOBE screen is loaded. Therefore, showing a white wallpaper is a better solution. Bug: 872936 Change-Id: Iaa600f9d455956743a6cfd3dd26f07ad2a4ed61d Reviewed-on: https://chromium-review.googlesource.com/1172066 Reviewed-by: Xiyuan Xia <xiyuan@chromium.org> Reviewed-by: Oliver Chang <ochang@chromium.org> Reviewed-by: Xiaoqian Dai <xdai@chromium.org> Commit-Queue: Wenzhao (Colin) Zang <wzang@chromium.org> Cr-Commit-Position: refs/heads/master@{#583154} [modify] https://crrev.com/7f67c48fca5aba83717fe9c99b2d92a2049c9f6a/ash/public/cpp/wallpaper_types.h [modify] https://crrev.com/7f67c48fca5aba83717fe9c99b2d92a2049c9f6a/ash/public/interfaces/wallpaper.mojom [modify] https://crrev.com/7f67c48fca5aba83717fe9c99b2d92a2049c9f6a/ash/wallpaper/wallpaper_controller.cc [modify] https://crrev.com/7f67c48fca5aba83717fe9c99b2d92a2049c9f6a/ash/wallpaper/wallpaper_controller.h [modify] https://crrev.com/7f67c48fca5aba83717fe9c99b2d92a2049c9f6a/ash/wallpaper/wallpaper_controller_unittest.cc [modify] https://crrev.com/7f67c48fca5aba83717fe9c99b2d92a2049c9f6a/chrome/browser/ui/ash/test_wallpaper_controller.cc [modify] https://crrev.com/7f67c48fca5aba83717fe9c99b2d92a2049c9f6a/chrome/browser/ui/ash/test_wallpaper_controller.h [modify] https://crrev.com/7f67c48fca5aba83717fe9c99b2d92a2049c9f6a/chrome/browser/ui/ash/wallpaper_controller_client.cc [modify] https://crrev.com/7f67c48fca5aba83717fe9c99b2d92a2049c9f6a/chrome/browser/ui/ash/wallpaper_controller_client.h [modify] https://crrev.com/7f67c48fca5aba83717fe9c99b2d92a2049c9f6a/tools/metrics/histograms/enums.xml
,
Aug 15
Merge request for the two CLs in #12 and #13.
,
Aug 15
Merge approved, M69.
,
Aug 15
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6c8be30b4d5fa4c892bbffc5e5a996c2fd30d6a0 commit 6c8be30b4d5fa4c892bbffc5e5a996c2fd30d6a0 Author: Wenzhao Zang <wzang@chromium.org> Date: Wed Aug 15 21:50:49 2018 [Merge to M69] cros: Dialogs should always be full-screen during OOBE The dialogs should always be full-screen as long as there's no user added yet. This covers the corner cases that user stops in the middle of OOBE and returns later. Do not show the dialog as full-screen (at least for now, until there's UX update) for cases that the OOBE is completed, but user pods are not shown (due to policy, or all users are removed manually on an enrolled device). TBR=wzang@chromium.org (cherry picked from commit 408f12cadfb0abebf664e74809706144b3a60590) Bug: 872936 Change-Id: I02a648585a7cbebd1b570096ab3608bff000e96d Reviewed-on: https://chromium-review.googlesource.com/1171916 Reviewed-by: Alexander Alekseev <alemate@chromium.org> Commit-Queue: Wenzhao (Colin) Zang <wzang@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#582437} Reviewed-on: https://chromium-review.googlesource.com/1176434 Reviewed-by: Wenzhao (Colin) Zang <wzang@chromium.org> Cr-Commit-Position: refs/branch-heads/3497@{#659} Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753} [modify] https://crrev.com/6c8be30b4d5fa4c892bbffc5e5a996c2fd30d6a0/chrome/browser/resources/chromeos/login/oobe_dialog.js
,
Aug 15
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6d56a0d4f7305cb08e51d2392571b383c283ee56 commit 6d56a0d4f7305cb08e51d2392571b383c283ee56 Author: Wenzhao Zang <wzang@chromium.org> Date: Wed Aug 15 22:00:38 2018 [Merge to M69] cros: Smoothen the transition between boot splash and full-screen OOBE 1) The full-screen white background in OOBE is rendered in web-ui, so there's a delay showing it. Users will see a glimpse of the default wallpaper (it's blue on an eve) between the boot splash screen and the OOBE screen (both white). To avoid this, we can show a white wallpaper during the entire OOBE screen, which can show right after the boot splash seamlessly. 2) The original idea was to delay the dismissal of the boot splash screen until OOBE dialog is ready. However, boot splash screen will not always show (e.g. when a guest user exits, or browser crash and restart). In such cases, we also want to avoid showing the blue default wallpaper before the white OOBE screen is loaded. Therefore, showing a white wallpaper is a better solution. TBR=wzang@chromium.org (cherry picked from commit 7f67c48fca5aba83717fe9c99b2d92a2049c9f6a) Bug: 872936 Change-Id: Iaa600f9d455956743a6cfd3dd26f07ad2a4ed61d Reviewed-on: https://chromium-review.googlesource.com/1172066 Reviewed-by: Xiyuan Xia <xiyuan@chromium.org> Reviewed-by: Oliver Chang <ochang@chromium.org> Reviewed-by: Xiaoqian Dai <xdai@chromium.org> Commit-Queue: Wenzhao (Colin) Zang <wzang@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#583154} Reviewed-on: https://chromium-review.googlesource.com/1176440 Reviewed-by: Wenzhao (Colin) Zang <wzang@chromium.org> Cr-Commit-Position: refs/branch-heads/3497@{#660} Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753} [modify] https://crrev.com/6d56a0d4f7305cb08e51d2392571b383c283ee56/ash/public/cpp/wallpaper_types.h [modify] https://crrev.com/6d56a0d4f7305cb08e51d2392571b383c283ee56/ash/public/interfaces/wallpaper.mojom [modify] https://crrev.com/6d56a0d4f7305cb08e51d2392571b383c283ee56/ash/wallpaper/wallpaper_controller.cc [modify] https://crrev.com/6d56a0d4f7305cb08e51d2392571b383c283ee56/ash/wallpaper/wallpaper_controller.h [modify] https://crrev.com/6d56a0d4f7305cb08e51d2392571b383c283ee56/ash/wallpaper/wallpaper_controller_unittest.cc [modify] https://crrev.com/6d56a0d4f7305cb08e51d2392571b383c283ee56/chrome/browser/ui/ash/test_wallpaper_controller.cc [modify] https://crrev.com/6d56a0d4f7305cb08e51d2392571b383c283ee56/chrome/browser/ui/ash/test_wallpaper_controller.h [modify] https://crrev.com/6d56a0d4f7305cb08e51d2392571b383c283ee56/chrome/browser/ui/ash/wallpaper_controller_client.cc [modify] https://crrev.com/6d56a0d4f7305cb08e51d2392571b383c283ee56/chrome/browser/ui/ash/wallpaper_controller_client.h [modify] https://crrev.com/6d56a0d4f7305cb08e51d2392571b383c283ee56/tools/metrics/histograms/enums.xml
,
Aug 15
This is fixed. But, we can continue discussing whether to remove full-screen-dialog property in OOBE. I don't think we should.
,
Aug 15
I'm okay leaving it in for the time being. Perhaps it will reduce complexity during oobe when the user session is created.
,
Aug 15
cindyb@ please always use the merge approval label Merge-Approved-MM to approved a merge. The merge violation will flag such bug as a violation if the label is not properly applicable. Thanks!
,
Aug 15
(I should mention that it still feels like a bit of a hack, especially given that there are now two places that render the white wallpaper).
,
Aug 15
Actually I think this is the optimal solution. This is indeed a workaround, but IMO that's the whole point of it: to work around the real issue that OOBE web-ui is rendered slower than the native elements. Before we re-implement OOBE in views (if that ever happens), it's reasonable to use something as a transition buffer to make sure everything appears together. But, after OOBE dialog has the ability to render full-screen, IMO we should not continue relying on the wallpaper. Perhaps, one improvement is to revert to the regular wallpaper right after OOBE dialog is shown, instead of waiting until user session starts, so that it doesn't feel "duplicate", but other than that, IMO this is the optimal solution. Note: 1) The UX mentioned once that they wanted to add new elements around the edge of the screen. So full-screen OOBE is a must-have when that happens. 2) If the property is removed, we may run into another polish issue when dismissing OOBE dialog (I haven't verified but I would imagine the wallpaper will always respond faster than web-ui elements to the same signal.) 3) If we ever need full-screen "Add person" in views-login, and if full-screen widget is not possible in login, the worst case is that we can still show a white wallpaper as the background. But there will be additional polish issue to make sure the two things are synchronous, so I think fixing the full-screen widget issue in login is a better solution.
,
Aug 16
This bug requires manual review: M69 has already been promoted to the beta branch, so this requires manual review Please contact the milestone owner if you have questions. Owners: amineer@(Android), kariahda@(iOS), cindyb@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Aug 16
,
Aug 16
,
Aug 18
Issue 868041 has been merged into this issue.
,
Aug 20
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible! If all merges have been completed, please remove any remaining Merge-Approved labels from this issue. Thanks for your time! To disable nags, add the Disable-Nags label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Aug 20
Verified on M69 (10895.31.0, 69.0.3497.50) beta channel on caroline.
,
Aug 20
|
||||||||||
►
Sign in to add a comment |
||||||||||
Comment 1 by jdufault@chromium.org
, Aug 9