Bug 595455 - After several background update check failures notify the user that they should check if there is a new version available. r=dtownsend, a=blocking2.0-beta7

browser/app/profile/firefox.js

@@ -91,6 +91,11 @@ pref("app.update.timer", 600000);
 // Enables some extra Application Update Logging (can reduce performance)
 pref("app.update.log", false);
 
+// The number of general background check failures to allow before notifying the
+// user of the failure. User initiated update checks always notify the user of
+// the failure.
+pref("app.update.backgroundMaxErrors", 10);
+
 // When |app.update.cert.requireBuiltIn| is true or not specified the
 // final certificate and all certificates the connection is redirected to before
 // the final certificate for the url specified in the |app.update.url|

toolkit/mozapps/update/content/updates.js

@@ -50,6 +50,7 @@ const CoR = Components.results;
 
 const XMLNS_XUL               = "http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul";
 
+const PREF_APP_UPDATE_BACKGROUNDERRORS   = "app.update.backgroundErrors";
 const PREF_APP_UPDATE_BILLBOARD_TEST_URL = "app.update.billboard.test_url";
 const PREF_APP_UPDATE_CERT_ERRORS        = "app.update.cert.errors";
 const PREF_APP_UPDATE_ENABLED            = "app.update.enabled";
@@ -75,6 +76,7 @@ const SRCEVT_BACKGROUND       = 2;
 
 const CERT_ATTR_CHECK_FAILED_NO_UPDATE  = 100;
 const CERT_ATTR_CHECK_FAILED_HAS_UPDATE = 101;
+const BACKGROUNDCHECK_MULTIPLE_FAILURES = 110;
 
 var gLogEnabled = false;
 var gUpdatesFoundPageId;
