Issue metadata
Sign in to add a comment
|
Split ash init into pre- and post-local state prefs being available |
||||||||||||||||||||||||
Issue descriptionSee https://docs.google.com/document/d/1pz-uv1LnJUIOnV0rA_Qp-4mCfnLLluzDii5j849XKjs/edit# for details. Basically, the local state PrefService is not available during Shell initialization on mash. We need to make sure we don't add more code that works in classic ash but subtly breaks in mash. So we need both classic ash and mash to work similarly and split the ash init into parts for before and after local state. This is conceptually similar to how we have pre- and post-bluetooth initialized phases. PaletteTray will need some minor changes because it currently uses local state during its init. See also issue 751143 (trying to block ash init until local state ready) and issue 751191 (wallpaper local state). +Sonny and Sammie just FYI
,
Aug 8 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bbe5cb1a8c2c0d01eadecd2510e054efe91fb898 commit bbe5cb1a8c2c0d01eadecd2510e054efe91fb898 Author: James Cook <jamescook@chromium.org> Date: Tue Aug 08 18:32:21 2017 cros: Refine local state prefs usage in ash On mash the ash process has to connect to the mojo pref service before local state prefs are available, which means there is a window during startup where they are not available. Classic ash allows immediate access to local state prefs, which makes it easy to write code that works in classic but not in mash. Make classic ash and mash local state behavior more similar: * Add ShellObserver::OnLocalStatePrefServiceReady() * Make Shell::GetLocalStatePrefService() return null until that observer method is fired * Eliminate ShellDelegate::GetLocalStatePrefService() so that code in classic ash can't use the delegate to get the PrefService too early * Fix PaletteTray to use the new approach WallpaperController will need to be fixed in a separate CL because wallpaper also has other differences between classic and mash. Bug: 752671 , 751191 , 752997 Test: added to ash_unittests Change-Id: Iead4ae3f4c2fc4ed065a0eb9ebae7668dadc6c25 Reviewed-on: https://chromium-review.googlesource.com/604478 Commit-Queue: James Cook <jamescook@chromium.org> Reviewed-by: Sammie Quon <sammiequon@chromium.org> Reviewed-by: Sam McNally <sammc@chromium.org> Cr-Commit-Position: refs/heads/master@{#492706} [modify] https://crrev.com/bbe5cb1a8c2c0d01eadecd2510e054efe91fb898/ash/mus/shell_delegate_mus.cc [modify] https://crrev.com/bbe5cb1a8c2c0d01eadecd2510e054efe91fb898/ash/mus/shell_delegate_mus.h [modify] https://crrev.com/bbe5cb1a8c2c0d01eadecd2510e054efe91fb898/ash/shell.cc [modify] https://crrev.com/bbe5cb1a8c2c0d01eadecd2510e054efe91fb898/ash/shell.h [modify] https://crrev.com/bbe5cb1a8c2c0d01eadecd2510e054efe91fb898/ash/shell/shell_delegate_impl.cc [modify] https://crrev.com/bbe5cb1a8c2c0d01eadecd2510e054efe91fb898/ash/shell/shell_delegate_impl.h [modify] https://crrev.com/bbe5cb1a8c2c0d01eadecd2510e054efe91fb898/ash/shell_delegate.h [modify] https://crrev.com/bbe5cb1a8c2c0d01eadecd2510e054efe91fb898/ash/shell_observer.h [modify] https://crrev.com/bbe5cb1a8c2c0d01eadecd2510e054efe91fb898/ash/shell_test_api.cc [modify] https://crrev.com/bbe5cb1a8c2c0d01eadecd2510e054efe91fb898/ash/shell_test_api.h [modify] https://crrev.com/bbe5cb1a8c2c0d01eadecd2510e054efe91fb898/ash/shell_unittest.cc [modify] https://crrev.com/bbe5cb1a8c2c0d01eadecd2510e054efe91fb898/ash/system/palette/palette_tray.cc [modify] https://crrev.com/bbe5cb1a8c2c0d01eadecd2510e054efe91fb898/ash/system/palette/palette_tray.h [modify] https://crrev.com/bbe5cb1a8c2c0d01eadecd2510e054efe91fb898/ash/system/palette/palette_tray_unittest.cc [modify] https://crrev.com/bbe5cb1a8c2c0d01eadecd2510e054efe91fb898/ash/test_shell_delegate.cc [modify] https://crrev.com/bbe5cb1a8c2c0d01eadecd2510e054efe91fb898/ash/test_shell_delegate.h [modify] https://crrev.com/bbe5cb1a8c2c0d01eadecd2510e054efe91fb898/chrome/browser/ui/ash/ash_init.cc [modify] https://crrev.com/bbe5cb1a8c2c0d01eadecd2510e054efe91fb898/chrome/browser/ui/ash/chrome_shell_delegate.cc [modify] https://crrev.com/bbe5cb1a8c2c0d01eadecd2510e054efe91fb898/chrome/browser/ui/ash/chrome_shell_delegate.h
,
Aug 8 2017
Done - I decided to go with ShellObserver::OnLocalStatePrefServiceInitialized() as the signal to do the second phase init. So far the things that need it might have more than one instance (e.g. one per monitor). It's simpler to do it this way than to add something like Shell::InitializeAfterLocalStateReady(). We can add that later if we need it.
,
Jan 22 2018
,
Feb 26 2018
|
|||||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||||
Comment 1 by jamescook@chromium.org
, Aug 7 2017