Bug 575767: Stop Feedback from collection data until the user has been notified. r=dtownsend GECKO20b1_20100628_RELBRANCH
authorJono S Xia <jdicarlo@mozilla.com>
Wed, 30 Jun 2010 10:54:21 -0700
branchGECKO20b1_20100628_RELBRANCH
changeset 46447 65c30e4ee631158408970e91fd14b3924ab18497
parent 46419 69b71ac327814ff96ad8ddafbd64edef54bbc281
child 46451 133daf2cf5fa2a23c101a2ee07de86262c883ca6
push id14193
push userdtownsend@mozilla.com
push dateWed, 30 Jun 2010 17:56:08 +0000
treeherdermozilla-central@accdced3e5b8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdtownsend
bugs575767
milestone2.0b1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 575767: Stop Feedback from collection data until the user has been notified. r=dtownsend
browser/app/profile/extensions/testpilot@labs.mozilla.com/modules/setup.js
browser/app/profile/extensions/testpilot@labs.mozilla.com/modules/tasks.js
--- a/browser/app/profile/extensions/testpilot@labs.mozilla.com/modules/setup.js
+++ b/browser/app/profile/extensions/testpilot@labs.mozilla.com/modules/setup.js
@@ -57,17 +57,17 @@ const POPUP_REMINDER_INTERVAL = "extensi
 const ALWAYS_SUBMIT_DATA = "extensions.testpilot.alwaysSubmitData";
 const LOG_FILE_NAME = "TestPilotErrorLog.log";
 
 let TestPilotSetup = {
   didReminderAfterStartup: false,
   startupComplete: false,
   _shortTimer: null,
   _longTimer: null,
-  _remoteExperimentLoader: null,
+  _remoteExperimentLoader: null, // TODO make this a lazy initializer too?
   taskList: [],
   version: "",
 
   // Lazy initializers:
   __application: null,
   get _application() {
     if (this.__application == null) {
       this.__application = Cc["@mozilla.org/fuel/application;1"]
@@ -367,20 +367,26 @@ let TestPilotSetup = {
     // exists.  No excuse to ever be running two copies of the same task.
     this.taskList.push(testPilotTask);
   },
 
   _showNotification: function TPS__showNotification(task, fragile, text, title,
                                                     iconClass, showSubmit,
 						    showAlwaysSubmitCheckbox,
                                                     linkText, linkUrl,
-						    isExtensionUpdate) {
+						    isExtensionUpdate,
+                                                    onCloseCallback) {
+    /* TODO: Refactor the arguments of this function, it's getting really
+     * unweildly.... maybe pass in an object, or even make a notification an
+     * object that you create and then call .show() on. */
+
     // If there are multiple windows, show notifications in the frontmost
     // window.
-    let doc = this._getFrontBrowserWindow().document;
+    let window = this._getFrontBrowserWindow();
+    let doc = window.document;
     let popup = doc.getElementById("pilot-notification-popup");
 
     let anchor;
     if (this._isFfx4BetaVersion()) {
       /* If we're in the Ffx4Beta version, popups come down from feedback
        * button, but if we're in the standalone extension version, they
        * come up from status bar icon. */
       anchor = doc.getElementById("feedback-menu-button");
@@ -417,24 +423,24 @@ let TestPilotSetup = {
     alwaysSubmitCheckbox.setAttribute("hidden", !showAlwaysSubmitCheckbox);
     if (showSubmit) {
       if (isExtensionUpdate) {
         submitBtn.setAttribute("label",
 	  this._stringBundle.GetStringFromName(
 	    "testpilot.notification.update"));
 	submitBtn.onclick = function() {
           this._extensionUpdater.check(EXTENSION_ID);
-          self._hideNotification();
+          self._hideNotification(window, onCloseCallback);
 	};
       } else {
         submitBtn.setAttribute("label",
 	  this._stringBundle.GetStringFromName("testpilot.submit"));
         // Functionality for submit button:
         submitBtn.onclick = function() {
-          self._hideNotification();
+          self._hideNotification(window, onCloseCallback);
           if (showAlwaysSubmitCheckbox && alwaysSubmitCheckbox.checked) {
             self._prefs.setValue(ALWAYS_SUBMIT_DATA, true);
           }
           task.upload( function(success) {
             if (success) {
               self._showNotification(
 		task, true,
                 self._stringBundle.GetStringFromName(
@@ -459,46 +465,52 @@ let TestPilotSetup = {
       link.setAttribute("class", "notification-link");
       link.onclick = function(event) {
         if (event.button == 0) {
 	  if (task) {
             task.loadPage();
 	  } else {
             self._openChromeless(linkUrl);
 	  }
-          self._hideNotification();
+          self._hideNotification(window, onCloseCallback);
         }
       };
       link.setAttribute("hidden", false);
     } else {
       link.setAttribute("hidden", true);
     }
 
     closeBtn.onclick = function() {
-      self._hideNotification();
+      self._hideNotification(window, onCloseCallback);
     };
 
     // Show the popup:
     popup.hidden = false;
     popup.setAttribute("open", "true");
     popup.openPopup( anchor, "after_end");
   },
 
   _openChromeless: function TPS__openChromeless(url) {
     let window = this._getFrontBrowserWindow();
     window.TestPilotWindowUtils.openChromeless(url);
   },
 
-  _hideNotification: function TPS__hideNotification() {
-    let window = this._getFrontBrowserWindow();
+  _hideNotification: function TPS__hideNotification(window, onCloseCallback) {
+    /* Note - we take window as an argument instead of just using the frontmost
+     * window because the window order might have changed since the notification
+     * appeared and we want to be sure we close the notification in the same
+     * window as we opened it in! */
     let popup = window.document.getElementById("pilot-notification-popup");
     popup.hidden = true;
     popup.setAttribute("open", "false");
     popup.removeAttribute("tpisextensionupdate");
     popup.hidePopup();
+    if (onCloseCallback) {
+      onCloseCallback();
+    }
   },
 
   _isShowingUpdateNotification : function() {
     let window = this._getFrontBrowserWindow();
     let popup = window.document.getElementById("pilot-notification-popup");
 
     return popup.hasAttribute("tpisextensionupdate");
   },
@@ -538,36 +550,38 @@ let TestPilotSetup = {
       }
     }
 
     // If there's no finished test, next highest priority is new tests that
     // are starting...
     if (this._prefs.getValue(POPUP_SHOW_ON_NEW, false)) {
       for (i = 0; i < this.taskList.length; i++) {
         task = this.taskList[i];
-        if (task.status == TaskConstants.STATUS_STARTING ||
+        if (task.status == TaskConstants.STATUS_PENDING ||
             task.status == TaskConstants.STATUS_NEW) {
           if (task.taskType == TaskConstants.TYPE_EXPERIMENT) {
 	    this._showNotification(
-	      task, true,
+	      task, false,
 	      this._stringBundle.formatStringFromName(
 		"testpilot.notification.newTestPilotStudy.message",
 		[task.title], 1),
 	      this._stringBundle.GetStringFromName(
 		"testpilot.notification.newTestPilotStudy"),
 	      "new-study", false, false,
 	      this._stringBundle.GetStringFromName("testpilot.moreInfo"),
-	      task.defaultUrl);
-            // Having shown the notification, update task status so that this
-            // notification won't be shown again.
-            task.changeStatus(TaskConstants.STATUS_IN_PROGRESS, true);
+	      task.defaultUrl, false, function() {
+                /* on close callback (Bug 575767) -- when the "new study
+                 * starting" popup is dismissed, then the study can start. */
+                task.changeStatus(TaskConstants.STATUS_IN_PROGRESS, true);
+                TestPilotSetup.reloadRemoteExperiments();
+              });
             return;
           } else if (task.taskType == TaskConstants.TYPE_SURVEY) {
 	    this._showNotification(
-	      task, true,
+	      task, false,
 	      this._stringBundle.formatStringFromName(
 		"testpilot.notification.newTestPilotSurvey.message",
 		[task.title], 1),
               this._stringBundle.GetStringFromName(
 		"testpilot.notification.newTestPilotSurvey"),
 	      "new-study", false, false,
 	      this._stringBundle.GetStringFromName("testpilot.moreInfo"),
 	      task.defaultUrl);
--- a/browser/app/profile/extensions/testpilot@labs.mozilla.com/modules/tasks.js
+++ b/browser/app/profile/extensions/testpilot@labs.mozilla.com/modules/tasks.js
@@ -198,16 +198,17 @@ var TestPilotTask = {
 
   onWindowClosed: function TestPilotTask_onWindowClosed(window) {
   },
 
   onUrlLoad: function TestPilotTask_onUrlLoad(url) {
   },
 
   onDetailPageOpened: function TestPilotTask_onDetailPageOpened(){
+    // TODO fold this into loadPage()?
   },
 
   checkDate: function TestPilotTask_checkDate() {
   },
 
   changeStatus: function TPS_changeStatus(newStatus, suppressNotification) {
     // TODO we always suppress notifications except when new status is
     // "finished"; maybe remove that argument and only fire notification
@@ -439,24 +440,19 @@ TestPilotExperiment.prototype = {
       break;
     }
     if (!waitForData) {
       callback(content);
     }
   },
 
   experimentIsRunning: function TestPilotExperiment_isRunning() {
-    if (this._optInRequired) {
-      return (this._status == TaskConstants.STATUS_STARTING ||
-              this._status == TaskConstants.STATUS_IN_PROGRESS);
-    } else {
-      // Tests that don't require extra opt-in should start running even
-      // if you haven't seen them yet.
-      return (this._status < TaskConstants.STATUS_FINISHED);
-    }
+    // bug 575767
+    return (this._status == TaskConstants.STATUS_STARTING ||
+            this._status == TaskConstants.STATUS_IN_PROGRESS);
   },
 
   // Pass events along to handlers:
   onNewWindow: function TestPilotExperiment_onNewWindow(window) {
     this._logger.trace("Experiment.onNewWindow called.");
     if (this.experimentIsRunning()) {
       this._handlers.onNewWindow(window);
     }
@@ -594,21 +590,33 @@ TestPilotExperiment.prototype = {
         numTimesRun++;
         this._logger.trace("Test recurring... incrementing " + RECUR_TIMES_PREF_PREFIX + this._id + " to " + numTimesRun);
         Application.prefs.setValue( RECUR_TIMES_PREF_PREFIX + this._id,
                                     numTimesRun );
         this._logger.trace("Incremented it.");
       }
     }
 
-    // No-opt-in required tests skip PENDING and go straight to STARTING.
+    // If the notify-on-new-study pref is turned off, and the test doesn't
+    // require opt-in, then it can jump straight ahead to STARTING.
     if (!this._optInRequired &&
-        this._status < TaskConstants.STATUS_STARTING &&
+        !Application.prefs.getValue("extensions.testpilot.popup.showOnNewStudy",
+                                    false) &&
+        (this._status == TaskConstants.STATUS_NEW ||
+         this._status == TaskConstants.STATUS_PENDING)) {
+      this._logger.info("Skipping pending and going straight to starting.");
+      this.changeStatus(TaskConstants.STATUS_STARTING, true);
+    }
+
+    // If a study is STARTING, and we're in the right date range,
+    // then start it, and move it to IN_PROGRESS.
+    if ( this._status == TaskConstants.STATUS_STARTING &&
         currentDate >= this._startDate &&
         currentDate <= this._endDate) {
+      this._logger.info("Study now starting.");
       let uuidGenerator =
         Cc["@mozilla.org/uuid-generator;1"].getService(Ci.nsIUUIDGenerator);
       let uuid = uuidGenerator.generateUUID().toString();
       // remove the brackets from the generated UUID
       if (uuid.indexOf("{") == 0) {
         uuid = uuid.substring(1, (uuid.length - 1));
       }
       // clear the data before starting.