Bug 1239042 - Show sync status for a minimum of 1.6s. r?markh draft
authorKit Cambridge <kcambridge@mozilla.com>
Fri, 22 Apr 2016 07:19:16 -0700
changeset 356705 8808c86764e75f047a22901a37ecf4c5a1ddf3bf
parent 356103 2045bc8c9e90a7ca0b8c6447ddecd812a71b29e1
child 519457 0ebf732dd5bcd9bc6bc6f0191ef193579d779a3f
push id16573
push userkcambridge@mozilla.com
push dateTue, 26 Apr 2016 22:47:25 +0000
reviewersmarkh
bugs1239042
milestone49.0a1
Bug 1239042 - Show sync status for a minimum of 1.6s. r?markh MozReview-Commit-ID: FKpSe9r6Td9
browser/base/content/browser-syncui.js
--- a/browser/base/content/browser-syncui.js
+++ b/browser/base/content/browser-syncui.js
@@ -7,16 +7,18 @@ Cu.import("resource://gre/modules/XPCOMU
 if (AppConstants.MOZ_SERVICES_CLOUDSYNC) {
   XPCOMUtils.defineLazyModuleGetter(this, "CloudSync",
                                     "resource://gre/modules/CloudSync.jsm");
 }
 
 XPCOMUtils.defineLazyModuleGetter(this, "fxAccounts",
                                   "resource://gre/modules/FxAccounts.jsm");
 
+const MIN_STATUS_ANIMATION_DURATION = 1600;
+
 // gSyncUI handles updating the tools menu and displaying notifications.
 var gSyncUI = {
   _obs: ["weave:service:sync:start",
          "weave:service:sync:finish",
          "weave:service:sync:error",
          "weave:service:setup-complete",
          "weave:service:login:start",
          "weave:service:login:finish",
@@ -27,18 +29,20 @@ var gSyncUI = {
          "weave:ui:login:error",
          "weave:ui:sync:error",
          "weave:ui:sync:finish",
          "weave:ui:clear-error",
          "weave:engine:sync:finish"
   ],
 
   _unloaded: false,
-  // The number of "active" syncs - while this is non-zero, our button will spin
-  _numActiveSyncTasks: 0,
+  // The last sync start time. Used to calculate the leftover animation time
+  // once syncing completes (bug 1239042).
+  _syncStartTime: 0,
+  _syncAnimationTimer: 0,
 
   init: function () {
     Cu.import("resource://services-common/stringbundle.js");
 
     // Proceed to set up the UI if Sync has already started up.
     // Otherwise we'll do it when Sync is firing up.
     if (this.weaveService.ready) {
       this.initUI();
@@ -47,23 +51,25 @@ var gSyncUI = {
 
     // Sync isn't ready yet, but we can still update the UI with an initial
     // state - we haven't called initUI() yet, but that's OK - that's more
     // about observers for state changes, and will be called once Sync is
     // ready to start sending notifications.
     this.updateUI();
 
     Services.obs.addObserver(this, "weave:service:ready", true);
+    Services.obs.addObserver(this, "quit-application", true);
 
     // Remove the observer if the window is closed before the observer
     // was triggered.
     window.addEventListener("unload", function onUnload() {
       gSyncUI._unloaded = true;
       window.removeEventListener("unload", onUnload, false);
       Services.obs.removeObserver(gSyncUI, "weave:service:ready");
+      Services.obs.removeObserver(gSyncUI, "quit-application");
 
       if (Weave.Status.ready) {
         gSyncUI._obs.forEach(function(topic) {
           Services.obs.removeObserver(gSyncUI, topic);
         });
       }
     }, false);
   },
@@ -149,16 +155,19 @@ var gSyncUI = {
     this._promiseUpdateUI().catch(err => {
       this.log.error("updateUI failed", err);
     })
   },
 
   // Updates the UI - returns a promise.
   _promiseUpdateUI() {
     return this._needsSetup().then(needsSetup => {
+      if (!gBrowser)
+        return Promise.resolve();
+
       let loginFailed = this._loginFailed();
 
       // Start off with a clean slate
       document.getElementById("sync-reauth-state").hidden = true;
       document.getElementById("sync-setup-state").hidden = true;
       document.getElementById("sync-syncnow-state").hidden = true;
 
       if (CloudSync && CloudSync.ready && CloudSync().adapters.count) {
@@ -176,46 +185,54 @@ var gSyncUI = {
     });
   },
 
   // Functions called by observers
   onActivityStart() {
     if (!gBrowser)
       return;
 
-    this.log.debug("onActivityStart with numActive", this._numActiveSyncTasks);
-    if (++this._numActiveSyncTasks == 1) {
-      let broadcaster = document.getElementById("sync-status");
-      broadcaster.setAttribute("syncstatus", "active");
-      broadcaster.setAttribute("label", this._stringBundle.GetStringFromName("syncing2.label"));
-      broadcaster.setAttribute("disabled", "true");
-    }
+    this.log.debug("onActivityStart");
+
+    clearTimeout(this._syncAnimationTimer);
+    this._syncStartTime = Date.now();
+
+    let broadcaster = document.getElementById("sync-status");
+    broadcaster.setAttribute("syncstatus", "active");
+    broadcaster.setAttribute("label", this._stringBundle.GetStringFromName("syncing2.label"));
+    broadcaster.setAttribute("disabled", "true");
+
+    this.updateUI();
+  },
+
+  _updateSyncStatus() {
+    if (!gBrowser)
+      return;
+    let broadcaster = document.getElementById("sync-status");
+    broadcaster.removeAttribute("syncstatus");
+    broadcaster.removeAttribute("disabled");
+    broadcaster.setAttribute("label", this._stringBundle.GetStringFromName("syncnow.label"));
     this.updateUI();
   },
 
   onActivityStop() {
     if (!gBrowser)
       return;
-    this.log.debug("onActivityStop with numActive", this._numActiveSyncTasks);
-    if (--this._numActiveSyncTasks) {
-      if (this._numActiveSyncTasks < 0) {
-        // This isn't particularly useful (it seems more likely we'll set a
-        // "start" without a "stop" meaning it forever remains > 0) but it
-        // might offer some value...
-        this.log.error("mismatched onActivityStart/Stop calls",
-                       new Error("active=" + this._numActiveSyncTasks));
-      }
-      return; // active tasks are still ongoing...
+    this.log.debug("onActivityStop");
+
+    let now = Date.now();
+    let syncDuration = now - this._syncStartTime;
+
+    if (syncDuration < MIN_STATUS_ANIMATION_DURATION) {
+      let animationTime = MIN_STATUS_ANIMATION_DURATION - syncDuration;
+      clearTimeout(this._syncAnimationTimer);
+      this._syncAnimationTimer = setTimeout(() => this._updateSyncStatus(), animationTime);
+    } else {
+      this._updateSyncStatus();
     }
-
-    let broadcaster = document.getElementById("sync-status");
-    broadcaster.removeAttribute("syncstatus");
-    broadcaster.removeAttribute("disabled");
-    broadcaster.setAttribute("label", this._stringBundle.GetStringFromName("syncnow.label"));
-    this.updateUI();
   },
 
   onLoginError: function SUI_onLoginError() {
     this.log.debug("onLoginError: login=${login}, sync=${sync}", Weave.Status);
 
     // We don't show any login errors here; browser-fxaccounts shows them in
     // the hamburger menu.
     this.updateUI();
@@ -483,16 +500,21 @@ var gSyncUI = {
         this.initNotifications();
         break;
       case "weave:engine:sync:finish":
         if (data != "clients") {
           return;
         }
         this.onClientsSynced();
         break;
+      case "quit-application":
+        // Stop the animation timer on shutdown, since we can't update the UI
+        // after this.
+        clearTimeout(this._syncAnimationTimer);
+        break;
     }
   },
 
   QueryInterface: XPCOMUtils.generateQI([
     Ci.nsIObserver,
     Ci.nsISupportsWeakReference
   ])
 };