Bug 1191336 - Part 2 - Don't trigger the datachoices infobar when checking if user is allowed to upload. r=gfritzsche
authorAlessio Placitelli <alessio.placitelli@gmail.com>
Fri, 07 Aug 2015 07:01:00 +0200
changeset 288657 f63aa8d0a399d02aed164a2b524416fcd60b4902
parent 288656 20565e58ac99bbfc05a23b1a05f9bab8b2d21087
child 288658 e1e678b33aa6ac01b524803faf9103f14bb4aaca
push id5067
push userraliiev@mozilla.com
push dateMon, 21 Sep 2015 14:04:52 +0000
treeherdermozilla-beta@14221ffe5b2f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgfritzsche
bugs1191336
milestone42.0a1
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 1191336 - Part 2 - Don't trigger the datachoices infobar when checking if user is allowed to upload. r=gfritzsche
browser/base/content/test/general/browser_datachoices_notification.js
toolkit/components/telemetry/TelemetryReportingPolicy.jsm
--- a/browser/base/content/test/general/browser_datachoices_notification.js
+++ b/browser/base/content/test/general/browser_datachoices_notification.js
@@ -19,16 +19,29 @@ const PREF_BRANCH = "datareporting.polic
 const PREF_DRS_ENABLED = "datareporting.healthreport.service.enabled";
 const PREF_BYPASS_NOTIFICATION = PREF_BRANCH + "dataSubmissionPolicyBypassNotification";
 const PREF_CURRENT_POLICY_VERSION = PREF_BRANCH + "currentPolicyVersion";
 const PREF_ACCEPTED_POLICY_VERSION = PREF_BRANCH + "dataSubmissionPolicyAcceptedVersion";
 const PREF_ACCEPTED_POLICY_DATE = PREF_BRANCH + "dataSubmissionPolicyNotifiedTime";
 
 const TEST_POLICY_VERSION = 37;
 
+function fakeShowPolicyTimeout(set, clear) {
+  let reportingPolicy =
+    Cu.import("resource://gre/modules/TelemetryReportingPolicy.jsm", {}).Policy;
+  reportingPolicy.setShowInfobarTimeout = set;
+  reportingPolicy.clearShowInfobarTimeout = clear;
+}
+
+function sendSessionRestoredNotification() {
+  let reportingPolicyImpl =
+    Cu.import("resource://gre/modules/TelemetryReportingPolicy.jsm", {}).TelemetryReportingPolicyImpl;
+  reportingPolicyImpl.observe(null, "sessionstore-windows-restored", null);
+}
+
 /**
  * Wait for a tick.
  */
 function promiseNextTick() {
   return new Promise(resolve => executeSoon(resolve));
 }
 
 /**
@@ -51,16 +64,31 @@ function promiseWaitForAlertActive(aNoti
  * @return {Promise} Resolved when the notification is closed.
  */
 function promiseWaitForNotificationClose(aNotification) {
   let deferred = PromiseUtils.defer();
   waitForNotificationClose(aNotification, deferred.resolve);
   return deferred.promise;
 }
 
