New issue
Advanced search Search tips

Issue 827062 link

Starred by 2 users

Issue metadata

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

Blocking:
issue 776464



Sign in to add a comment

Remove //ash/wallpaper dependency in //chrome

Project Member Reported by wzang@chromium.org, Mar 29 2018

Issue description

Currently the following files in //chrome still have remaining //ash/wallpaper dependency:

wallpaper_private_api.cc
signin_screen_handler.cc
customization_wallpaper_util.cc
customization_wallpaper_downloader_browsertest.cc
kiosk_browsertest.cc

Removing the dependency should be straightforward.




 

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

Blocking: 776464
Project Member

Comment 2 by bugdroid1@chromium.org, Apr 4 2018

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

commit 25e1bcd846d82970374fb26626a3e4a327405bb0
Author: Wenzhao Zang <wzang@chromium.org>
Date: Wed Apr 04 00:28:31 2018

cros: Create mojo call for getting wallpaper location

Bug:  827062 
Test: --enable-features=Mash
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I8191b88f1561d4f0927f8af17e168b88a54c38a2
Reviewed-on: https://chromium-review.googlesource.com/963010
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Will Harris <wfh@chromium.org>
Commit-Queue: Wenzhao (Colin) Zang <wzang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547887}
[modify] https://crrev.com/25e1bcd846d82970374fb26626a3e4a327405bb0/ash/public/interfaces/wallpaper.mojom
[modify] https://crrev.com/25e1bcd846d82970374fb26626a3e4a327405bb0/ash/wallpaper/wallpaper_controller.cc
[modify] https://crrev.com/25e1bcd846d82970374fb26626a3e4a327405bb0/ash/wallpaper/wallpaper_controller.h
[modify] https://crrev.com/25e1bcd846d82970374fb26626a3e4a327405bb0/chrome/browser/chromeos/extensions/wallpaper_private_api.cc
[modify] https://crrev.com/25e1bcd846d82970374fb26626a3e4a327405bb0/chrome/browser/chromeos/extensions/wallpaper_private_api.h
[modify] https://crrev.com/25e1bcd846d82970374fb26626a3e4a327405bb0/chrome/browser/ui/ash/test_wallpaper_controller.cc
[modify] https://crrev.com/25e1bcd846d82970374fb26626a3e4a327405bb0/chrome/browser/ui/ash/test_wallpaper_controller.h
[modify] https://crrev.com/25e1bcd846d82970374fb26626a3e4a327405bb0/chrome/browser/ui/ash/wallpaper_controller_client.cc
[modify] https://crrev.com/25e1bcd846d82970374fb26626a3e4a327405bb0/chrome/browser/ui/ash/wallpaper_controller_client.h

Project Member

Comment 3 by bugdroid1@chromium.org, Apr 11 2018

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

commit e9330ce826457bc2e16a4a0b01a32f12b3f193ec
Author: Wenzhao Zang <wzang@chromium.org>
Date: Wed Apr 11 07:30:11 2018

cros: Make WallpaperWindowStateManager work under Mash

Create mojo API and move the tests in 'wallpaper_private_api_unittest.cc'
to //ash, because:

1) They're only testing wallpaper_window_state_manager.

2) We want to remove //ash dependency in chrome wallpaper code.

(IIUC)|MultiUserWindowManagerChromeOS| under //chrome is responsible
for tracking the window ownership under multi-profile session. The
wallpaper_window_state_manager is only checking if the window is
visible. So if we move the tests to //ash, they only need to test that
the minimize/restore functions are no-op for invisible windows. Why
the windows are visible/invisible is beyond its scope.

