Restore to user selected wallpaper after browser crash

Restore to user selected wallpaper after browser crash

After browser crash, the wallpaper was reverted to the default wallpaper. This
CL fix the bug.


BUG=118751
TEST=Test on any chromeos device
select a wallpaper different than the default one
go to about:inducebrowsercrashforrealz, the browser will crash and restart
verify if the wallpaper reverted to the default wallpaper after restart.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=128232

[bshe, 2012-03-21 01:23:27]

+flackr for everything
+ben for OWNERS

This CL fixes the wallpaper revert to default wallpaper after browser crash
problem. 

I did some plumbing work to get user selected wallpaper index in ash. And I also
deleted some functions which no longer necessary. 

Thanks!

[flackr, 2012-03-21 17:44:19]

https://chromiumcodereview.appspot.com/9764012/diff/1/ash/desktop_background/...
File ash/desktop_background/desktop_background_controller.cc (right):

https://chromiumcodereview.appspot.com/9764012/diff/1/ash/desktop_background/...
ash/desktop_background/desktop_background_controller.cc:42: void
DesktopBackgroundController::OnUserLoginOrSwitchToImageMode() {
Rather than adding this method, how about changing OnDesktopBackgroundChanged to
not take an index and just get the user wallpaper index. Then you can use one
method for updating the visible background in all cases.

https://chromiumcodereview.appspot.com/9764012/diff/1/ash/shell.cc
File ash/shell.cc (right):

https://chromiumcodereview.appspot.com/9764012/diff/1/ash/shell.cc#newcode604
ash/shell.cc:604:
GetWallpaper(user_wallpaper_delegate_->GetUserWallpaperIndex()));
Seems this would be equivalent to:
desktop_background_controller_->OnUserLoginOrSwitchToImageMode(). Can we use
that?

[bshe, 2012-03-22 02:16:02]

Done. PTAL.

Thanks!

https://chromiumcodereview.appspot.com/9764012/diff/1/ash/desktop_background/...
File ash/desktop_background/desktop_background_controller.cc (right):

https://chromiumcodereview.appspot.com/9764012/diff/1/ash/desktop_background/...
ash/desktop_background/desktop_background_controller.cc:42: void
DesktopBackgroundController::OnUserLoginOrSwitchToImageMode() {
Done.

On 2012/03/21 17:44:20, flackr wrote:
> Rather than adding this method, how about changing OnDesktopBackgroundChanged
to
> not take an index and just get the user wallpaper index. Then you can use one
> method for updating the visible background in all cases.

Done.

https://chromiumcodereview.appspot.com/9764012/diff/1/ash/shell.cc
File ash/shell.cc (right):

https://chromiumcodereview.appspot.com/9764012/diff/1/ash/shell.cc#newcode604
ash/shell.cc:604:
GetWallpaper(user_wallpaper_delegate_->GetUserWallpaperIndex()));
On 2012/03/21 17:44:20, flackr wrote:
> Seems this would be equivalent to:
> desktop_background_controller_->OnUserLoginOrSwitchToImageMode(). Can we use
> that?

Yes and no. Ideally, they should be equivalent. But since we have this issue
117244 not been fixed, we need to create the background widget when shell
initialize and only destroy it when shell got destroyed. So here, I need to use
SetDesktopBackgroundImageMode to initialized the background widget and only
update it in OnDesktopBackgroundChanged function.

[flackr, 2012-03-22 02:26:09]

https://chromiumcodereview.appspot.com/9764012/diff/8003/ash/accelerators/acc...
File ash/accelerators/accelerator_controller.cc (right):

https://chromiumcodereview.appspot.com/9764012/diff/8003/ash/accelerators/acc...
ash/accelerators/accelerator_controller.cc:100:
desktop_background_controller->OnUserLoginOrSwitchToImageMode();
OnDesktopBackgroundChanged?

https://chromiumcodereview.appspot.com/9764012/diff/8003/chrome/browser/chrom...
File chrome/browser/chromeos/background/desktop_background_observer.cc (right):