@@ -404,7 +406,8 @@ var gUpdates = {
         // their permission to install, and it's ready for download.
         this.setUpdate(arg0);
         if (this.update.errorCode == CERT_ATTR_CHECK_FAILED_NO_UPDATE ||
-            this.update.errorCode == CERT_ATTR_CHECK_FAILED_HAS_UPDATE) {
+            this.update.errorCode == CERT_ATTR_CHECK_FAILED_HAS_UPDATE ||
+            this.update.errorCode == BACKGROUNDCHECK_MULTIPLE_FAILURES) {
           aCallback("errorextra");
           return;
         }
@@ -591,6 +594,11 @@ var gCheckingPage = {
     // notifications will never happen.
     Services.prefs.deleteBranch(PREF_APP_UPDATE_NEVER_BRANCH);
 
+    // The user will be notified if there is an error so clear the background
+    // check error count.
+    if (Services.prefs.prefHasUserValue(PREF_APP_UPDATE_BACKGROUNDERRORS))
+      Services.prefs.clearUserPref(PREF_APP_UPDATE_BACKGROUNDERRORS);
+
     this._checker = CoC["@mozilla.org/updates/update-checker;1"].
                     createInstance(CoI.nsIUpdateChecker);
     this._checker.checkForUpdates(this.updateListener, true);
@@ -666,7 +674,7 @@ var gCheckingPage = {
       gUpdates.setUpdate(update);
       if (update.errorCode &&
           (update.errorCode == CERT_ATTR_CHECK_FAILED_NO_UPDATE ||
-           update.errorCode == CERT_ATTR_CHECK_FAILED_HAS_UPDATE )) {
+           update.errorCode == CERT_ATTR_CHECK_FAILED_HAS_UPDATE)) {
         gUpdates.wiz.goTo("errorextra");
       }
       else {
@@ -1624,13 +1632,19 @@ var gErrorExtraPage = {
     if (Services.prefs.prefHasUserValue(PREF_APP_UPDATE_CERT_ERRORS))
       Services.prefs.clearUserPref(PREF_APP_UPDATE_CERT_ERRORS);
 
+    if (Services.prefs.prefHasUserValue(PREF_APP_UPDATE_BACKGROUNDERRORS))
+      Services.prefs.clearUserPref(PREF_APP_UPDATE_BACKGROUNDERRORS);
+
     if (gUpdates.update.errorCode == CERT_ATTR_CHECK_FAILED_HAS_UPDATE) {
       document.getElementById("errorCertAttrHasUpdateLabel").hidden = false;
     }
     else {
-      document.getElementById("errorCertCheckNoUpdateLabel").hidden = false;
+      if (gUpdates.update.errorCode == CERT_ATTR_CHECK_FAILED_NO_UPDATE)
+        document.getElementById("errorCertCheckNoUpdateLabel").hidden = false;
+      else
+        document.getElementById("genericBackgroundErrorLabel").hidden = false;
       var manualURL = Services.urlFormatter.formatURLPref(PREF_APP_UPDATE_MANUAL_URL);
-      var errorLinkLabel = document.getElementById("errorCertAttrLinkLabel");
+      var errorLinkLabel = document.getElementById("errorExtraLinkLabel");
       errorLinkLabel.value = manualURL;
       errorLinkLabel.setAttribute("url", manualURL);
       errorLinkLabel.hidden = false;

toolkit/mozapps/update/content/updates.xul

@@ -242,7 +242,7 @@
       <label id="genericBackgroundErrorLabel"
              hidden="true">&genericBackgroundError.label;</label>
       <hbox>
-        <label id="errorCertAttrLinkLabel" class="text-link" hidden="true"
+        <label id="errorExtraLinkLabel" class="text-link" hidden="true"
                value="" onclick="openUpdateURL(event);"/>
       </hbox>
     </vbox>

toolkit/mozapps/update/nsUpdateService.js

@@ -53,6 +53,8 @@ const Cr = Components.results;
 
 const PREF_APP_UPDATE_AUTO                = "app.update.auto";
 const PREF_APP_UPDATE_BACKGROUND_INTERVAL = "app.update.download.backgroundInterval";
+const PREF_APP_UPDATE_BACKGROUNDERRORS    = "app.update.backgroundErrors";
+const PREF_APP_UPDATE_BACKGROUNDMAXERRORS = "app.update.backgroundMaxErrors";
 const PREF_APP_UPDATE_CERTS_BRANCH        = "app.update.certs.";
 const PREF_APP_UPDATE_CERT_CHECKATTRS     = "app.update.cert.checkAttributes";
 const PREF_APP_UPDATE_CERT_ERRORS         = "app.update.cert.errors";
@@ -124,6 +126,7 @@ const ELEVATION_CANCELED = 9;
 
 const CERT_ATTR_CHECK_FAILED_NO_UPDATE  = 100;
 const CERT_ATTR_CHECK_FAILED_HAS_UPDATE = 101;
+const BACKGROUNDCHECK_MULTIPLE_FAILURES = 110;
 
 const DOWNLOAD_CHUNK_SIZE           = 300000; // bytes
 const DOWNLOAD_BACKGROUND_INTERVAL  = 600;    // seconds
@@ -1286,16 +1289,25 @@ UpdateService.prototype = {
         LOG("UpdateService:notify:listener - error during background update: " +
             update.statusText);
 
-        if (!update.errorCode ||
+        var maxErrors;
-            update.errorCode != CERT_ATTR_CHECK_FAILED_NO_UPDATE &&
+        var errCount;
-            update.errorCode != CERT_ATTR_CHECK_FAILED_HAS_UPDATE)
+        if (update.errorCode == CERT_ATTR_CHECK_FAILED_NO_UPDATE ||
-          return;
+            update.errorCode == CERT_ATTR_CHECK_FAILED_HAS_UPDATE) {
+          errCount = getPref("getIntPref", PREF_APP_UPDATE_CERT_ERRORS, 0);
+          errCount++;
+          Services.prefs.setIntPref(PREF_APP_UPDATE_CERT_ERRORS, errCount);
+          maxErrors = getPref("getIntPref", PREF_APP_UPDATE_CERT_MAXERRORS, 5);
+        }
+        else {
+          update.errorCode = BACKGROUNDCHECK_MULTIPLE_FAILURES;
+          errCount = getPref("getIntPref", PREF_APP_UPDATE_BACKGROUNDERRORS, 0);
+          errCount++;
+          Services.prefs.setIntPref(PREF_APP_UPDATE_BACKGROUNDERRORS, errCount);
+          maxErrors = getPref("getIntPref", PREF_APP_UPDATE_BACKGROUNDMAXERRORS,
+                              10);
+        }
 
-        var errCount = getPref("getIntPref", PREF_APP_UPDATE_CERT_ERRORS, 0);
+        if (errCount >= maxErrors) {
-        errCount++;
-        Services.prefs.setIntPref(PREF_APP_UPDATE_CERT_ERRORS, errCount);
-
-        if (errCount >= getPref("getIntPref", PREF_APP_UPDATE_CERT_MAXERRORS, 5)) {
           var prompter = Cc["@mozilla.org/updates/update-prompt;1"].
                          createInstance(Ci.nsIUpdatePrompt);
           prompter.showUpdateError(update);
@@ -2157,6 +2169,9 @@ Checker.prototype = {
       if (Services.prefs.prefHasUserValue(PREF_APP_UPDATE_CERT_ERRORS))
         Services.prefs.clearUserPref(PREF_APP_UPDATE_CERT_ERRORS);
 
+      if (Services.prefs.prefHasUserValue(PREF_APP_UPDATE_BACKGROUNDERRORS))
+        Services.prefs.clearUserPref(PREF_APP_UPDATE_BACKGROUNDERRORS);
+
       // Tell the Update Service about the updates
       this._callback.onCheckComplete(event.target, updates, updates.length);
     }
@@ -2834,7 +2849,8 @@ UpdatePrompt.prototype = {
     }
 
     if (update.errorCode == CERT_ATTR_CHECK_FAILED_NO_UPDATE ||
-        update.errorCode == CERT_ATTR_CHECK_FAILED_HAS_UPDATE) {
+        update.errorCode == CERT_ATTR_CHECK_FAILED_HAS_UPDATE ||
+        update.errorCode == BACKGROUNDCHECK_MULTIPLE_FAILURES) {
       this._showUIWhenIdle(null, URI_UPDATE_PROMPT_DIALOG, null,
                            UPDATE_WINDOW_NAME, null, update);
       return;

toolkit/mozapps/update/test/chrome/Makefile.in

@@ -100,6 +100,7 @@ _CHROME_FILES = \
   test_0132_check_invalidCertAttrs_hasUpdate.xul \
   test_0141_notify_invalidCertAttrs_noUpdate.xul \
   test_0142_notify_invalidCertAttrs_hasUpdate.xul \
+  test_0151_notify_backgroundCheckError.xul \
   test_9999_cleanup.xul \
   $(NULL)
 

toolkit/mozapps/update/test/chrome/test_0131_check_invalidCertAttrs_noUpdate.xul

@@ -25,8 +25,9 @@ const TESTS = [ {
   pageid: PAGEID_CHECKING
 }, {
   pageid: PAGEID_ERROR_EXTRA,
-  extraDelayedCheckFunction: checkCertErrorPage,
+  extraDelayedCheckFunction: checkErrorExtraPage,
   shouldBeHidden: false,
+  displayedTextElem: "errorCertCheckNoUpdateLabel",
   buttonClick: "finish"
 } ];
 

toolkit/mozapps/update/test/chrome/test_0132_check_invalidCertAttrs_hasUpdate.xul

@@ -25,8 +25,9 @@ const TESTS = [ {
   pageid: PAGEID_CHECKING
 }, {
   pageid: PAGEID_ERROR_EXTRA,
-  extraDelayedCheckFunction: checkCertErrorPage,
+  extraDelayedCheckFunction: checkErrorExtraPage,
   shouldBeHidden: true,
+  displayedTextElem: "errorCertAttrHasUpdateLabel",
   buttonClick: "finish"
 } ];
 

toolkit/mozapps/update/test/chrome/test_0141_notify_invalidCertAttrs_noUpdate.xul

@@ -23,8 +23,9 @@
 
 const TESTS = [ {
   pageid: PAGEID_ERROR_EXTRA,
-  extraDelayedCheckFunction: checkCertErrorPage,
+  extraDelayedCheckFunction: checkErrorExtraPage,
   shouldBeHidden: false,
+  displayedTextElem: "errorCertCheckNoUpdateLabel",
   buttonClick: "finish"
 } ];
 
@@ -39,7 +40,8 @@ function runTest() {
   gDefaultPrefBranch.setCharPref(PREF_APP_UPDATE_URL, url);
   debugDump("Default Update URL: " + url);
 
-  certErrorsPrefObserver.init();
+  errorsPrefObserver.init(PREF_APP_UPDATE_CERT_ERRORS,
+                          PREF_APP_UPDATE_CERT_MAXERRORS);
 
   gAUS.notify(null);
 }

toolkit/mozapps/update/test/chrome/test_0142_notify_invalidCertAttrs_hasUpdate.xul

@@ -23,8 +23,9 @@
 
 const TESTS = [ {
   pageid: PAGEID_ERROR_EXTRA,
-  extraDelayedCheckFunction: checkCertErrorPage,
+  extraDelayedCheckFunction: checkErrorExtraPage,
   shouldBeHidden: true,
+  displayedTextElem: "errorCertAttrHasUpdateLabel",
   buttonClick: "finish"
 } ];
 
@@ -40,7 +41,8 @@ function runTest() {
   gDefaultPrefBranch.setCharPref(PREF_APP_UPDATE_URL, url);
   debugDump("Default Update URL: " + url);
 
-  certErrorsPrefObserver.init();
+  errorsPrefObserver.init(PREF_APP_UPDATE_CERT_ERRORS,
+                          PREF_APP_UPDATE_CERT_MAXERRORS);
 
   gAUS.notify(null);
 }

toolkit/mozapps/update/test/chrome/test_0151_notify_backgroundCheckError.xul

@@ -0,0 +1,53 @@
+<?xml version="1.0"?>
+<!--
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/
+ */
+-->
+
+<?xml-stylesheet href="chrome://global/skin" type="text/css"?>
+<?xml-stylesheet href="chrome://mochikit/content/tests/SimpleTest/test.css" type="text/css"?>
+
+<window title="Test notification when multiple background check errors occur (bug 595455)"
+        xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
+        onload="runTestDefault();">
+<script type="application/javascript" 
+        src="chrome://mochikit/content/MochiKit/packed.js"/>
+<script type="application/javascript"
+        src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"/>
+<script type="application/javascript"
+        src="utils.js"/>
+
+<script type="application/javascript">
+<![CDATA[
+
+const TESTS = [ {
+  pageid: PAGEID_ERROR_EXTRA,
+  extraDelayedCheckFunction: checkErrorExtraPage,
+  shouldBeHidden: false,
+  displayedTextElem: "genericBackgroundErrorLabel",
+  buttonClick: "finish"
+} ];
+
+function runTest() {
+  debugDump("Entering runTest");
+
+  let url = URL_UPDATE + "?xmlMalformed=1";
+  setUpdateURLOverride(url);
+  debugDump("Update URL: " + url);
+
+  errorsPrefObserver.init(PREF_APP_UPDATE_BACKGROUNDERRORS,
+                          PREF_APP_UPDATE_BACKGROUNDMAXERRORS);
+
+  gAUS.notify(null);
+}
+
+]]>
+</script>
+
+<body xmlns="http://www.w3.org/1999/xhtml">
+  <p id="display"></p>
+  <div id="content" style="display: none"></div>
+  <pre id="test"></pre>
+</body>
+</window>

toolkit/mozapps/update/test/chrome/utils.js

@@ -609,9 +609,10 @@ function checkPrefHasUserValue(aPrefHasValue) {
 }
 
 /**
- * Checks whether the link is hidden (certificate attribute check error with an
+ * Checks whether the link is hidden (general background update check error or
- * update) or not (certificate attribute check error without an update) on the
+ * a certificate attribute check error with an update) or not (certificate
- * errorcertcheck page and that the app.update.cert.errors preference does note
+ * attribute check error without an update) on the errorextra page and that the
+ * app.update.cert.errors and app.update.backgroundErrors preferences do not
  & have a user value.
  *
  * @param  aShouldBeHidden (optional)
@@ -619,16 +620,23 @@ function checkPrefHasUserValue(aPrefHasValue) {
  *         aShouldBeHidden is undefined the value of the current test's
  *         shouldBeHidden property will be used.
  */
-function checkCertErrorPage(aShouldBeHidden) {
+function checkErrorExtraPage(aShouldBeHidden) {
   let shouldBeHidden = aShouldBeHidden === undefined ? gTest.shouldBeHidden
                                                      : aShouldBeHidden;
-  is(gWin.document.getElementById("errorCertAttrLinkLabel").hidden, shouldBeHidden,
+  is(gWin.document.getElementById("errorExtraLinkLabel").hidden, shouldBeHidden,
-     "Checking errorCertAttrLinkLabel hidden attribute equals " +
+     "Checking errorExtraLinkLabel hidden attribute equals " +
      (shouldBeHidden ? "true" : "false"));
 
+  is(gWin.document.getElementById(gTest.displayedTextElem).hidden, false,
+     "Checking " + gTest.displayedTextElem + " should not be hidden");
+
   ok(!Services.prefs.prefHasUserValue(PREF_APP_UPDATE_CERT_ERRORS),
      "Preference " + PREF_APP_UPDATE_CERT_ERRORS + " should not have a " +
      "user value");
+
+  ok(!Services.prefs.prefHasUserValue(PREF_APP_UPDATE_BACKGROUNDERRORS),
+     "Preference " + PREF_APP_UPDATE_BACKGROUNDERRORS + " should not have a " +
+     "user value");
 }
 
 /**
@@ -786,10 +794,22 @@ function resetPrefs() {
     Services.prefs.clearUserPref(PREF_APP_UPDATE_LOG);
   }
 
+  if (Services.prefs.prefHasUserValue(PREF_APP_UPDATE_CERT_ERRORS)) {
+    Services.prefs.clearUserPref(PREF_APP_UPDATE_CERT_ERRORS);
+  }
+
   if (Services.prefs.prefHasUserValue(PREF_APP_UPDATE_CERT_MAXERRORS)) {
     Services.prefs.clearUserPref(PREF_APP_UPDATE_CERT_MAXERRORS);
   }
 
+  if (Services.prefs.prefHasUserValue(PREF_APP_UPDATE_BACKGROUNDERRORS)) {
+    Services.prefs.clearUserPref(PREF_APP_UPDATE_BACKGROUNDERRORS);
+  }
+
+  if (Services.prefs.prefHasUserValue(PREF_APP_UPDATE_BACKGROUNDMAXERRORS)) {
+    Services.prefs.clearUserPref(PREF_APP_UPDATE_BACKGROUNDMAXERRORS);
+  }
+
   if (Services.prefs.prefHasUserValue(PREF_APP_UPDATE_CERT_INVALID_ATTR_NAME)) {
     Services.prefs.clearUserPref(PREF_APP_UPDATE_CERT_INVALID_ATTR_NAME);
   }
@@ -1105,35 +1125,45 @@ function getUpdateWindow() {
 }
 
 /**
- * Helper for certificate attribute check errors.
+ * Helper for background check errors.
  */
-var certErrorsPrefObserver = {
+var errorsPrefObserver = {
+  observedPref: null,
+  maxErrorPref: null,
+
   /**
-   * Sets up the app.update.cert.errors preference observer and sets the
+   * Sets up a preference observer and sets the associated maximum errors
-   * app.update.cert.maxErrors preference.
+   * preference used for background notification.
    *
-   * @param  aMaxErrors
+   * @param  aObservePref
+   *         The preference to observe.
+   * @param  aMaxErrorPref
+   *         The maximum errors preference.
+   * @param  aMaxErrorCount
    *         The value to set the app.update.cert.maxErrors preference to.
    */
-  init: function(aMaxErrors) {
+  init: function(aObservePref, aMaxErrorPref, aMaxErrorCount) {
-    let maxErrors = aMaxErrors ? aMaxErrors : 5;
+    this.observedPref = aObservePref;
-    Services.prefs.setIntPref(PREF_APP_UPDATE_CERT_MAXERRORS, maxErrors);
+    this.maxErrorPref = aMaxErrorPref;
-    Services.prefs.addObserver(PREF_APP_UPDATE_CERT_ERRORS, this, false);
+
+    let maxErrors = aMaxErrorCount ? aMaxErrorCount : 5;
+    Services.prefs.setIntPref(aMaxErrorPref, maxErrors);
+    Services.prefs.addObserver(aObservePref, this, false);
   },
 
   /**
    * Preference observer for the app.update.cert.errors preference.
    */
   observe: function XPI_observe(aSubject, aTopic, aData) {
-    if (aData == PREF_APP_UPDATE_CERT_ERRORS) {
+    if (aData == this.observedPref) {
-      let errCount = Services.prefs.getIntPref(PREF_APP_UPDATE_CERT_ERRORS);
+      let errCount = Services.prefs.getIntPref(this.observedPref);
-      let errMax = Services.prefs.getIntPref(PREF_APP_UPDATE_CERT_MAXERRORS);
+      let errMax = Services.prefs.getIntPref(this.maxErrorPref);
       if (errCount >= errMax) {
-        debugDump("prefObserver - removing pref observer");
+        debugDump("errorsPrefObserver - removing pref observer");
-        Services.prefs.removeObserver(PREF_APP_UPDATE_CERT_ERRORS, this);
+        Services.prefs.removeObserver(this.observedPref, this);
       }
       else {
-        debugDump("prefObserver - notifying AUS");
+        debugDump("errorsPrefObserver - notifying AUS");
         SimpleTest.executeSoon(function() {
           gAUS.notify(null);
         });

toolkit/mozapps/update/test/shared.js

@@ -44,6 +44,8 @@ const AUS_Ci = Components.interfaces;
 const AUS_Cr = Components.results;
 const AUS_Cu = Components.utils;
 
+const PREF_APP_UPDATE_BACKGROUNDERRORS    = "app.update.backgroundErrors";
+const PREF_APP_UPDATE_BACKGROUNDMAXERRORS = "app.update.backgroundMaxErrors";
 const PREF_APP_UPDATE_CERTS_BRANCH        = "app.update.certs.";
 const PREF_APP_UPDATE_CERT_CHECKATTRS     = "app.update.cert.checkAttributes";
 const PREF_APP_UPDATE_CERT_ERRORS         = "app.update.cert.errors";