Bug:  827062 
Test: --enable-features=Mash
Change-Id: Ib7573e8911a6979f598f85cbb0b059be1c77b389
Reviewed-on: https://chromium-review.googlesource.com/967885
Commit-Queue: Wenzhao (Colin) Zang <wzang@chromium.org>
Reviewed-by: Oliver Chang <ochang@chromium.org>
Reviewed-by: Xiaoqian Dai <xdai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549811}
[modify] https://crrev.com/e9330ce826457bc2e16a4a0b01a32f12b3f193ec/ash/BUILD.gn
[modify] https://crrev.com/e9330ce826457bc2e16a4a0b01a32f12b3f193ec/ash/public/interfaces/wallpaper.mojom
[modify] https://crrev.com/e9330ce826457bc2e16a4a0b01a32f12b3f193ec/ash/wallpaper/wallpaper_controller.cc
[modify] https://crrev.com/e9330ce826457bc2e16a4a0b01a32f12b3f193ec/ash/wallpaper/wallpaper_controller.h
[modify] https://crrev.com/e9330ce826457bc2e16a4a0b01a32f12b3f193ec/ash/wallpaper/wallpaper_window_state_manager.cc
[modify] https://crrev.com/e9330ce826457bc2e16a4a0b01a32f12b3f193ec/ash/wallpaper/wallpaper_window_state_manager.h
[add] https://crrev.com/e9330ce826457bc2e16a4a0b01a32f12b3f193ec/ash/wallpaper/wallpaper_window_state_manager_unittest.cc
[modify] https://crrev.com/e9330ce826457bc2e16a4a0b01a32f12b3f193ec/chrome/browser/chromeos/extensions/wallpaper_private_api.cc
[modify] https://crrev.com/e9330ce826457bc2e16a4a0b01a32f12b3f193ec/chrome/browser/chromeos/extensions/wallpaper_private_api_unittest.cc
[modify] https://crrev.com/e9330ce826457bc2e16a4a0b01a32f12b3f193ec/chrome/browser/ui/ash/test_wallpaper_controller.cc
[modify] https://crrev.com/e9330ce826457bc2e16a4a0b01a32f12b3f193ec/chrome/browser/ui/ash/test_wallpaper_controller.h
[modify] https://crrev.com/e9330ce826457bc2e16a4a0b01a32f12b3f193ec/chrome/browser/ui/ash/wallpaper_controller_client.cc
[modify] https://crrev.com/e9330ce826457bc2e16a4a0b01a32f12b3f193ec/chrome/browser/ui/ash/wallpaper_controller_client.h

Project Member

Comment 4 by bugdroid1@chromium.org, Apr 13 2018

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

commit 04868179df19c6b76e91e7682ea9056a114546db
Author: Wenzhao Zang <wzang@chromium.org>
Date: Fri Apr 13 00:55:26 2018

mash: Remove WallpaperControllerObserver dependency in //chrome

1) Changed all the observers in //chrome to use the mojo interface.

2) Since views-based login may still take some time to be fully ready,
   prefer to remove the dependency in signin_screen_handler now. Since
   app list and login screen are two main observers of wallpaper events
   we'll be able to clean up some mojo interface after both are moved
   to //ash.

