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

Issue 2627243003: Manual Revert of Reland: chromeos: Fix shelf appearing at login screen under mash (patchset #7 id:1… (Closed)

Created:
3 years, 11 months ago by Timothy Loh
Modified:
3 years, 11 months ago
Reviewers:
msw, James Cook, sky, xiyuan
CC:
chromium-reviews, alemate+watch_chromium.org, sadrul, tfarina, achuith+watch_chromium.org, pam+watch_chromium.org, oshima+watch_chromium.org, kalyank, davemoore+watch_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Manual Revert of Reland: chromeos: Fix shelf appearing at login screen under mash (patchset #7 id:140001 of https://codereview.chromium.org/2625733003/ ) Reason for revert: compile broken and reverting https://codereview.chromium.org/2628973002 didn't seem to fix it. Original issue's description: > Reland: chromeos: Fix shelf appearing at login screen under mash > > The reland adds a fix for a crash in window animation code when restoring > a session that has a minimized window. The animation tries to compute a > shelf item position before the shelf is initialized. The animation is not > visible to the user. Returning an empty rect results in a default target > position, which is fine in this case. > > Added guards for access to ShelfView before shelf is initialized. > > Original CL description: > > Previously chrome --mash would always create the shelf on startup, even at > the login screen. Now it waits until login is complete, like classic ash. > > Ash watches for SessionState::ACTIVE via a SessionStateObserver to create the > shelf, rather than NOTIFICATION_LOGIN_USER_PROFILE_PREPARED. For most login > flows this doesn't matter, but for supervised user creation it means that the > shelf is not created until the flow is completed. This is an improvement > because in the old code the shelf would be created too early and had to be > explicitly hidden. > > Always create a SessionControllerClient in chrome, even in classic ash. This > allows classic ash to use the mojo pathway via ash::SessionController. > > Remove WmShell::ShowShelf(), which was introduced in crrev.com/10693003 to > delay showing the shelf until post-login OOBE (like avatar picture select) > was complete. It does not seem to be needed anymore, either in production or > in tests. > > Change AshTestImplMus to simulate a user logging in which is required to > create the shelf on the primary display and also to have a non-zero user > count to create the shelf on additional displays. > > Remove some unnecessary OS_CHROMEOS ifdefs in //c/b/ui/ash. > > BUG=666021, 679513 > TEST=ash_unittests and chrome browser_tests > > Committed: https://crrev.com/7f99e933e595824281102ff6737075dccbbc4d5c > Cr-Commit-Position: refs/heads/master@{#439845} > > Reverted: > > Review-Url: https://codereview.chromium.org/2619943002 > Cr-Original-Commit-Position: refs/heads/master@{#442484} > Review-Url: https://codereview.chromium.org/2625733003 > Cr-Commit-Position: refs/heads/master@{#443106} > Committed: https://chromium.googlesource.com/chromium/src/+/75298de72cf7c25e137c3c5e63e4d01aa0569c1c TBR=msw@chromium.org,xiyuan@chromium.org,sky@chromium.org,jamescook@chromium.org NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=666021, 679513 Review-Url: https://codereview.chromium.org/2627243003 Cr-Commit-Position: refs/heads/master@{#443138} Committed: https://chromium.googlesource.com/chromium/src/+/99bed8e7ec46b0943261ffb46894503605aeba3f

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+76 lines, -191 lines) Patch
M ash/aura/wm_shell_aura.h View 1 chunk +0 lines, -3 lines 0 comments Download
M ash/aura/wm_shell_aura.cc View 1 chunk +0 lines, -9 lines 0 comments Download
M ash/common/shelf/shelf_widget.h View 3 chunks +3 lines, -7 lines 0 comments Download
M ash/common/shelf/shelf_widget.cc View 4 chunks +5 lines, -14 lines 0 comments Download
M ash/common/test/test_session_state_delegate.cc View 1 chunk +6 lines, -3 lines 0 comments Download
M ash/common/wm_shell.h View 5 chunks +6 lines, -8 lines 0 comments Download
M ash/common/wm_shell.cc View 4 chunks +6 lines, -11 lines 0 comments Download
M ash/mus/test/wm_test_base.h View 1 chunk +0 lines, -4 lines 0 comments Download
M ash/mus/test/wm_test_base.cc View 3 chunks +0 lines, -31 lines 0 comments Download
M ash/mus/window_manager.cc View 3 chunks +6 lines, -4 lines 0 comments Download
M ash/root_window_controller_unittest.cc View 4 chunks +4 lines, -6 lines 0 comments Download
M ash/shell.h View 1 chunk +3 lines, -0 lines 0 comments Download
M ash/shell.cc View 1 chunk +5 lines, -0 lines 0 comments Download
M chrome/browser/chromeos/login/session/chrome_session_manager.cc View 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/chromeos/login/supervised/supervised_user_creation_screen.cc View 2 chunks +3 lines, -0 lines 0 comments Download
M chrome/browser/sessions/session_restore_browsertest_chromeos.cc View 3 chunks +2 lines, -42 lines 0 comments Download
M chrome/browser/ui/ash/chrome_shell_delegate.cc View 2 chunks +2 lines, -0 lines 0 comments Download
M chrome/browser/ui/ash/launcher/chrome_launcher_controller_impl_browsertest.cc View 3 chunks +0 lines, -9 lines 0 comments Download
M chrome/browser/ui/ash/multi_user/multi_user_window_manager_stub.h View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/ui/ash/multi_user/multi_user_window_manager_stub.cc View 1 chunk +0 lines, -4 lines 0 comments Download
M chrome/browser/ui/ash/session_controller_client.h View 1 chunk +0 lines, -3 lines 0 comments Download
M chrome/browser/ui/ash/session_controller_client.cc View 3 chunks +0 lines, -13 lines 0 comments Download
M chrome/browser/ui/browser_browsertest.cc View 2 chunks +0 lines, -8 lines 0 comments Download
M chrome/browser/ui/views/ash/chrome_browser_main_extra_parts_ash.h View 1 chunk +0 lines, -2 lines 0 comments Download
M chrome/browser/ui/views/ash/chrome_browser_main_extra_parts_ash.cc View 6 chunks +22 lines, -8 lines 0 comments Download

Messages

Total messages: 5 (3 generated)
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/2627243003/1
3 years, 11 months ago (2017-01-12 03:39:29 UTC) #2
commit-bot: I haz the power
3 years, 11 months ago (2017-01-12 03:40:59 UTC) #5
Message was sent while issue was closed.
Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/99bed8e7ec46b0943261ffb46894...

Powered by Google App Engine
This is Rietveld 408576698