+function triggerInfoBar(expectedTimeoutMs) {
+  let showInfobarCallback = null;
+  let timeoutMs = null;
+  fakeShowPolicyTimeout((callback, timeout) => {
+    showInfobarCallback = callback;
+    timeoutMs = timeout;
+  }, () => {});
+  sendSessionRestoredNotification();
+  Assert.ok(!!showInfobarCallback, "Must have a timer callback.");
+  if (expectedTimeoutMs !== undefined) {
+    Assert.equal(timeoutMs, expectedTimeoutMs, "Timeout should match");
+  }
+  showInfobarCallback();
+}
+
 let checkInfobarButton = Task.async(function* (aNotification) {
   // Check that the button on the data choices infobar does the right thing.
   let buttons = aNotification.getElementsByTagName("button");
   Assert.equal(buttons.length, 1, "There is 1 button in the data reporting notification.");
   let button = buttons[0];
 
   // Add an observer to ensure the "advanced" pane opened (but don't bother
   // closing it - we close the entire window when done.)
@@ -125,21 +153,21 @@ add_task(function* test_single_window(){
   Assert.equal(Preferences.get(PREF_ACCEPTED_POLICY_VERSION, 0), 0,
                "No version should be set on init.");
   Assert.equal(Preferences.get(PREF_ACCEPTED_POLICY_DATE, 0), 0,
                "No date should be set on init.");
   Assert.ok(!TelemetryReportingPolicy.testIsUserNotified(),
             "User not notified about datareporting policy.");
 
   let alertShownPromise = promiseWaitForAlertActive(notificationBox);
-  // This should be false and trigger the Infobar.
   Assert.ok(!TelemetryReportingPolicy.canUpload(),
-            "User should not be allowed to upload and the infobar should be triggered.");
+            "User should not be allowed to upload.");
 
   // Wait for the infobar to be displayed.
+  triggerInfoBar(10 * 1000);
   yield alertShownPromise;
 
   Assert.equal(notificationBox.allNotifications.length, 1, "Notification Displayed.");
   Assert.ok(TelemetryReportingPolicy.canUpload(), "User should be allowed to upload now.");
 
   yield promiseNextTick();
   let promiseClosed = promiseWaitForNotificationClose(notificationBox.currentNotification);
   yield checkInfobarButton(notificationBox.currentNotification);
@@ -180,20 +208,21 @@ add_task(function* test_multiple_windows
   Assert.equal(Preferences.get(PREF_ACCEPTED_POLICY_DATE, 0), 0, "No date should be set on init.");
   Assert.ok(!TelemetryReportingPolicy.testIsUserNotified(), "User not notified about datareporting policy.");
 
   let showAlertPromises = [
     promiseWaitForAlertActive(notificationBoxes[0]),
     promiseWaitForAlertActive(notificationBoxes[1])
   ];
 
-  // This should be false and trigger the Infobar.
   Assert.ok(!TelemetryReportingPolicy.canUpload(),
-            "User should not be allowed to upload and the infobar should be triggered.");
+            "User should not be allowed to upload.");
 
+  // Wait for the infobars.
+  triggerInfoBar(10 * 1000);
   yield Promise.all(showAlertPromises);
 
   // Both notification were displayed. Close one and check that both gets closed.
   let closeAlertPromises = [
     promiseWaitForNotificationClose(notificationBoxes[0].currentNotification),
     promiseWaitForNotificationClose(notificationBoxes[1].currentNotification)
   ];
   notificationBoxes[0].currentNotification.close();
--- a/toolkit/components/telemetry/TelemetryReportingPolicy.jsm
+++ b/toolkit/components/telemetry/TelemetryReportingPolicy.jsm
@@ -332,57 +332,57 @@ let TelemetryReportingPolicyImpl = {
    */
   canUpload: function() {
     // If data submission is disabled, there's no point in showing the infobar. Just
     // forbid to upload.
     if (!this.dataSubmissionEnabled) {
       return false;
     }
 
-    // Make sure the user is notified of the current policy. If he isn't, don't try
-    // to upload anything.
-    if (!this._ensureUserNotified()) {
-      return false;
-    }
-
-    // Submission is enabled and user is notified: upload is allowed.
-    return true;
+    // Submission is enabled. We enable upload if user is notified or we need to bypass
+    // the policy.
+    const bypassNotification = Preferences.get(PREF_BYPASS_NOTIFICATION, false);
+    return this.isUserNotifiedOfCurrentPolicy || bypassNotification;
   },
 
   /**
    * Migrate the data policy preferences, if needed.
    */
   _migratePreferences: function() {
     // Current prefs are mostly the same than the old ones, except for some deprecated ones.
     for (let pref of DEPRECATED_FHR_PREFS) {
       Preferences.reset(pref);
     }
   },
 
   /**
-   * Make sure the user is notified about the policy before allowing upload.
-   * @return {Boolean} True if the user was notified, false otherwise.
+   * Show the data choices infobar if the user wasn't already notified and data submission
+   * is enabled.
    */
-  _ensureUserNotified: function() {
-    const BYPASS_NOTIFICATION = Preferences.get(PREF_BYPASS_NOTIFICATION, false);
-    if (this.isUserNotifiedOfCurrentPolicy || BYPASS_NOTIFICATION) {
-      return true;
+  _showInfobar: function() {
+    if (!this.dataSubmissionEnabled) {
+      this._log.trace("_showInfobar - Data submission disabled by the policy.");
+      return;
     }
 
-    this._log.trace("ensureUserNotified - User not notified, notifying now.");
-    if (this._notificationInProgress) {
-      this._log.trace("ensureUserNotified - User not notified, notification in progress.");
-      return false;
+    const bypassNotification = Preferences.get(PREF_BYPASS_NOTIFICATION, false);
+    if (this.isUserNotifiedOfCurrentPolicy || bypassNotification) {
+      this._log.trace("_showInfobar - User already notified or bypassing the policy.");
+      return;
     }
 
+    if (this._notificationInProgress) {
+      this._log.trace("_showInfobar - User not notified, notification already in progress.");
+      return;
+    }
+
+    this._log.trace("_showInfobar - User not notified, notifying now.");
     this._notificationInProgress = true;
     let request = new NotifyPolicyRequest(this._log);
     Observers.notify("datareporting:notify-data-policy:request", request);
-
-    return false;
   },
 
   /**
    * Called when the user is notified with the infobar.
    */
   _infobarShownCallback: function() {
     this._log.trace("_infobarShownCallback");
     this._recordNotificationData();
@@ -407,13 +407,13 @@ let TelemetryReportingPolicyImpl = {
     }
 
     const isFirstRun = Preferences.get(PREF_FIRST_RUN, true);
     const delay =
       isFirstRun ? NOTIFICATION_DELAY_FIRST_RUN_MSEC: NOTIFICATION_DELAY_NEXT_RUNS_MSEC;
 
     this._startupNotificationTimerId = Policy.setShowInfobarTimeout(
         // Calling |canUpload| eventually shows the infobar, if needed.
-        () => this.canUpload(), delay);
+        () => this._showInfobar(), delay);
     // We performed at least a run, flip the firstRun preference.
     Preferences.set(PREF_FIRST_RUN, false);
   },
 };