Convert wallpaper picker to v2 app

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

Index: chrome/browser/resources/chromeos/wallpaper_manager/js/wallpaper_manager.js
diff --git a/chrome/browser/resources/chromeos/wallpaper_manager/js/wallpaper_manager.js b/chrome/browser/resources/chromeos/wallpaper_manager/js/wallpaper_manager.js
index c8696a65fae22dd457c0f7d1137627805defb1e0..0d498d6fd5ffb617a6cee70e0be332d0b1c4b262 100644
--- a/chrome/browser/resources/chromeos/wallpaper_manager/js/wallpaper_manager.js
+++ b/chrome/browser/resources/chromeos/wallpaper_manager/js/wallpaper_manager.js
@@ -20,8 +20,8 @@ function WallpaperManager(dialogDom) {
   this.butterBar_ = new ButterBar(this.dialogDom_);
   this.customWallpaperData_ = null;
   this.currentWallpaper_ = null;
+  this.wallpaperRequest_ = null;
   this.fetchManifest_();
-  this.initDom_();
 }
 
 // Anonymous 'namespace'.
@@ -64,51 +64,86 @@ function WallpaperManager(dialogDom) {
   };
 
   /**
-   * Parses a string as manifest(JSON). Sets manifest to an empty object if a
-   * parsing exception is catched.
-   * @param {string} response The string to parse as JSON.
-   */
-  WallpaperManager.prototype.parseManifest_ = function(response) {
-    try {
-      this.manifest_ = JSON.parse(response);
-    } catch (e) {
-      this.butterBar_.showError_('Failed to parse manifest.');
-      this.manifest_ = {};
-    }
-  };
-
-  /**
    * Requests wallpaper manifest file from server.
    */
   WallpaperManager.prototype.fetchManifest_ = function() {
-    var xhr = new XMLHttpRequest();
     var locale = navigator.language;
     var urls = [
         ManifestBaseURL + locale + '.json',
         // Fallback url. Use 'en' locale by default.
         ManifestBaseURL + 'en.json'];
 
-    for (var i = 0; i < urls.length; i++) {
-      xhr.open('GET', urls[i], false);
+    var asyncFetchManifestFromUrls = function(urls, func, successCallback,
+                                              failureCallback) {
+      var index = 0;
+      var loop = {
+        next: function() {
+          if (index < urls.length) {
+            func(loop, urls[index]);
+            index++;
+          } else {
+            failure();
+          }
+        },
+
+        success: function(response) {
+          successCallback(response);
+        },
+
+        failure: function() {
+          failureCallback();
+        }
+      };
+      loop.next();
+    };
+
+    var fetchManifestAsync = function(loop, url) {
+      var xhr = new XMLHttpRequest();
+      xhr.open('GET', url, true);
       try {
         xhr.send(null);
-        // TODO(bshe): We should save the downloaded manifest to local disk.
-        // Other components may want to use it (i.e. screen saver).
-        if (xhr.status === 200) {
-          this.parseManifest_(xhr.responseText);
-          return;
-        }
+        xhr.addEventListener('load', function(e) {

[flackr, 2012/11/13 22:22:31]

Reading the XHR documentation it seems like 'load' is only on successful load.
Should you perhaps handle loadend or ready state change and state == this.DONE
instead? See http://www.w3.org/TR/XMLHttpRequest/#introduction. I think this
will only catch successful connections to the server that didn't return a 200
status code.

[bshe, 2012/11/15 00:17:57]

Done. I used loadend event.

On 2012/11/13 22:22:31, flackr wrote:

+          // TODO(bshe): We should save the downloaded manifest to local disk.
+          // Other components may want to use it (e.g. screen saver).
+          if (xhr.status === 200) {

[flackr, 2012/11/13 22:22:31]

nit: braces not needed anymore on this if since they are both single line
blocks.

[bshe, 2012/11/15 00:17:57]

Done.

+            loop.success(xhr.responseText);
+          } else {
+            loop.next();
+          }
+        });
       } catch (e) {
-        this.manifest_ = {};
-        this.butterBar_.showError_(str('connectionFailed'));
-        return;
+        loop.failure();
       }
+    };
+
+    asyncFetchManifestFromUrls(urls, fetchManifestAsync,
+                               this.onLoadManifestSuccess_.bind(this),
+                               this.onLoadManifestFailed_.bind(this));
+  };
+
+  /**
+   * Parses a string as manifest(JSON). Sets manifest to an empty object if a
+   * parsing exception is catched. Called after manifest is successfully loaded.
+   * @param {string} response The string to parse as JSON.
+   */
+  WallpaperManager.prototype.onLoadManifestSuccess_ = function(response) {
+    try {
+      this.manifest_ = JSON.parse(response);
+    } catch (e) {
+      this.butterBar_.showError_('Failed to parse manifest.');

[flackr, 2012/11/13 22:22:31]

This isn't a localized string, and the message is somewhat confusing. You could
fall back to the generic connection failed message since the only way you could
get into this state is if you loaded bad data which probably means your
connection is bad. Probably simplest / best to move the JSON parsing back to
fetchManifestAsync and have a JSON parsing error call onLoadManifestFailed.

[bshe, 2012/11/15 00:17:57]

Done.

On 2012/11/13 22:22:31, flackr wrote:

+      this.manifest_ = {};
     }
-    this.manifest_ = {};
-    this.butterBar_.showError_(str('connectionFailed'));
+    this.initDom_();
+  };
 
+  // Sets manifest to an empty object and shows connection error. Called after
+  // manifest failed to load.
+  WallpaperManager.prototype.onLoadManifestFailed_ = function() {
     // TODO(bshe): Fall back to saved manifest if there is a problem fetching
     // manifest from server.
+    this.manifest_ = {};
+    this.butterBar_.showError_(str('connectionFailed'));
+    this.initDom_();
   };
 
   /**