New issue
Advanced search Search tips

Issue 795159 link

Starred by 1 user
Status: Fixed
Owner:
wzang@chromium.org
Closed: Mar 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug

Blocking:
issue 776464



Sign in to add a comment

Create WallpaperPaths class under ash/public/cpp

Project Member Reported by wzang@chromium.org, Dec 15 2017 
//chrome and //ash are sharing some wallpaper file paths. We should first try to reduce the path sharing, but for certain cases such as device policy wallpapers it's not feasible to completely remove the access from //chrome.

We can create a WallpaperPaths class under ash/public/cpp, with static variables for the paths and Set/Get methods. Putting the logic in //ash/public/cpp is a signal that it has been vetted for mash.

(Based on the comments from CL https://chromium-review.googlesource.com/c/chromium/src/+/820854/9)

Comment 1 by afakhry@chromium.org, Dec 15 2017

Components: UI>Shell>Wallpaper

Comment 2 by wzang@chromium.org, Mar 23 2018 

Update: at this point creating the class is unnecessary. We only need a one-time
initialization of the wallpaper paths, and they can be internal to // ash after that.
Project Member

Comment 3 by bugdroid1@chromium.org, Mar 27 2018 

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

commit efa536ef50be9d3ff56926d164150df15987fc01
Author: Wenzhao Zang <wzang@chromium.org>
Date: Tue Mar 27 20:04:22 2018

cros: Make wallpaper paths internal to WallpaperController

1) The original idea was to create a WallpaperPaths class in
   //ash/public, but it's unnecessary now. We only need a one-time
   initialization of the wallpaper paths, and they can be internal to
   //ash after that.

2) Saving and syncing thumbnail is entirely implemented in
   wallpaper_manager.js, so delete all thumbnail related code in
   *.cc files. The |thumbnail_path| variable in https://goo.gl/HZw6r7
   is left because https://chromiumcodereview.appspot.com/12334030
   forgot to clean it up.

3) Removed code duplication of thumbnail generation in wallpaper_api
   and wallpaper_private_api.

Bug:  795159 
Test: Manual
Change-Id: Ia861da7579f743e5fc380128c6d28405246de2c4
Reviewed-on: https://chromium-review.googlesource.com/967882
Reviewed-by: Xiaoqian Dai <xdai@chromium.org>
Reviewed-by: Will Harris <wfh@chromium.org>
Commit-Queue: Wenzhao (Colin) Zang <wzang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546222}
[modify] https://crrev.com/efa536ef50be9d3ff56926d164150df15987fc01/ash/public/interfaces/wallpaper.mojom
[modify] https://crrev.com/efa536ef50be9d3ff56926d164150df15987fc01/ash/wallpaper/wallpaper_controller.cc
[modify] https://crrev.com/efa536ef50be9d3ff56926d164150df15987fc01/ash/wallpaper/wallpaper_controller.h
[modify] https://crrev.com/efa536ef50be9d3ff56926d164150df15987fc01/ash/wallpaper/wallpaper_controller_unittest.cc
[modify] https://crrev.com/efa536ef50be9d3ff56926d164150df15987fc01/chrome/browser/chromeos/extensions/wallpaper_api.cc
[modify] https://crrev.com/efa536ef50be9d3ff56926d164150df15987fc01/chrome/browser/chromeos/extensions/wallpaper_api.h
[modify] https://crrev.com/efa536ef50be9d3ff56926d164150df15987fc01/chrome/browser/chromeos/extensions/wallpaper_function_base.cc
[modify] https://crrev.com/efa536ef50be9d3ff56926d164150df15987fc01/chrome/browser/chromeos/extensions/wallpaper_function_base.h
[modify] https://crrev.com/efa536ef50be9d3ff56926d164150df15987fc01/chrome/browser/chromeos/extensions/wallpaper_private_api.cc
[modify] https://crrev.com/efa536ef50be9d3ff56926d164150df15987fc01/chrome/browser/chromeos/extensions/wallpaper_private_api.h
[modify] https://crrev.com/efa536ef50be9d3ff56926d164150df15987fc01/chrome/browser/ui/ash/test_wallpaper_controller.cc
[modify] https://crrev.com/efa536ef50be9d3ff56926d164150df15987fc01/chrome/browser/ui/ash/test_wallpaper_controller.h
[modify] https://crrev.com/efa536ef50be9d3ff56926d164150df15987fc01/chrome/browser/ui/ash/wallpaper_controller_client.cc
[modify] https://crrev.com/efa536ef50be9d3ff56926d164150df15987fc01/chrome/browser/ui/ash/wallpaper_policy_handler.cc
[modify] https://crrev.com/efa536ef50be9d3ff56926d164150df15987fc01/chrome/browser/ui/ash/wallpaper_policy_handler.h

Comment 4 by wzang@chromium.org, Mar 29 2018

Status: Fixed (was: Assigned)
►

Sign in to add a comment