https://chromiumcodereview.appspot.com/9764012/diff/8003/chrome/browser/chrom...
chrome/browser/chromeos/background/desktop_background_observer.cc:29: //
Guest/incognito user or users haven't logged in (in log in screen) do
Fix sentence. I'm guessing you should say something like:
If at the login screen or logged in as a guest/incognito user, then use the
default wallpaper.

[bshe, 2012-03-22 02:48:11]

Done. Thanks for quick review!

https://chromiumcodereview.appspot.com/9764012/diff/8003/ash/accelerators/acc...
File ash/accelerators/accelerator_controller.cc (right):

https://chromiumcodereview.appspot.com/9764012/diff/8003/ash/accelerators/acc...
ash/accelerators/accelerator_controller.cc:100:
desktop_background_controller->OnUserLoginOrSwitchToImageMode();
On 2012/03/22 02:26:09, flackr wrote:
> OnDesktopBackgroundChanged?

Done.

https://chromiumcodereview.appspot.com/9764012/diff/8003/chrome/browser/chrom...
File chrome/browser/chromeos/background/desktop_background_observer.cc (right):

https://chromiumcodereview.appspot.com/9764012/diff/8003/chrome/browser/chrom...
chrome/browser/chromeos/background/desktop_background_observer.cc:29: //
Guest/incognito user or users haven't logged in (in log in screen) do
You read my mind. :)
On 2012/03/22 02:26:09, flackr wrote:
> Fix sentence. I'm guessing you should say something like:
> If at the login screen or logged in as a guest/incognito user, then use the
> default wallpaper.

[flackr, 2012-03-22 03:19:10]

lgtm with nit.

https://chromiumcodereview.appspot.com/9764012/diff/12009/chrome/browser/chro...
File chrome/browser/chromeos/background/desktop_background_observer.cc (right):

https://chromiumcodereview.appspot.com/9764012/diff/12009/chrome/browser/chro...
chrome/browser/chromeos/background/desktop_background_observer.cc:21: explicit
UserWallpaperDelegate() {
Only use explicit for single argument constructors.

[Ben Goodger (Google), 2012-03-22 03:23:16]

LGTM

https://chromiumcodereview.appspot.com/9764012/diff/12009/chrome/browser/chro...
File chrome/browser/chromeos/background/desktop_background_observer.cc (right):

https://chromiumcodereview.appspot.com/9764012/diff/12009/chrome/browser/chro...
chrome/browser/chromeos/background/desktop_background_observer.cc:27: virtual
const int GetUserWallpaperIndex() OVERRIDE{
nit: space before {

https://chromiumcodereview.appspot.com/9764012/diff/12009/chrome/browser/chro...
chrome/browser/chromeos/background/desktop_background_observer.cc:40: };
private:

DISALLOW_COPY_AND...

[bshe, 2012-03-22 05:21:15]

Done. Thanks!

https://chromiumcodereview.appspot.com/9764012/diff/12009/chrome/browser/chro...
File chrome/browser/chromeos/background/desktop_background_observer.cc (right):

https://chromiumcodereview.appspot.com/9764012/diff/12009/chrome/browser/chro...
chrome/browser/chromeos/background/desktop_background_observer.cc:21: explicit
UserWallpaperDelegate() {
On 2012/03/22 03:19:10, flackr wrote:
> Only use explicit for single argument constructors.

Done.

https://chromiumcodereview.appspot.com/9764012/diff/12009/chrome/browser/chro...
chrome/browser/chromeos/background/desktop_background_observer.cc:27: virtual
const int GetUserWallpaperIndex() OVERRIDE{
On 2012/03/22 03:23:16, Ben Goodger (Google) wrote:
> nit: space before {

Done.

https://chromiumcodereview.appspot.com/9764012/diff/12009/chrome/browser/chro...
chrome/browser/chromeos/background/desktop_background_observer.cc:40: };
On 2012/03/22 03:23:16, Ben Goodger (Google) wrote:
> private:
> 
> DISALLOW_COPY_AND...

Done.