Use default wallpaper at login screen when user choose a custom wallpaper

Use default wallpaper at login screen when user choose a custom wallpaper


BUG=143004
TEST=Set a custom wallpaper, logout. At login screen, click that user's profile
picture. The wallpaper should be default wallpaper. Login again, the user custom
wallpaper should display again.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=152045

[bshe, 2012-08-16 15:00:14]

Hi Ivan.

Could you please take a look at this CL?

For security reason, we now load the default wallpaper instead of custom
wallpaper(if user chose one) at login screen.

Thanks

[Ivan Korotkov, 2012-08-16 15:45:01]

LGTM but please wait for a clear resolution whether we really want to do so
before submitting. It doesn't make any sense to me since there is still the same
issue with user images.

Also, please consider adding some hint to the user (in a different CL, maybe) in
options UI, otherwise users will consider it a bug that there's a different
wallpaper on the login screen.

https://chromiumcodereview.appspot.com/10827376/diff/4001/chrome/browser/chro...
File chrome/browser/chromeos/login/wallpaper_manager.cc (left):

https://chromiumcodereview.appspot.com/10827376/diff/4001/chrome/browser/chro...
chrome/browser/chromeos/login/wallpaper_manager.cc:403: if
(GetCustomWallpaperFromCache(email, &custom_wallpaper)) {
Why is GetCustomWallpaperFromCache not needed anymore in case of a logged-in
user? Is this because we

https://chromiumcodereview.appspot.com/10827376/diff/4001/chrome/browser/chro...
File chrome/browser/chromeos/login/wallpaper_manager.cc (right):

https://chromiumcodereview.appspot.com/10827376/diff/4001/chrome/browser/chro...
chrome/browser/chromeos/login/wallpaper_manager.cc:403: index =
ash::GetDefaultWallpaperIndex();
Please comment on the purpose of this.

[bshe, 2012-08-16 16:02:59]

On 2012/08/16 15:45:01, Ivan Korotkov wrote:
> LGTM but please wait for a clear resolution whether we really want to do so
> before submitting. It doesn't make any sense to me since there is still the
same
> issue with user images.
> 
> Also, please consider adding some hint to the user (in a different CL, maybe)
in
> options UI, otherwise users will consider it a bug that there's a different
> wallpaper on the login screen.
> 
>
https://chromiumcodereview.appspot.com/10827376/diff/4001/chrome/browser/chro...
> File chrome/browser/chromeos/login/wallpaper_manager.cc (left):
> 
>
https://chromiumcodereview.appspot.com/10827376/diff/4001/chrome/browser/chro...
> chrome/browser/chromeos/login/wallpaper_manager.cc:403: if
> (GetCustomWallpaperFromCache(email, &custom_wallpaper)) {
> Why is GetCustomWallpaperFromCache not needed anymore in case of a logged-in
> user? Is this because we
> 
>
https://chromiumcodereview.appspot.com/10827376/diff/4001/chrome/browser/chro...
> File chrome/browser/chromeos/login/wallpaper_manager.cc (right):
> 
>
https://chromiumcodereview.appspot.com/10827376/diff/4001/chrome/browser/chro...
> chrome/browser/chromeos/login/wallpaper_manager.cc:403: index =
> ash::GetDefaultWallpaperIndex();
> Please comment on the purpose of this.

Thanks. I will wait for the final decision.

[bshe, 2012-08-16 22:32:49]

thanks

http://codereview.chromium.org/10827376/diff/4001/chrome/browser/chromeos/log...
File chrome/browser/chromeos/login/wallpaper_manager.cc (left):

http://codereview.chromium.org/10827376/diff/4001/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/wallpaper_manager.cc:403: if
(GetCustomWallpaperFromCache(email, &custom_wallpaper)) {
If we cache (load from file and decode it) the custom wallpaper at login screen,
the only difference from display it on screen is that we dont call OnPaint
function to paint it. My understanding is that the security concern is mostly
about the decoding process. So I removed the cache custom wallpaper in this Cl
as well. In that case, we never cache a custom wallpaper. So I removed
GetCustomWallpeperFromCache. 

On 2012/08/16 15:45:01, Ivan Korotkov wrote:
> Why is GetCustomWallpaperFromCache not needed anymore in case of a logged-in
> user? Is this because we

http://codereview.chromium.org/10827376/diff/4001/chrome/browser/chromeos/log...
File chrome/browser/chromeos/login/wallpaper_manager.cc (right):

http://codereview.chromium.org/10827376/diff/4001/chrome/browser/chromeos/log...
chrome/browser/chromeos/login/wallpaper_manager.cc:403: index =
ash::GetDefaultWallpaperIndex();
Done

On 2012/08/16 15:45:01, Ivan Korotkov wrote:
> Please comment on the purpose of this.

[commit-bot: I haz the power, 2012-08-16 22:33:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bshe@chromium.org/10827376/7002

[commit-bot: I haz the power, 2012-08-17 00:13:54]

Try job failure for 10827376-7002 (retry) on linux_rel for step
"interactive_ui_tests".
It's a second try, previously, step "interactive_ui_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2012-08-17 01:18:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bshe@chromium.org/10827376/7002

[commit-bot: I haz the power, 2012-08-17 04:38:57]

Change committed as 152045