Bug:  827062 
Test: --enable-features=Mash
Change-Id: I81a26faa8ccb29c472ed4dc2257604d67f56e897
Reviewed-on: https://chromium-review.googlesource.com/998814
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Xiaoqian Dai <xdai@chromium.org>
Reviewed-by: Sam McNally <sammc@chromium.org>
Commit-Queue: Wenzhao (Colin) Zang <wzang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550448}
[modify] https://crrev.com/04868179df19c6b76e91e7682ea9056a114546db/ash/public/cpp/wallpaper_types.h
[modify] https://crrev.com/04868179df19c6b76e91e7682ea9056a114546db/ash/public/interfaces/wallpaper.mojom
[modify] https://crrev.com/04868179df19c6b76e91e7682ea9056a114546db/ash/shelf/shelf_background_animator.cc
[modify] https://crrev.com/04868179df19c6b76e91e7682ea9056a114546db/ash/shelf/shelf_background_animator.h
[modify] https://crrev.com/04868179df19c6b76e91e7682ea9056a114546db/ash/wallpaper/wallpaper_controller.cc
[modify] https://crrev.com/04868179df19c6b76e91e7682ea9056a114546db/ash/wallpaper/wallpaper_controller.h
[modify] https://crrev.com/04868179df19c6b76e91e7682ea9056a114546db/ash/wallpaper/wallpaper_controller_observer.h
[modify] https://crrev.com/04868179df19c6b76e91e7682ea9056a114546db/ash/wallpaper/wallpaper_controller_test_api.h
[modify] https://crrev.com/04868179df19c6b76e91e7682ea9056a114546db/ash/wallpaper/wallpaper_controller_unittest.cc
[modify] https://crrev.com/04868179df19c6b76e91e7682ea9056a114546db/ash/wallpaper/wallpaper_view.cc
[modify] https://crrev.com/04868179df19c6b76e91e7682ea9056a114546db/ash/wm/overview/window_grid.cc
[modify] https://crrev.com/04868179df19c6b76e91e7682ea9056a114546db/ash/wm/workspace/backdrop_controller.cc
[modify] https://crrev.com/04868179df19c6b76e91e7682ea9056a114546db/ash/wm/workspace/backdrop_controller.h
[modify] https://crrev.com/04868179df19c6b76e91e7682ea9056a114546db/chrome/browser/chromeos/arc/wallpaper/arc_wallpaper_service.cc
[modify] https://crrev.com/04868179df19c6b76e91e7682ea9056a114546db/chrome/browser/chromeos/arc/wallpaper/arc_wallpaper_service.h
[modify] https://crrev.com/04868179df19c6b76e91e7682ea9056a114546db/chrome/browser/chromeos/customization/customization_wallpaper_downloader_browsertest.cc
[modify] https://crrev.com/04868179df19c6b76e91e7682ea9056a114546db/chrome/browser/chromeos/login/kiosk_browsertest.cc
[modify] https://crrev.com/04868179df19c6b76e91e7682ea9056a114546db/chrome/browser/chromeos/login/users/wallpaper_policy_browsertest.cc
[modify] https://crrev.com/04868179df19c6b76e91e7682ea9056a114546db/chrome/browser/ui/app_list/app_list_view_delegate.cc
[modify] https://crrev.com/04868179df19c6b76e91e7682ea9056a114546db/chrome/browser/ui/app_list/app_list_view_delegate.h
[modify] https://crrev.com/04868179df19c6b76e91e7682ea9056a114546db/chrome/browser/ui/ash/test_wallpaper_controller.cc
[modify] https://crrev.com/04868179df19c6b76e91e7682ea9056a114546db/chrome/browser/ui/ash/test_wallpaper_controller.h
[modify] https://crrev.com/04868179df19c6b76e91e7682ea9056a114546db/chrome/browser/ui/ash/wallpaper_controller_client.cc
[modify] https://crrev.com/04868179df19c6b76e91e7682ea9056a114546db/chrome/browser/ui/ash/wallpaper_controller_client.h
[modify] https://crrev.com/04868179df19c6b76e91e7682ea9056a114546db/chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc
[modify] https://crrev.com/04868179df19c6b76e91e7682ea9056a114546db/chrome/browser/ui/webui/chromeos/login/signin_screen_handler.h

Project Member

Comment 5 by bugdroid1@chromium.org, Apr 14 2018

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

commit 4beb478f436f4b32d5ae37e23522cec229fd1d20
Author: Wenzhao Zang <wzang@chromium.org>
Date: Sat Apr 14 00:55:28 2018

cros: Add OnWallpaperBlurChanged calls

CL 998814 didn't add calls to notify observers when blur state changes.
(Although, at this point there's no code path of changing blur states
at login screen, in the future there'll be support for switching
between device/non-device wallpapers and the blur states will change
dynamically.)

Bug:  827062 
Test: --enable-features=Mash
Change-Id: I4bc57fd397d55e2f6a927cbafa8a39de05d5bb1a
Reviewed-on: https://chromium-review.googlesource.com/1013147
Reviewed-by: Weidong Guo <weidongg@chromium.org>
Commit-Queue: Wenzhao (Colin) Zang <wzang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550840}
[modify] https://crrev.com/4beb478f436f4b32d5ae37e23522cec229fd1d20/ash/wallpaper/wallpaper_controller.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Apr 14 2018

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

commit b02a519ea14995daa57bedecbb9f83f71c81218b
Author: Wenzhao Zang <wzang@chromium.org>
Date: Sat Apr 14 09:00:41 2018

cros: Remove AshTestBase in ArcWallpaperServiceTest

All the AshTestBase inheritance in //chrome should be removed
eventually.

Bug:  827062 
Change-Id: I78272914a03f75267b72d478bdfe2381a87203df
Reviewed-on: https://chromium-review.googlesource.com/1012307
Reviewed-by: Alexander Alekseev <alemate@chromium.org>
Commit-Queue: Wenzhao (Colin) Zang <wzang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550888}
[modify] https://crrev.com/b02a519ea14995daa57bedecbb9f83f71c81218b/chrome/browser/chromeos/arc/wallpaper/arc_wallpaper_service_unittest.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Apr 17 2018

