New issue
Advanced search Search tips

Issue 751191 link

Starred by 1 user

Issue metadata

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

Blocked on:
issue 751143

Blocking:
issue 753012



Sign in to add a comment

mash: Use local state pref |kWallpaperColors| for wallpaper color caching

Project Member Reported by wzang@chromium.org, Aug 1 2017

Issue description

For now wallpaper color caching is skipped in the case of config::MASH, because local state is not available for an arbitrary amount of time after ash startup. Once the issue is fixed, MASH should use color caching in the same way with config::CLASSIC case.

 
Labels: OS-Chrome
Summary: mash: Use local state pref |kWallpaperColors| for wallpaper color caching (was: MASH should be able to read |kWallpaperColors| for wallpaper color caching)
Blocking: 753012
Project Member

Comment 3 by bugdroid1@chromium.org, 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

Labels: m61debt
Project Member

Comment 5 by bugdroid1@chromium.org, Aug 30 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a853cd85ddccd0d99e2bcd1b8fa06bf786404754

commit a853cd85ddccd0d99e2bcd1b8fa06bf786404754
Author: Sam McNally <sammc@chromium.org>
Date: Wed Aug 30 01:55:10 2017

Unify classic ash and mash access to local state prefs.

Change classic ash to always use mojo to access local state prefs,
matching mash.

Change-Id: Ifd45ac5b2fa832a1a16efeaac4e9be91a1b2b266
Bug:  751191 
Reviewed-on: https://chromium-review.googlesource.com/616505
Reviewed-by: Mustafa Emre Acer <meacer@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Commit-Queue: Sam McNally <sammc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#498337}
[modify] https://crrev.com/a853cd85ddccd0d99e2bcd1b8fa06bf786404754/ash/shell.cc
[modify] https://crrev.com/a853cd85ddccd0d99e2bcd1b8fa06bf786404754/ash/shell.h
[modify] https://crrev.com/a853cd85ddccd0d99e2bcd1b8fa06bf786404754/ash/shell_unittest.cc
[modify] https://crrev.com/a853cd85ddccd0d99e2bcd1b8fa06bf786404754/ash/system/palette/palette_tray.cc
[modify] https://crrev.com/a853cd85ddccd0d99e2bcd1b8fa06bf786404754/ash/system/palette/palette_tray_unittest.cc
[modify] https://crrev.com/a853cd85ddccd0d99e2bcd1b8fa06bf786404754/ash/test/ash_test_base.cc
[modify] https://crrev.com/a853cd85ddccd0d99e2bcd1b8fa06bf786404754/ash/test/ash_test_base.h
[modify] https://crrev.com/a853cd85ddccd0d99e2bcd1b8fa06bf786404754/ash/test/ash_test_helper.cc
[modify] https://crrev.com/a853cd85ddccd0d99e2bcd1b8fa06bf786404754/ash/test/ash_test_helper.h
[modify] https://crrev.com/a853cd85ddccd0d99e2bcd1b8fa06bf786404754/ash/wallpaper/wallpaper_controller.cc
[modify] https://crrev.com/a853cd85ddccd0d99e2bcd1b8fa06bf786404754/ash/wallpaper/wallpaper_controller.h
[modify] https://crrev.com/a853cd85ddccd0d99e2bcd1b8fa06bf786404754/chrome/browser/chrome_content_browser_manifest_overlay.json
[modify] https://crrev.com/a853cd85ddccd0d99e2bcd1b8fa06bf786404754/chrome/browser/chromeos/preferences.cc
[modify] https://crrev.com/a853cd85ddccd0d99e2bcd1b8fa06bf786404754/chrome/browser/ui/ash/ash_init.cc

Status: Fixed (was: Assigned)
Fixed in Sam's CL above.

Comment 7 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Comment 8 by dchan@chromium.org, Jan 23 2018

Status: Fixed (was: Archived)
Components: -Internals>MUS Internals>Services>WindowService

Sign in to add a comment