Bug 1225648 - Remove duplicate skipDefaultBrowser code and fix double-counting error when tracking prompt counts. r=Gijs

browser/components/nsBrowserGlue.js

@@ -1276,6 +1276,14 @@ BrowserGlue.prototype = {
       let shouldCheck = false;
 #else
       let shouldCheck = ShellService.shouldCheckDefaultBrowser;
+#endif
+#ifndef RELEASE_BUILD
+      let promptCount =
+        Services.prefs.getIntPref("browser.shell.defaultBrowserCheckCount");
+      let skipDefaultBrowserCheck =
+        Services.prefs.getBoolPref("browser.shell.skipDefaultBrowserCheck");
+#else
+      let skipDefaultBrowserCheck = false;
 #endif
       let willRecoverSession = false;
       try {
@@ -1302,6 +1310,27 @@ BrowserGlue.prototype = {
 
       let willPrompt = shouldCheck && !isDefault && !willRecoverSession;
 
+      // Skip the "Set Default Browser" check during first-run or after the
+      // browser has been run a few times.
+      if (willPrompt) {
+        if (skipDefaultBrowserCheck) {
+          Services.prefs.setBoolPref("browser.shell.skipDefaultBrowserCheck", false);
+          willPrompt = false;
+        } else {
+          promptCount++;
+        }
+        if (promptCount > 3) {
+          willPrompt = false;
+        }
+      }
+
+#ifndef RELEASE_BUILD
+      if (willPrompt) {
+        Services.prefs.setIntPref("browser.shell.defaultBrowserCheckCount",
+                                  promptCount);
+      }
+#endif
+
       try {
         // Report default browser status on startup to telemetry
         // so we can track whether we are the default.
@@ -1311,15 +1340,8 @@ BrowserGlue.prototype = {
                           .add(isDefaultError);
         Services.telemetry.getHistogramById("BROWSER_SET_DEFAULT_ALWAYS_CHECK")
                           .add(shouldCheck);
-        let promptCount =
-          Services.prefs.getIntPref("browser.shell.defaultBrowserCheckCount");
-        if (willPrompt) {
-          promptCount++;
-        }
         Services.telemetry.getHistogramById("BROWSER_SET_DEFAULT_DIALOG_PROMPT_RAWCOUNT")
                           .add(promptCount);
-        Services.prefs.setIntPref("browser.shell.defaultBrowserCheckCount",
-                                  promptCount)
       }
       catch (ex) { /* Don't break the default prompt if telemetry is broken. */ }
 

browser/components/shell/nsGNOMEShellService.cpp

@@ -325,47 +325,6 @@ nsGNOMEShellService::SetDefaultBrowser(bool aClaimAllTypes,
   return NS_OK;
 }
 
-NS_IMETHODIMP
-nsGNOMEShellService::GetShouldSkipCheckDefaultBrowser(bool* aResult)
-{
-  NS_ENSURE_ARG_POINTER(aResult);
-
-  nsresult rv;
-  nsCOMPtr<nsIPrefBranch> prefs(do_GetService(NS_PREFSERVICE_CONTRACTID, &rv));
-  if (NS_FAILED(rv)) {
-    return rv;
-  }
-
-  rv = prefs->GetBoolPref(PREF_SKIPDEFAULTBROWSERCHECK, aResult);
-  if (NS_FAILED(rv)) {
-    return rv;
-  }
-  if (*aResult) {
-    // Only skip the default browser check once. The next attempt in
-    // a new session should proceed.
-    return prefs->SetBoolPref(PREF_SKIPDEFAULTBROWSERCHECK, false);
-  }
-
-  int32_t defaultBrowserCheckCount;
-  rv = prefs->GetIntPref(PREF_DEFAULTBROWSERCHECKCOUNT,
-                         &defaultBrowserCheckCount);
-  if (NS_FAILED(rv)) {
-    return rv;
-  }
-  if (defaultBrowserCheckCount < 4) {
-    *aResult = false;
-    return prefs->SetIntPref(PREF_DEFAULTBROWSERCHECKCOUNT,
-                             defaultBrowserCheckCount + 1);
-  }
-
-  // Disable the default browser check after three attempts.
-  // Don't modify PREF_CHECKDEFAULTBROWSER since that is a
-  // user-initiated action and it shouldn't get re-enabled
-  // if it has been user disabled.
-  *aResult = true;
-  return rv;
-}
-
 NS_IMETHODIMP
 nsGNOMEShellService::GetShouldCheckDefaultBrowser(bool* aResult)
 {
@@ -377,18 +336,6 @@ nsGNOMEShellService::GetShouldCheckDefaultBrowser(bool* aResult)
   }
 
   nsresult rv;
-#ifndef RELEASE_BUILD
-  bool skipDefaultBrowserCheck;
-  rv = GetShouldSkipCheckDefaultBrowser(&skipDefaultBrowserCheck);
-  if (NS_FAILED(rv)) {
-    return rv;
-  }
-  if (skipDefaultBrowserCheck) {
-    *aResult = false;
-    return rv;
-  }
-#endif
-
   nsCOMPtr<nsIPrefBranch> prefs(do_GetService(NS_PREFSERVICE_CONTRACTID, &rv));
   if (NS_FAILED(rv)) {
     return rv;

browser/components/shell/nsIMacShellService.idl

@@ -5,7 +5,7 @@
 
 #include "nsIShellService.idl"
 
-[scriptable, uuid(7f8ca08e-1df4-4735-86e9-50dedb48e5e8)]
+[scriptable, uuid(291a27cd-ef4c-46c6-a2f8-83182498167e)]
 interface nsIMacShellService : nsIShellService
 {
   const long APPLICATION_KEYCHAIN_ACCESS  = 2;

browser/components/shell/nsIShellService.idl

@@ -8,7 +8,7 @@
 interface nsIDOMElement;
 interface nsIFile;
 
-[scriptable, uuid(55cb78a8-2fc4-48f4-9345-ff0e541c5cc4)]
+[scriptable, uuid(53f4bc4a-5b86-4643-8e67-4907ecbab34c)]
 interface nsIShellService : nsISupports
 {
   /**
@@ -46,13 +46,6 @@ interface nsIShellService : nsISupports
    */
   attribute boolean shouldCheckDefaultBrowser;
 
-  /**
-   * Used to determine whether or not the "Set Default Browser" check
-   * should be skipped during first-run or after the browser has been
-   * run a few times.
-   */
-  readonly attribute boolean shouldSkipCheckDefaultBrowser;
-
   /**
    * Used to determine whether or not to offer "Set as desktop background"
    * functionality. Even if shell service is available it is not

browser/components/shell/nsIWindowsShellService.idl

@@ -5,7 +5,7 @@
 
 #include "nsIShellService.idl"
 
-[scriptable, uuid(89b0a761-d9a0-4c39-ab83-d81566459a31)]
+[scriptable, uuid(13f20725-4fd5-431f-90a1-525ab31755b1)]
 interface nsIWindowsShellService : nsIShellService
 {
     /**

browser/components/shell/nsMacShellService.cpp

@@ -103,47 +103,6 @@ nsMacShellService::SetDefaultBrowser(bool aClaimAllTypes, bool aForAllUsers)
   return NS_OK;
 }
 
-NS_IMETHODIMP
-nsMacShellService::GetShouldSkipCheckDefaultBrowser(bool* aResult)
-{
-  NS_ENSURE_ARG_POINTER(aResult);
-
-  nsresult rv;
-  nsCOMPtr<nsIPrefBranch> prefs(do_GetService(NS_PREFSERVICE_CONTRACTID, &rv));
-  if (NS_FAILED(rv)) {
-    return rv;
-  }
-
-  rv = prefs->GetBoolPref(PREF_SKIPDEFAULTBROWSERCHECK, aResult);
-  if (NS_FAILED(rv)) {
-    return rv;
-  }
-  if (*aResult) {
-    // Only skip the default browser check once. The next attempt in
-    // a new session should proceed.
-    return prefs->SetBoolPref(PREF_SKIPDEFAULTBROWSERCHECK, false);
-  }
-
-  int32_t defaultBrowserCheckCount;
-  rv = prefs->GetIntPref(PREF_DEFAULTBROWSERCHECKCOUNT,
-                         &defaultBrowserCheckCount);
-  if (NS_FAILED(rv)) {
-    return rv;
-  }
-  if (defaultBrowserCheckCount < 4) {
-    *aResult = false;
-    return prefs->SetIntPref(PREF_DEFAULTBROWSERCHECKCOUNT,
-                             defaultBrowserCheckCount + 1);
-  }
-
-  // Disable the default browser check after three attempts.
-  // Don't modify PREF_CHECKDEFAULTBROWSER since that is a
-  // user-initiated action and it shouldn't get re-enabled
-  // if it has been user disabled.
-  *aResult = true;
-  return rv;
-}
-
 NS_IMETHODIMP
 nsMacShellService::GetShouldCheckDefaultBrowser(bool* aResult)
 {
@@ -155,18 +114,6 @@ nsMacShellService::GetShouldCheckDefaultBrowser(bool* aResult)
   }
 
   nsresult rv;
-#ifndef RELEASE_BUILD
-  bool skipDefaultBrowserCheck;
-  rv = GetShouldSkipCheckDefaultBrowser(&skipDefaultBrowserCheck);
-  if (NS_FAILED(rv)) {
-    return rv;
-  }
-  if (skipDefaultBrowserCheck) {
-    *aResult = false;
-    return rv;
-  }
-#endif
-
   nsCOMPtr<nsIPrefBranch> prefs(do_GetService(NS_PREFSERVICE_CONTRACTID, &rv));
   if (NS_FAILED(rv)) {
     return rv;

browser/components/shell/nsWindowsShellService.cpp

@@ -984,47 +984,6 @@ nsWindowsShellService::SetDefaultBrowser(bool aClaimAllTypes, bool aForAllUsers)
   return rv;
 }
 
-NS_IMETHODIMP
-nsWindowsShellService::GetShouldSkipCheckDefaultBrowser(bool* aResult)
-{
-  NS_ENSURE_ARG_POINTER(aResult);
-
-  nsresult rv;
-  nsCOMPtr<nsIPrefBranch> prefs(do_GetService(NS_PREFSERVICE_CONTRACTID, &rv));
-  if (NS_FAILED(rv)) {
-    return rv;
-  }
-
-  rv = prefs->GetBoolPref(PREF_SKIPDEFAULTBROWSERCHECK, aResult);
-  if (NS_FAILED(rv)) {
-    return rv;
-  }
-  if (*aResult) {
-    // Only skip the default browser check once. The next attempt in
-    // a new session should proceed.
-    return prefs->SetBoolPref(PREF_SKIPDEFAULTBROWSERCHECK, false);
-  }
-
-  int32_t defaultBrowserCheckCount;
-  rv = prefs->GetIntPref(PREF_DEFAULTBROWSERCHECKCOUNT,
-                         &defaultBrowserCheckCount);
-  if (NS_FAILED(rv)) {
-    return rv;
-  }
-  if (defaultBrowserCheckCount < 4) {
-    *aResult = false;
-    return prefs->SetIntPref(PREF_DEFAULTBROWSERCHECKCOUNT,
-                             defaultBrowserCheckCount + 1);
-  }
-
-  // Disable the default browser check after three attempts.
-  // Don't modify PREF_CHECKDEFAULTBROWSER since that is a
-  // user-initiated action and it shouldn't get re-enabled
-  // if it has been user disabled.
-  *aResult = true;
-  return rv;
-}
-
 NS_IMETHODIMP
 nsWindowsShellService::GetShouldCheckDefaultBrowser(bool* aResult)
 {
@@ -1038,18 +997,6 @@ nsWindowsShellService::GetShouldCheckDefaultBrowser(bool* aResult)
   }
 
   nsresult rv;
-#ifndef RELEASE_BUILD
-  bool skipDefaultBrowserCheck;
-  rv = GetShouldSkipCheckDefaultBrowser(&skipDefaultBrowserCheck);
-  if (NS_FAILED(rv)) {
-    return rv;
-  }
-  if (skipDefaultBrowserCheck) {
-    *aResult = false;
-    return rv;
-  }
-#endif
-
   nsCOMPtr<nsIPrefBranch> prefs(do_GetService(NS_PREFSERVICE_CONTRACTID, &rv));
   if (NS_FAILED(rv)) {
     return rv;