Labels: merge-merged-testbranch
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/04868179df19c6b76e91e7682ea9056a114546db

commit 04868179df19c6b76e91e7682ea9056a114546db
Author: Wenzhao Zang <wzang@chromium.org>
Date: Fri Apr 13 00:55:26 2018

mash: Remove WallpaperControllerObserver dependency in //chrome

1) Changed all the observers in //chrome to use the mojo interface.

2) Since views-based login may still take some time to be fully ready,
   prefer to remove the dependency in signin_screen_handler now. Since
   app list and login screen are two main observers of wallpaper events
   we'll be able to clean up some mojo interface after both are moved
   to //ash.

Bug:  827062 
Test: --enable-features=Mash
Change-Id: I81a26faa8ccb29c472ed4dc2257604d67f56e897
Reviewed-on: https://chromium-review.googlesource.com/998814
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Xiaoqian Dai <xdai@chromium.org>
Reviewed-by: Sam McNally <sammc@chromium.org>
Commit-Queue: Wenzhao (Colin) Zang <wzang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550448}
[modify] https://crrev.com/04868179df19c6b76e91e7682ea9056a114546db/ash/public/cpp/wallpaper_types.h
[modify] https://crrev.com/04868179df19c6b76e91e7682ea9056a114546db/ash/public/interfaces/wallpaper.mojom
[modify] https://crrev.com/04868179df19c6b76e91e7682ea9056a114546db/ash/shelf/shelf_background_animator.cc
[modify] https://crrev.com/04868179df19c6b76e91e7682ea9056a114546db/ash/shelf/shelf_background_animator.h
[modify] https://crrev.com/04868179df19c6b76e91e7682ea9056a114546db/ash/wallpaper/wallpaper_controller.cc
[modify] https://crrev.com/04868179df19c6b76e91e7682ea9056a114546db/ash/wallpaper/wallpaper_controller.h
[modify] https://crrev.com/04868179df19c6b76e91e7682ea9056a114546db/ash/wallpaper/wallpaper_controller_observer.h
[modify] https://crrev.com/04868179df19c6b76e91e7682ea9056a114546db/ash/wallpaper/wallpaper_controller_test_api.h
[modify] https://crrev.com/04868179df19c6b76e91e7682ea9056a114546db/ash/wallpaper/wallpaper_controller_unittest.cc
[modify] https://crrev.com/04868179df19c6b76e91e7682ea9056a114546db/ash/wallpaper/wallpaper_view.cc
[modify] https://crrev.com/04868179df19c6b76e91e7682ea9056a114546db/ash/wm/overview/window_grid.cc
[modify] https://crrev.com/04868179df19c6b76e91e7682ea9056a114546db/ash/wm/workspace/backdrop_controller.cc
[modify] https://crrev.com/04868179df19c6b76e91e7682ea9056a114546db/ash/wm/workspace/backdrop_controller.h
[modify] https://crrev.com/04868179df19c6b76e91e7682ea9056a114546db/chrome/browser/chromeos/arc/wallpaper/arc_wallpaper_service.cc
[modify] https://crrev.com/04868179df19c6b76e91e7682ea9056a114546db/chrome/browser/chromeos/arc/wallpaper/arc_wallpaper_service.h
[modify] https://crrev.com/04868179df19c6b76e91e7682ea9056a114546db/chrome/browser/chromeos/customization/customization_wallpaper_downloader_browsertest.cc
[modify] https://crrev.com/04868179df19c6b76e91e7682ea9056a114546db/chrome/browser/chromeos/login/kiosk_browsertest.cc
[modify] https://crrev.com/04868179df19c6b76e91e7682ea9056a114546db/chrome/browser/chromeos/login/users/wallpaper_policy_browsertest.cc
[modify] https://crrev.com/04868179df19c6b76e91e7682ea9056a114546db/chrome/browser/ui/app_list/app_list_view_delegate.cc
[modify] https://crrev.com/04868179df19c6b76e91e7682ea9056a114546db/chrome/browser/ui/app_list/app_list_view_delegate.h
[modify] https://crrev.com/04868179df19c6b76e91e7682ea9056a114546db/chrome/browser/ui/ash/test_wallpaper_controller.cc
[modify] https://crrev.com/04868179df19c6b76e91e7682ea9056a114546db/chrome/browser/ui/ash/test_wallpaper_controller.h
[modify] https://crrev.com/04868179df19c6b76e91e7682ea9056a114546db/chrome/browser/ui/ash/wallpaper_controller_client.cc
[modify] https://crrev.com/04868179df19c6b76e91e7682ea9056a114546db/chrome/browser/ui/ash/wallpaper_controller_client.h
[modify] https://crrev.com/04868179df19c6b76e91e7682ea9056a114546db/chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc
[modify] https://crrev.com/04868179df19c6b76e91e7682ea9056a114546db/chrome/browser/ui/webui/chromeos/login/signin_screen_handler.h

