New issue
Advanced search Search tips

Issue 798604 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug

Blocked on:
issue 776464



Sign in to add a comment

Move OpenWallpaperPicker to WallpaperControllerClient

Project Member Reported by wzang@chromium.org, Jan 2 2018

Issue description

Wallpaper picker is an extension and it can be opened from both //chrome (via appearance_handler) and //ash (via wallpaper_controller). Therefore, the implementation should reside in WallpaperControllerClient.


 

Comment 1 by osh...@chromium.org, Jan 26 2018

Components: UI>Shell>Wallpaper
Project Member

Comment 2 by bugdroid1@chromium.org, Feb 2 2018

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

commit 7064244c0c8cdef74691107487c32ef2bc90f08a
Author: Wenzhao Zang <wzang@chromium.org>
Date: Fri Feb 02 04:41:44 2018

cros: Move OpenWallpaperPicker to WallpaperControllerClient

1) Opening wallpaper picker is NOT allowed if:
   a) The active user is policy controlled (must be checked by
      //ash/WallpaperController), or
   b) The active user type is GUEST or KIOSK etc. (can be checked
      from //ash or //chrome).

2) The wallpaper picker can be opened either from the setting page
   (//chrome) or the shelf context menu (//ash). In addition, the
   setting page need to distinguish between cases a) and b), in order
   to show different UI. So we need to create two mojo callbacks.

3) In addition, WallpaperControllerClient should not rely on the
   setting page to check a) and b): it must call into //ash to ask,
   in case a refactoring of the setting page will cause
   regression (there was once a bug because of this.)

4) The check of b) used to rely on LoginState but it was recently
   deprecated, so it's using SessionController in //ash instead.

TBR=blundell@chromium.org
      in Settings is a) invisible for guest session, b) disabled for
      users controlled by wallpaper policy, c) enabled for other cases.

Bug:  798604 
Test: 1) ash_unittests 2) Manual test by checking the 'Wallpaper' item
Change-Id: Icb1479d2112f02b823237c418622cc0cdbae65fc
Reviewed-on: https://chromium-review.googlesource.com/848193
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Oliver Chang <ochang@chromium.org>
Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Xiaoqian Dai <xdai@chromium.org>
Commit-Queue: Wenzhao (Colin) Zang <wzang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#533952}
[modify] https://crrev.com/7064244c0c8cdef74691107487c32ef2bc90f08a/ash/BUILD.gn
[modify] https://crrev.com/7064244c0c8cdef74691107487c32ef2bc90f08a/ash/default_wallpaper_delegate.cc
[modify] https://crrev.com/7064244c0c8cdef74691107487c32ef2bc90f08a/ash/default_wallpaper_delegate.h
[modify] https://crrev.com/7064244c0c8cdef74691107487c32ef2bc90f08a/ash/public/interfaces/wallpaper.mojom
[modify] https://crrev.com/7064244c0c8cdef74691107487c32ef2bc90f08a/ash/shelf/shelf_context_menu_model.cc
[modify] https://crrev.com/7064244c0c8cdef74691107487c32ef2bc90f08a/ash/test_shell_delegate.cc
[delete] https://crrev.com/767edb03214a64265b6b360e65a22388f6016fdd/ash/wallpaper/test_wallpaper_delegate.cc
[delete] https://crrev.com/767edb03214a64265b6b360e65a22388f6016fdd/ash/wallpaper/test_wallpaper_delegate.h
[modify] https://crrev.com/7064244c0c8cdef74691107487c32ef2bc90f08a/ash/wallpaper/wallpaper_controller.cc
[modify] https://crrev.com/7064244c0c8cdef74691107487c32ef2bc90f08a/ash/wallpaper/wallpaper_controller.h
[modify] https://crrev.com/7064244c0c8cdef74691107487c32ef2bc90f08a/ash/wallpaper/wallpaper_controller_unittest.cc
[modify] https://crrev.com/7064244c0c8cdef74691107487c32ef2bc90f08a/ash/wallpaper/wallpaper_delegate.h
[modify] https://crrev.com/7064244c0c8cdef74691107487c32ef2bc90f08a/ash/wallpaper/wallpaper_delegate_mus.cc
[modify] https://crrev.com/7064244c0c8cdef74691107487c32ef2bc90f08a/ash/wallpaper/wallpaper_delegate_mus.h
[modify] https://crrev.com/7064244c0c8cdef74691107487c32ef2bc90f08a/chrome/browser/chromeos/BUILD.gn
[modify] https://crrev.com/7064244c0c8cdef74691107487c32ef2bc90f08a/chrome/browser/chromeos/background/ash_wallpaper_delegate.cc
[delete] https://crrev.com/767edb03214a64265b6b360e65a22388f6016fdd/chrome/browser/chromeos/extensions/wallpaper_manager_util.cc
[delete] https://crrev.com/767edb03214a64265b6b360e65a22388f6016fdd/chrome/browser/chromeos/extensions/wallpaper_manager_util.h
[modify] https://crrev.com/7064244c0c8cdef74691107487c32ef2bc90f08a/chrome/browser/chromeos/extensions/wallpaper_private_api.cc
[modify] https://crrev.com/7064244c0c8cdef74691107487c32ef2bc90f08a/chrome/browser/chromeos/login/users/wallpaper/wallpaper_manager.cc
[modify] https://crrev.com/7064244c0c8cdef74691107487c32ef2bc90f08a/chrome/browser/chromeos/login/users/wallpaper/wallpaper_manager.h
[modify] https://crrev.com/7064244c0c8cdef74691107487c32ef2bc90f08a/chrome/browser/ui/ash/test_wallpaper_controller.cc
[modify] https://crrev.com/7064244c0c8cdef74691107487c32ef2bc90f08a/chrome/browser/ui/ash/test_wallpaper_controller.h
[modify] https://crrev.com/7064244c0c8cdef74691107487c32ef2bc90f08a/chrome/browser/ui/ash/wallpaper_controller_client.cc
[modify] https://crrev.com/7064244c0c8cdef74691107487c32ef2bc90f08a/chrome/browser/ui/ash/wallpaper_controller_client.h
[modify] https://crrev.com/7064244c0c8cdef74691107487c32ef2bc90f08a/chrome/browser/ui/webui/settings/appearance_handler.cc
[modify] https://crrev.com/7064244c0c8cdef74691107487c32ef2bc90f08a/chrome/browser/ui/webui/settings/appearance_handler.h
[modify] https://crrev.com/7064244c0c8cdef74691107487c32ef2bc90f08a/components/arc/intent_helper/DEPS
[modify] https://crrev.com/7064244c0c8cdef74691107487c32ef2bc90f08a/components/arc/intent_helper/arc_intent_helper_bridge.cc

Comment 3 by wzang@chromium.org, Feb 2 2018

Status: Fixed (was: Started)

Sign in to add a comment