New issue
Advanced search Search tips

Issue 793423 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2018
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 776464



Sign in to add a comment

Remove legacy wallpaper code

Project Member Reported by wzang@chromium.org, Dec 8 2017

Issue description

Some legacy code paths in WallpaperManager should be safe for removal, such as some migration code:

https://codereview.chromium.org/23480087
https://chromiumcodereview.appspot.com/10827154 

We will first evaluate the impact of removing them.
 

Comment 1 by wzang@chromium.org, Dec 8 2017

Status: Started (was: Available)
Project Member

Comment 2 by bugdroid1@chromium.org, Dec 21 2017

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

commit a9c94615e538efbc230237efd0acd6b65cdd2b50
Author: Wenzhao Zang <wzang@chromium.org>
Date: Thu Dec 21 04:57:06 2017

Remove legacy wallpaper files id migration and login caching code

Removed the code added by the following two CLs:

1) https://codereview.chromium.org/23480087 which migrates the custom
   wallpaper directory to a new place with new name. It was over 4
   years ago and it's reasonable to remove them.

2) https://chromiumcodereview.appspot.com/10827154 which pre-caches the
   user wallpapers at login screen, with the goal of showing wallpapers
   faster when there're multiple user pods being toggled. It should
   be safe to remove because:

   a) It wasn't caching the main user's wallpaper anyways, but only
      for the other unfocused user pods, but after the redesign of the
      login screen, we already disabled small user pods to submit
      wallpaper change request from the front end
      (https://chromium-review.googlesource.com/c/chromium/src/+/580376
      ) So users basically have no ways to toggle the wallpaper at a
      very fast pace.

   b) It can only cache up to 3 users. In the cases of 3+ users, we
      should already hear complaints if the caching had a big effect.

   c) Custom wallpapers are cached as long as they are shown once by
      another code path. So that CL only affected the first time that
      the wallpaper is shown.

3) The file wallpaper_manager_unittest.cc is removed since it's only
   testing a single removed function (ClearDisposableWallpaperCache).
   This function was only used once which is when a user logs in,
   clears the cached wallpapers of other user pods at login screen.
   This optimization should have limited impact but causes extra burden
   to maintain.

Test: Manual
Bug:  793423 
Change-Id: I6847fa7d29eab2030e6ee2ba2a008a0442889a08
Reviewed-on: https://chromium-review.googlesource.com/817700
Commit-Queue: Wenzhao (Colin) Zang <wzang@chromium.org>
Reviewed-by: Biao She <bshe@chromium.org>
Reviewed-by: Alexander Alekseev <alemate@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525615}
[modify] https://crrev.com/a9c94615e538efbc230237efd0acd6b65cdd2b50/chrome/browser/chromeos/BUILD.gn
[modify] https://crrev.com/a9c94615e538efbc230237efd0acd6b65cdd2b50/chrome/browser/chromeos/login/users/wallpaper/wallpaper_manager.cc
[modify] https://crrev.com/a9c94615e538efbc230237efd0acd6b65cdd2b50/chrome/browser/chromeos/login/users/wallpaper/wallpaper_manager.h
[modify] https://crrev.com/a9c94615e538efbc230237efd0acd6b65cdd2b50/chrome/browser/chromeos/login/users/wallpaper/wallpaper_manager_browsertest.cc
[delete] https://crrev.com/58f89c5fab07cfd001972798387abe414c419559/chrome/browser/chromeos/login/users/wallpaper/wallpaper_manager_unittest.cc

Comment 3 by wzang@chromium.org, Jan 5 2018

Status: Fixed (was: Started)

Sign in to add a comment