Project Member

Comment 8 by bugdroid1@chromium.org, Apr 17 2018

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

commit 4beb478f436f4b32d5ae37e23522cec229fd1d20
Author: Wenzhao Zang <wzang@chromium.org>
Date: Sat Apr 14 00:55:28 2018

cros: Add OnWallpaperBlurChanged calls

CL 998814 didn't add calls to notify observers when blur state changes.
(Although, at this point there's no code path of changing blur states
at login screen, in the future there'll be support for switching
between device/non-device wallpapers and the blur states will change
dynamically.)

Bug:  827062 
Test: --enable-features=Mash
Change-Id: I4bc57fd397d55e2f6a927cbafa8a39de05d5bb1a
Reviewed-on: https://chromium-review.googlesource.com/1013147
Reviewed-by: Weidong Guo <weidongg@chromium.org>
Commit-Queue: Wenzhao (Colin) Zang <wzang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550840}
[modify] https://crrev.com/4beb478f436f4b32d5ae37e23522cec229fd1d20/ash/wallpaper/wallpaper_controller.cc

Project Member

Comment 9 by bugdroid1@chromium.org, Apr 17 2018

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

commit b02a519ea14995daa57bedecbb9f83f71c81218b
Author: Wenzhao Zang <wzang@chromium.org>
Date: Sat Apr 14 09:00:41 2018

cros: Remove AshTestBase in ArcWallpaperServiceTest

All the AshTestBase inheritance in //chrome should be removed
eventually.

Bug:  827062 
Change-Id: I78272914a03f75267b72d478bdfe2381a87203df
Reviewed-on: https://chromium-review.googlesource.com/1012307
Reviewed-by: Alexander Alekseev <alemate@chromium.org>
Commit-Queue: Wenzhao (Colin) Zang <wzang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550888}
[modify] https://crrev.com/b02a519ea14995daa57bedecbb9f83f71c81218b/chrome/browser/chromeos/arc/wallpaper/arc_wallpaper_service_unittest.cc

Project Member

Comment 10 by bugdroid1@chromium.org, Apr 20 2018

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

commit e88248f6c5c8f2a8e0d07ea506a9a2728051e2fd
Author: Wenzhao Zang <wzang@chromium.org>
Date: Fri Apr 20 02:49:24 2018

mash: Remove //ash dependency in wallpaper_private_api.cc

We need this because:
1) Mustash requires all //ash dependency except //ash/public to be
   removed from //chrome.
2) Currently saving and retrieving online wallpapers are handled
   separately in //chrome and //ash, and it has duplicate logic in the
   path calculation.
3) Currently wallpaper_private_api.cc is responsible for saving online
   wallpapers, so it defeats the goal of making wallpaper paths
   internal to //ash.

Also fixed a bug: on Linux build, online wallpaper is lost at login
screen.

