Fix custom wallpaper going back from wallpaper 2 to wallpaper 1

chrome/browser/resources/chromeos/wallpaper_manager/js/wallpaper_manager.js

 // Copyright (c) 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 /**
  * WallpaperManager constructor.
  *
  * WallpaperManager objects encapsulate the functionality of the wallpaper
  * manager extension.
  *
@@ skipping 601 matching lines @@
         msg = 'Unknown Error';
         break;
     }
     console.error('Error: ' + msg);
     this.showError_(str('accessFileFailure'));
   };
 
   /**
    * Handles click on a different thumbnail in wallpaper grid.
    */
   WallpaperManager.prototype.onThumbnailSelectionChanged_ = function() {

[flackr, 2013/04/16 01:28:41]

This method and description seem to be inaccurately described. This is handling
a change of the wallpaper model which may have changed the selected thumbnail.

[bshe, 2013/04/16 16:19:26]

Done.

     var selectedItem = this.wallpaperGrid_.selectedItem;
-    if (selectedItem && selectedItem.source == 'ADDNEW')
+    this.setWallpaperAttribution_(selectedItem);
+
+    // splice may dispatch a change event because the position of selected
+    // element changing. But the actual selected element didn't change after
+    // splice. Check if the new selected element equals to the previous selected
+    // element before continuing. Otherwise, wallpaper may reset to previous one
+    // as described in http://crbug.com/229036.
+    if (!selectedItem || selectedItem.source == 'ADDNEW' ||
+        selectedItem == this.wallpaperGrid_.previousSelectedItem) {
       return;
+    }
 
-    if (selectedItem && selectedItem.baseURL &&
-        !this.wallpaperGrid_.inProgramSelection) {
+    this.wallpaperGrid_.previousSelectedItem = selectedItem;

[flackr, 2013/04/16 01:28:41]

previousSelectedItem doesn't belong in wallpaperGrid since wallpaperGrid doesn't
care whether the selectedItem actually change. You could store the
active/selected/displayed (whichever you think is the best name) item as a
member of this class and only act if it changes. Could this not be checked
against this.currentWallpaper or this.wallpaperGrid_.activeItem or would that
not be correct?

[bshe, 2013/04/16 16:19:26]

I can't use currentWallpaper or activeItem to replace previousSelectedItem. They
are not always the same. For example, if we select a thumbnail and failed to set
the corresponding wallpaper, activeItem and currentWallpaper wont change but the
selectedItem changed. Then we select another item, the previousSelectedItem !=
currentWallpaper or activeItem in that case.

You mean just move previousSelectedItem to this class, right? I have created a
new onChange_ function to call this function only when selectedItem changes.

On 2013/04/16 01:28:41, flackr wrote:

+    if (selectedItem.baseURL && !this.wallpaperGrid_.inProgramSelection) {
       if (selectedItem.source == wallpapers.WallpaperSourceEnum.Custom) {
         var items = {};
         var key = selectedItem.baseURL;
         var self = this;
         this.storage_.get(key, function(items) {
           selectedItem.layout = items[key] ? items[key] : 'CENTER_CROPPED';
           self.setSelectedWallpaper_(selectedItem);
         });
       } else {
         this.setSelectedWallpaper_(selectedItem);
       }
     }
-    this.setWallpaperAttribution_(selectedItem);
   };
 
   /**
    * Set attributions of wallpaper with given URL. If URL is not valid, clear
    * the attributions.
    * @param {{baseURL: string, dynamicURL: string, layout: string,
    *          author: string, authorWebsite: string, availableOffline: boolean}}
    *     selectedItem selected wallpaper item in grid.
    * @private
    */
@@ skipping 110 matching lines @@
                 $('set-wallpaper-layout').disabled = false;
                 var wallpaperInfo = {
                   baseURL: fileName,
                   layout: layout,
                   source: wallpapers.WallpaperSourceEnum.Custom,
                   availableOffline: true
                 };
                 self.currentWallpaper_ = fileName;
                 var items = {};
                 items[self.currentWallpaper_] = layout;
-                self.storage_.set(items, function() {});
                 self.wallpaperGrid_.dataModel.splice(0, 0, wallpaperInfo);
                 self.wallpaperGrid_.selectedItem = wallpaperInfo;
                 self.wallpaperGrid_.activeItem = wallpaperInfo;
               };
 
               fileWriter.onerror = errorHandler;
 
               var blob = new Blob([new Int8Array(thumbnail)],
                                   {'type' : 'image\/jpeg'});
               fileWriter.write(blob);
@@ skipping 234 matching lines @@
           }
         }
       }
       this.wallpaperGrid_.dataModel = wallpapersDataModel;
       this.wallpaperGrid_.selectedItem = selectedItem;
       this.wallpaperGrid_.activeItem = selectedItem;
     }
   };
 
 })();