Move wallpaper progress bar on top of thumbnails (completely remove butter bar)

chrome/browser/resources/chromeos/wallpaper_manager/js/progress_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.
+
+/**
+ * Monitor the downloading progress of a XMLHttpRequest |xhr_| and shows the
+ * progress on |progressBar_|.
+ * @constructor
+ */
+function ProgressManager() {
+  this.xhr_ = null;
+  this.progressBar_ = cr.doc.createElement('div');

[flackr, 2013/02/05 01:24:20]

Could we avoid constructing the progress bar, perhaps template the construction?

[bshe, 2013/02/05 17:33:10]

Created a template in html.

I was trying to make ProgressManager self contained. So I constructed the
progressBar object in this constructor.

On 2013/02/05 01:24:20, flackr wrote:

+  this.progressBar_.classList.add('progress-bar');
+  var progressTrack = cr.doc.createElement('div');
+  progressTrack.classList.add('progress-track');
+  this.progressBar_.appendChild(progressTrack);
+  this.selectedGridItem_ = null;
+}
+
+/**
+ * Sets the XMLHttpRequest |xhr| to monitor and the wallpaper thumbnail grid

[flackr, 2013/02/05 01:24:20]

s/monitor/monitor,

[bshe, 2013/02/05 17:33:10]

Done.

+ * item |selectedGridItem| to show progress bar. Cancels previous xhr and

[flackr, 2013/02/05 01:24:20]

s/progress bar/a progress bar for

[bshe, 2013/02/05 17:33:10]

Done.

+ * hides/removes previous progress bar if any.
+ * Note: this must be called before xhr.send() function. Otherwise, we wont get
+ * loadstart event.
+ * @param {XMLHttpRequest} xhr The XMLHttpRequest.
+ * @param {WallpaperThumbnailsGridItem} selectedGridItem The wallpaper thumbnail
+ *     grid item. It extends from cr.ui.ListItem.
+ */
+ProgressManager.prototype.reset = function(xhr, selectedGridItem) {
+  if (this.xhr_)
+    this.xhr_.abort();

[flackr, 2013/02/05 01:24:20]

this seems potentially dangerous if it runs. Will calling this.xhr_.abort issue
an 'abort' event? If the event callback is asynchronous it could unset this.xhr_
after the new xhr has been registered.

[bshe, 2013/02/05 17:33:10]

the abort will issue an 'abort' event. And the event handler seems synchronous.
I also stepped into the abort function using dev tool. And it seems it calls
onDownloadAbort_ before return. So it is probably OK here?

On 2013/02/05 01:24:20, flackr wrote:

[flackr, 2013/02/06 15:30:46]

Sounds good, just wanted to be sure.

On 2013/02/05 17:33:10, bshe wrote:

+  this.hide(this.selectedGridItem_);
+  this.xhr_ = xhr;
+  this.selectedGridItem_ = selectedGridItem;
+  this.xhr_.addEventListener('loadstart',
+                             this.onDownloadStart_.bind(this));
+  this.xhr_.addEventListener('progress',
+                             this.onDownloadProgress_.bind(this));
+  this.xhr_.addEventListener('abort',
+                             this.onDownloadAbort_.bind(this));
+  this.xhr_.addEventListener('error',
+                             this.onDownloadError_.bind(this));
+  this.xhr_.addEventListener('load',
+                             this.onDownloadComplete_.bind(this));
+};
+
+/**
+ * Removes the progress bar in |selectedGridItem| if any. May called

[flackr, 2013/02/05 01:24:20]

s/May/May be

[bshe, 2013/02/05 17:33:10]

Done.

[flackr, 2013/02/06 15:30:46]

Sorry, the space was intentional, s/Maybe/May be

On 2013/02/05 17:33:10, bshe wrote:

[bshe, 2013/02/06 17:51:31]

oops. Sorry. Done.

On 2013/02/06 15:30:46, flackr wrote:

+ * asynchronously.
+ * @param {WallpaperThumbnailsGridItem} selectedGridItem The wallpaper thumbnail
+       grid item. It extends from cr.ui.ListItem.
+ */
+ProgressManager.prototype.hide = function(selectedGridItem) {

[flackr, 2013/02/05 01:24:20]

You're always hiding the selected grid item's progress bar. Remove the argument
and just hide this.selectedGridItem_. nit: call this hideProgressBar to be
explicit.

[bshe, 2013/02/05 17:33:10]

I have changed the function name. But this function is also called by
onFinished_ function in wallpaper_manager.js. And the onFinished_ is an
asynchronous callback function of setWallpaper. If we always hide
selectedGridItem_, we may hide the new one accidentally.

On 2013/02/05 01:24:20, flackr wrote:

+  if (selectedGridItem && selectedGridItem.querySelector('.progress-bar'))
+    selectedGridItem.removeChild(this.progressBar_);
+};
+
+/**
+ * Calculates and updates the width of progress track.
+ * @private
+ */
+ProgressManager.prototype.showProgress_ = function() {

[flackr, 2013/02/05 01:24:20]

How about setProgress_ = function(percentComplete) { since every time you call
showProgress you set this.percentComplete_ just before calling it? You don't
even need to store the progress just update the progress-track to this length.

[bshe, 2013/02/05 17:33:10]

Done.

+  this.progressBar_.querySelector('.progress-track').style.width =
+      (this.percentComplete_ * 100) + '%';
+};
+
+/**
+ * Shows a 0% progress bar to indicate downloading starts.
+ * @private
+ * @param {Event} e A loadstart ProgressEvent from XMLHttpRequest.
+ */
+ProgressManager.prototype.onDownloadStart_ = function(e) {
+  this.percentComplete_ = 0;
+  this.showProgress_();
+  this.selectedGridItem_.appendChild(this.progressBar_);
+};
+
+/**
+ * Hides progress bar when progression is terminated.
+ * @private
+ * @param {Event} e An abort ProgressEvent from XMLHttpRequest.
+ */
+ProgressManager.prototype.onDownloadAbort_ = function(e) {
+  this.xhr_ = null;
+  this.hide(this.selectedGridItem_);
+};
+
+/**
+ * Download completed successfully. Shows a 100% progress bar and clears |xhr_|.
+ * @private
+ * @param {Event} e A load ProgressEvent from XMLHttpRequest.
+ */
+ProgressManager.prototype.onDownloadComplete_ = function(e) {
+  this.progressBar_.querySelector('.progress-track').style.width = '100%';

[flackr, 2013/02/05 01:24:20]

Call this.setProgress_(1);

[bshe, 2013/02/05 17:33:10]

Done.

+  this.xhr_ = null;
+};
+
+/**
+ * Shows error message when progression failed.
+ * @private
+ * @param {Event} e An error ProgressEvent from XMLHttpRequest.
+ */
+ProgressManager.prototype.onDownloadError_ = function(e) {
+  // TODO(bshe): Add error message back once we decide how to show error
+  // message in the new UI. http://crbug.com/162563
+  this.xhr_ = null;
+  this.hide(this.selectedGridItem_);
+};
+
+/**
+ * Calculates downloading percentage and shows downloading progress.
+ * @private
+ * @param {Event} e A progress ProgressEvent from XMLHttpRequest.
+ */
+ProgressManager.prototype.onDownloadProgress_ = function(e) {
+  if (e.lengthComputable) {
+    this.percentComplete_ = e.loaded / e.total;
+    this.showProgress_();
+  }
+};