Bug:  827062 
Change-Id: I0d3ea5daf537db6e189c7a8b2ede469521e9dcbf
Reviewed-on: https://chromium-review.googlesource.com/1013211
Reviewed-by: Sam McNally <sammc@chromium.org>
Reviewed-by: Xiaoqian Dai <xdai@chromium.org>
Commit-Queue: Wenzhao (Colin) Zang <wzang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552244}
[modify] https://crrev.com/e88248f6c5c8f2a8e0d07ea506a9a2728051e2fd/ash/public/interfaces/wallpaper.mojom
[modify] https://crrev.com/e88248f6c5c8f2a8e0d07ea506a9a2728051e2fd/ash/wallpaper/wallpaper_controller.cc
[modify] https://crrev.com/e88248f6c5c8f2a8e0d07ea506a9a2728051e2fd/ash/wallpaper/wallpaper_controller.h
[modify] https://crrev.com/e88248f6c5c8f2a8e0d07ea506a9a2728051e2fd/ash/wallpaper/wallpaper_controller_unittest.cc
[modify] https://crrev.com/e88248f6c5c8f2a8e0d07ea506a9a2728051e2fd/chrome/browser/chromeos/extensions/wallpaper_function_base.cc
[modify] https://crrev.com/e88248f6c5c8f2a8e0d07ea506a9a2728051e2fd/chrome/browser/chromeos/extensions/wallpaper_function_base.h
[modify] https://crrev.com/e88248f6c5c8f2a8e0d07ea506a9a2728051e2fd/chrome/browser/chromeos/extensions/wallpaper_private_api.cc
[modify] https://crrev.com/e88248f6c5c8f2a8e0d07ea506a9a2728051e2fd/chrome/browser/chromeos/extensions/wallpaper_private_api.h
[modify] https://crrev.com/e88248f6c5c8f2a8e0d07ea506a9a2728051e2fd/chrome/browser/ui/ash/test_wallpaper_controller.cc
[modify] https://crrev.com/e88248f6c5c8f2a8e0d07ea506a9a2728051e2fd/chrome/browser/ui/ash/test_wallpaper_controller.h
[modify] https://crrev.com/e88248f6c5c8f2a8e0d07ea506a9a2728051e2fd/chrome/browser/ui/ash/wallpaper_controller_client.cc
[modify] https://crrev.com/e88248f6c5c8f2a8e0d07ea506a9a2728051e2fd/chrome/browser/ui/ash/wallpaper_controller_client.h
[modify] https://crrev.com/e88248f6c5c8f2a8e0d07ea506a9a2728051e2fd/chrome/test/data/extensions/api_test/wallpaper_manager/test.js

Project Member

Comment 11 by bugdroid1@chromium.org, Apr 20 2018

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

commit d23e644f95b2f0d21f34d44ecf5adc9a59914df6
Author: Morten Stenshorne <mstensho@chromium.org>
Date: Fri Apr 20 10:28:26 2018

Revert "mash: Remove //ash dependency in wallpaper_private_api.cc"

This reverts commit e88248f6c5c8f2a8e0d07ea506a9a2728051e2fd.

Reason for revert: Caused bot failure/flakiness;  crbug.com/835242 

Original change's description:
> mash: Remove //ash dependency in wallpaper_private_api.cc
> 
> We need this because:
> 1) Mustash requires all //ash dependency except //ash/public to be
>    removed from //chrome.
> 2) Currently saving and retrieving online wallpapers are handled
>    separately in //chrome and //ash, and it has duplicate logic in the
>    path calculation.
> 3) Currently wallpaper_private_api.cc is responsible for saving online
>    wallpapers, so it defeats the goal of making wallpaper paths
>    internal to //ash.
> 
> Also fixed a bug: on Linux build, online wallpaper is lost at login
> screen.
> 
> Bug:  827062 
> Change-Id: I0d3ea5daf537db6e189c7a8b2ede469521e9dcbf
> Reviewed-on: https://chromium-review.googlesource.com/1013211
> Reviewed-by: Sam McNally <sammc@chromium.org>
> Reviewed-by: Xiaoqian Dai <xdai@chromium.org>
> Commit-Queue: Wenzhao (Colin) Zang <wzang@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#552244}

TBR=sammc@chromium.org,xdai@chromium.org,wzang@chromium.org

