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

@@ -24,6 +24,19 @@ const PREF_ACCEPTED_POLICY_DATE = PREF_BRANCH + "dataSubmissionPolicyNotifiedTim
 
 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.
  */
@@ -56,6 +69,21 @@ function promiseWaitForNotificationClose(aNotification) {
   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");
@@ -130,11 +158,11 @@ add_task(function* test_single_window(){
             "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.");
@@ -185,10 +213,11 @@ add_task(function* test_multiple_windows(){
     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.

toolkit/components/telemetry/TelemetryReportingPolicy.jsm

@@ -337,14 +337,10 @@ let TelemetryReportingPolicyImpl = {
       return false;
     }
 
-    // Make sure the user is notified of the current policy. If he isn't, don't try
+    // Submission is enabled. We enable upload if user is notified or we need to bypass
-    // to upload anything.
+    // the policy.
-    if (!this._ensureUserNotified()) {
+    const bypassNotification = Preferences.get(PREF_BYPASS_NOTIFICATION, false);
-      return false;
+    return this.isUserNotifiedOfCurrentPolicy || bypassNotification;
-    }
-
-    // Submission is enabled and user is notified: upload is allowed.
-    return true;
   },
 
   /**
@@ -358,26 +354,30 @@ let TelemetryReportingPolicyImpl = {
   },
 
   /**
-   * Make sure the user is notified about the policy before allowing upload.
+   * Show the data choices infobar if the user wasn't already notified and data submission
-   * @return {Boolean} True if the user was notified, false otherwise.
+   * is enabled.
    */
-  _ensureUserNotified: function() {
+  _showInfobar: function() {
-    const BYPASS_NOTIFICATION = Preferences.get(PREF_BYPASS_NOTIFICATION, false);
+    if (!this.dataSubmissionEnabled) {
-    if (this.isUserNotifiedOfCurrentPolicy || BYPASS_NOTIFICATION) {
+      this._log.trace("_showInfobar - Data submission disabled by the policy.");
-      return true;
+      return;
+    }
+
+    const bypassNotification = Preferences.get(PREF_BYPASS_NOTIFICATION, false);
+    if (this.isUserNotifiedOfCurrentPolicy || bypassNotification) {
+      this._log.trace("_showInfobar - User already notified or bypassing 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.");
+      this._log.trace("_showInfobar - User not notified, notification already in progress.");
-      return false;
+      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;
   },
 
   /**
@@ -412,7 +412,7 @@ let TelemetryReportingPolicyImpl = {
 
     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);
   },