Change-Id: If32a841f67435ba6066c234eca5fce41678cca1c
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  827062 
Reviewed-on: https://chromium-review.googlesource.com/1021630
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552299}
[modify] https://crrev.com/d23e644f95b2f0d21f34d44ecf5adc9a59914df6/ash/public/interfaces/wallpaper.mojom
[modify] https://crrev.com/d23e644f95b2f0d21f34d44ecf5adc9a59914df6/ash/wallpaper/wallpaper_controller.cc
[modify] https://crrev.com/d23e644f95b2f0d21f34d44ecf5adc9a59914df6/ash/wallpaper/wallpaper_controller.h
[modify] https://crrev.com/d23e644f95b2f0d21f34d44ecf5adc9a59914df6/ash/wallpaper/wallpaper_controller_unittest.cc
[modify] https://crrev.com/d23e644f95b2f0d21f34d44ecf5adc9a59914df6/chrome/browser/chromeos/extensions/wallpaper_function_base.cc
[modify] https://crrev.com/d23e644f95b2f0d21f34d44ecf5adc9a59914df6/chrome/browser/chromeos/extensions/wallpaper_function_base.h
[modify] https://crrev.com/d23e644f95b2f0d21f34d44ecf5adc9a59914df6/chrome/browser/chromeos/extensions/wallpaper_private_api.cc
[modify] https://crrev.com/d23e644f95b2f0d21f34d44ecf5adc9a59914df6/chrome/browser/chromeos/extensions/wallpaper_private_api.h
[modify] https://crrev.com/d23e644f95b2f0d21f34d44ecf5adc9a59914df6/chrome/browser/ui/ash/test_wallpaper_controller.cc
[modify] https://crrev.com/d23e644f95b2f0d21f34d44ecf5adc9a59914df6/chrome/browser/ui/ash/test_wallpaper_controller.h
[modify] https://crrev.com/d23e644f95b2f0d21f34d44ecf5adc9a59914df6/chrome/browser/ui/ash/wallpaper_controller_client.cc
[modify] https://crrev.com/d23e644f95b2f0d21f34d44ecf5adc9a59914df6/chrome/browser/ui/ash/wallpaper_controller_client.h
[modify] https://crrev.com/d23e644f95b2f0d21f34d44ecf5adc9a59914df6/chrome/test/data/extensions/api_test/wallpaper_manager/test.js

Project Member

Comment 12 by bugdroid1@chromium.org, Apr 20 2018

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

commit b932cd20fb8b36d18718e0600ec7def620fe2e73
Author: Wenzhao Zang <wzang@chromium.org>
Date: Fri Apr 20 22:54:53 2018

Reland "mash: Remove //ash dependency in wallpaper_private_api.cc"

This is a reland of e88248f6c5c8f2a8e0d07ea506a9a2728051e2fd

The original CL makes a test flaky. ( crbug.com/835242 )

The detailed failure reason is, in the refactored code, saving the
wallpaper file does not (and should not) block the response to the
wallpaper picker renderer, so the test fails when the file saving
operation hasn't completed. Since the CL moved the implementation
to //ash, that test should ideally be deleted since //ash already
has the same test coverage.

Original change's description:
> mash: Remove //ash dependency in wallpaper_private_api.cc
>
> We need this because:
> 1) Mustash requires all //ash dependency except //ash/public to be
>    removed from //chrome.
> 2) Currently saving and retrieving online wallpapers are handled
>    separately in //chrome and //ash, and it has duplicate logic in the
>    path calculation.
> 3) Currently wallpaper_private_api.cc is responsible for saving online
>    wallpapers, so it defeats the goal of making wallpaper paths
>    internal to //ash.
>
> Also fixed a bug: on Linux build, online wallpaper is lost at login
> screen.
>
> Bug:  827062 
> Change-Id: I0d3ea5daf537db6e189c7a8b2ede469521e9dcbf
> Reviewed-on: https://chromium-review.googlesource.com/1013211
> Reviewed-by: Sam McNally <sammc@chromium.org>
> Reviewed-by: Xiaoqian Dai <xdai@chromium.org>
> Commit-Queue: Wenzhao (Colin) Zang <wzang@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#552244}

TBR=xdai@chromium.org, sammc@chromium.org

Bug:  827062 
Change-Id: I6cef49c2f17048e8412e1006088ab1ccb32c2f9b
Reviewed-on: https://chromium-review.googlesource.com/1022431
Reviewed-by: Wenzhao (Colin) Zang <wzang@chromium.org>
Commit-Queue: Wenzhao (Colin) Zang <wzang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552507}
[modify] https://crrev.com/b932cd20fb8b36d18718e0600ec7def620fe2e73/ash/public/interfaces/wallpaper.mojom
[modify] https://crrev.com/b932cd20fb8b36d18718e0600ec7def620fe2e73/ash/wallpaper/wallpaper_controller.cc
[modify] https://crrev.com/b932cd20fb8b36d18718e0600ec7def620fe2e73/ash/wallpaper/wallpaper_controller.h
[modify] https://crrev.com/b932cd20fb8b36d18718e0600ec7def620fe2e73/ash/wallpaper/wallpaper_controller_unittest.cc
[modify] https://crrev.com/b932cd20fb8b36d18718e0600ec7def620fe2e73/chrome/browser/chromeos/extensions/wallpaper_function_base.cc
[modify] https://crrev.com/b932cd20fb8b36d18718e0600ec7def620fe2e73/chrome/browser/chromeos/extensions/wallpaper_function_base.h
[modify] https://crrev.com/b932cd20fb8b36d18718e0600ec7def620fe2e73/chrome/browser/chromeos/extensions/wallpaper_private_api.cc
[modify] https://crrev.com/b932cd20fb8b36d18718e0600ec7def620fe2e73/chrome/browser/chromeos/extensions/wallpaper_private_api.h
[modify] https://crrev.com/b932cd20fb8b36d18718e0600ec7def620fe2e73/chrome/browser/ui/ash/test_wallpaper_controller.cc
[modify] https://crrev.com/b932cd20fb8b36d18718e0600ec7def620fe2e73/chrome/browser/ui/ash/test_wallpaper_controller.h
[modify] https://crrev.com/b932cd20fb8b36d18718e0600ec7def620fe2e73/chrome/browser/ui/ash/wallpaper_controller_client.cc
[modify] https://crrev.com/b932cd20fb8b36d18718e0600ec7def620fe2e73/chrome/browser/ui/ash/wallpaper_controller_client.h
[modify] https://crrev.com/b932cd20fb8b36d18718e0600ec7def620fe2e73/chrome/test/data/extensions/api_test/wallpaper_manager/test.js

Project Member

Comment 13 by bugdroid1@chromium.org, Apr 23 2018

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

commit cb9d36ddfbbce2ac659d7230685c1067eaf7bd9a
Author: Wenzhao Zang <wzang@chromium.org>
Date: Mon Apr 23 18:19:24 2018

mash: Remove //ash/wallpaper dependency in //c/b/chromeos/customization

1) Prefer to keep the implementation of customized default wallpapers
   in //chrome.

2) There's no need to put the constants of small/large file suffixes
   to //ash/public, since they are implementation details, and //ash
   only needs to know the entire file path.

3) The original idea was to put
   |ash::WallpaperController::ResizeAndSaveWallpaper| to //ash/public
   but customized default wallpaper only needs a simplified version of
   it, so it's inlined.

Bug:  827062 
Change-Id: I4c94adff973af697ea5f929ba751eb2fe8f019de
Reviewed-on: https://chromium-review.googlesource.com/1015744
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Wenzhao (Colin) Zang <wzang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552753}
[modify] https://crrev.com/cb9d36ddfbbce2ac659d7230685c1067eaf7bd9a/ash/public/cpp/wallpaper_types.h
[modify] https://crrev.com/cb9d36ddfbbce2ac659d7230685c1067eaf7bd9a/ash/wallpaper/wallpaper_controller.cc
[modify] https://crrev.com/cb9d36ddfbbce2ac659d7230685c1067eaf7bd9a/ash/wallpaper/wallpaper_controller.h
[modify] https://crrev.com/cb9d36ddfbbce2ac659d7230685c1067eaf7bd9a/ash/wallpaper/wallpaper_controller_unittest.cc
[modify] https://crrev.com/cb9d36ddfbbce2ac659d7230685c1067eaf7bd9a/chrome/browser/chromeos/customization/customization_document.cc
[modify] https://crrev.com/cb9d36ddfbbce2ac659d7230685c1067eaf7bd9a/chrome/browser/chromeos/customization/customization_wallpaper_downloader_browsertest.cc
[modify] https://crrev.com/cb9d36ddfbbce2ac659d7230685c1067eaf7bd9a/chrome/browser/chromeos/customization/customization_wallpaper_util.cc
[modify] https://crrev.com/cb9d36ddfbbce2ac659d7230685c1067eaf7bd9a/chrome/browser/chromeos/customization/customization_wallpaper_util.h
[modify] https://crrev.com/cb9d36ddfbbce2ac659d7230685c1067eaf7bd9a/chrome/browser/ui/ash/wallpaper_controller_client.cc
[modify] https://crrev.com/cb9d36ddfbbce2ac659d7230685c1067eaf7bd9a/chrome/browser/ui/webui/chromeos/login/DEPS

Comment 14 by wzang@chromium.org, Apr 24 2018

Status: Fixed (was: Started)

Sign in to add a comment