Bug 1118285 - The browser.newtab.url preference is abused and should be removed.
authorNihanth Subramanya <nhnt11@.gmail.com>
Wed, 27 May 2015 17:35:03 -0700
changeset 250359 a03993fdddd818553758b8d282a29ff456d650ee
parent 250358 bfa8b42bd2803ccac497b0c113856c2d863b315f
child 250360 0f598226e3faa8f5c2c7f52d0e5b9ea56013009b
push id13752
push usernhnt11@gmail.com
push dateSat, 27 Jun 2015 00:25:50 +0000
treeherderfx-team@a03993fdddd8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1118285
milestone41.0a1
Bug 1118285 - The browser.newtab.url preference is abused and should be removed.
browser/app/profile/firefox.js
browser/base/content/browser.js
browser/base/content/tabbrowser.xml
browser/base/content/test/general/browser_bug763468_perwindowpb.js
browser/base/content/test/general/browser_bug767836_perwindowpb.js
browser/base/content/utilityOverlay.js
browser/components/privatebrowsing/test/browser/browser_privatebrowsing_urlbarfocus.js
browser/modules/NewTabURL.jsm
browser/modules/moz.build
browser/modules/test/xpcshell/test_NewTabURL.js
browser/modules/test/xpcshell/xpcshell.ini
docshell/base/nsDocShell.cpp
embedding/browser/nsIWebBrowserChrome3.idl
testing/talos/talos.json
xpfe/appshell/nsContentTreeOwner.cpp
xpfe/appshell/nsIXULBrowserWindow.idl
--- a/browser/app/profile/firefox.js
+++ b/browser/app/profile/firefox.js
@@ -1660,18 +1660,16 @@ pref("devtools.telemetry.tools.opened.ve
 // preference is a string so that localizers can alter it.
 pref("browser.menu.showCharacterEncoding", "chrome://browser/locale/browser.properties");
 
 // Allow using tab-modal prompts when possible.
 pref("prompts.tab_modal.enabled", true);
 // Whether the Panorama should animate going in/out of tabs
 pref("browser.panorama.animate_zoom", true);
 
-// Defines the url to be used for new tabs.
-pref("browser.newtab.url", "about:newtab");
 // Activates preloading of the new tab url.
 pref("browser.newtab.preload", true);
 
 // Remembers if the about:newtab intro has been shown
 pref("browser.newtabpage.introShown", false);
 
 // Remembers if the about:newtab update intro has been shown
 pref("browser.newtabpage.updateIntroShown", false);
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -49,16 +49,18 @@ XPCOMUtils.defineLazyServiceGetter(this,
                                    "mozIAsyncFavicons");
 XPCOMUtils.defineLazyServiceGetter(this, "gDNSService",
                                    "@mozilla.org/network/dns-service;1",
                                    "nsIDNSService");
 XPCOMUtils.defineLazyModuleGetter(this, "LightweightThemeManager",
                                   "resource://gre/modules/LightweightThemeManager.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "Pocket",
                                   "resource:///modules/Pocket.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "NewTabURL",
+                                  "resource:///modules/NewTabURL.jsm");
 
 // Can't use XPCOMUtils for these because the scripts try to define the variables
 // on window, and so the defineProperty inside defineLazyGetter fails.
 Object.defineProperty(window, "pktApi", {
   get: function() {
     // Avoid this getter running again:
     delete window.pktApi;
     Services.scriptloader.loadSubScript("chrome://browser/content/pocket/pktApi.js", window);
@@ -4086,16 +4088,20 @@ var XULBrowserWindow = {
     if (!E10SUtils.shouldLoadURI(aDocShell, aURI, aReferrer)) {
       E10SUtils.redirectLoad(aDocShell, aURI, aReferrer);
       return false;
     }
 
     return true;
   },
 
+  shouldAddToSessionHistory: function(aDocShell, aURI) {
+    return aURI.spec != NewTabURL.get();
+  },
+
   onProgressChange: function (aWebProgress, aRequest,
                               aCurSelfProgress, aMaxSelfProgress,
                               aCurTotalProgress, aMaxTotalProgress) {
     // Do nothing.
   },
 
   onProgressChange64: function (aWebProgress, aRequest,
                                 aCurSelfProgress, aMaxSelfProgress,
--- a/browser/base/content/tabbrowser.xml
+++ b/browser/base/content/tabbrowser.xml
@@ -1604,17 +1604,17 @@
 
       <method name="_isPreloadingEnabled">
         <body>
           <![CDATA[
             // Preloading for the newtab page is enabled when the pref is true
             // and the URL is "about:newtab". We do not support preloading for
             // custom newtab URLs.
             return Services.prefs.getBoolPref("browser.newtab.preload") &&
-                   !Services.prefs.prefHasUserValue("browser.newtab.url");
+                   !NewTabURL.overridden;
           ]]>
         </body>
       </method>
 
       <method name="_createPreloadBrowser">
         <body>
           <![CDATA[
             // Do nothing if we have a preloaded browser already
--- a/browser/base/content/test/general/browser_bug763468_perwindowpb.js
+++ b/browser/base/content/test/general/browser_bug763468_perwindowpb.js
@@ -3,28 +3,27 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 // This test makes sure that opening a new tab in private browsing mode opens about:privatebrowsing
 function test() {
   // initialization
   waitForExplicitFinish();
   let windowsToClose = [];
   let newTab;
-  let newTabPrefName = "browser.newtab.url";
   let newTabURL;
   let mode;
 
   function doTest(aIsPrivateMode, aWindow, aCallback) {
     whenNewTabLoaded(aWindow, function () {
       if (aIsPrivateMode) {
         mode = "per window private browsing";
         newTabURL = "about:privatebrowsing";
       } else {
         mode = "normal";
-        newTabURL = Services.prefs.getCharPref(newTabPrefName) || "about:blank";
+        newTabURL = NewTabURL.get();
       }
 
       is(aWindow.gBrowser.currentURI.spec, newTabURL,
         "URL of NewTab should be " + newTabURL + " in " + mode +  " mode");
 
       aWindow.gBrowser.removeTab(aWindow.gBrowser.selectedTab);
       aCallback()
     });
--- a/browser/base/content/test/general/browser_bug767836_perwindowpb.js
+++ b/browser/base/content/test/general/browser_bug767836_perwindowpb.js
@@ -1,62 +1,61 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 function test() {
   //initialization
   waitForExplicitFinish();
-  let newTabPrefName = "browser.newtab.url";
   let newTabURL;
   let testURL = "http://example.com/";
   let mode;
 
   function doTest(aIsPrivateMode, aWindow, aCallback) {
     openNewTab(aWindow, function () {
       if (aIsPrivateMode) {
         mode = "per window private browsing";
         newTabURL = "about:privatebrowsing";
       } else {
         mode = "normal";
-        newTabURL = Services.prefs.getCharPref(newTabPrefName) || "about:blank";
+        newTabURL = NewTabURL.get();
       }
 
       // Check the new tab opened while in normal/private mode
       is(aWindow.gBrowser.selectedBrowser.currentURI.spec, newTabURL,
         "URL of NewTab should be " + newTabURL + " in " + mode +  " mode");
       // Set the custom newtab url
-      Services.prefs.setCharPref(newTabPrefName, testURL);
-      ok(Services.prefs.prefHasUserValue(newTabPrefName), "Custom newtab url is set");
+      NewTabURL.override(testURL);
+      is(NewTabURL.get(), testURL, "Custom newtab url is set");
 
       // Open a newtab after setting the custom newtab url
       openNewTab(aWindow, function () {
         is(aWindow.gBrowser.selectedBrowser.currentURI.spec, testURL,
            "URL of NewTab should be the custom url");
 
-        // clear the custom url preference
-        Services.prefs.clearUserPref(newTabPrefName);
-        ok(!Services.prefs.prefHasUserValue(newTabPrefName), "No custom newtab url is set");
+        // Clear the custom url.
+        NewTabURL.reset();
+        is(NewTabURL.get(), "about:newtab", "No custom newtab url is set");
 
         aWindow.gBrowser.removeTab(aWindow.gBrowser.selectedTab);
         aWindow.gBrowser.removeTab(aWindow.gBrowser.selectedTab);
         aWindow.close();
         aCallback()
       });
     });
   }
 
   function testOnWindow(aIsPrivate, aCallback) {
     whenNewWindowLoaded({private: aIsPrivate}, function(win) {
       executeSoon(function() aCallback(win));
     });
   }
 
   // check whether any custom new tab url has been configured
-  ok(!Services.prefs.prefHasUserValue(newTabPrefName), "No custom newtab url is set");
+  ok(!NewTabURL.overridden, "No custom newtab url is set");
 
   // test normal mode
   testOnWindow(false, function(aWindow) {
     doTest(false, aWindow, function() {
       // test private mode
       testOnWindow(true, function(aWindow) {
         doTest(true, aWindow, function() {
           finish();
--- a/browser/base/content/utilityOverlay.js
+++ b/browser/base/content/utilityOverlay.js
@@ -4,42 +4,26 @@
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 // Services = object with smart getters for common XPCOM services
 Components.utils.import("resource://gre/modules/Services.jsm");
 Components.utils.import("resource://gre/modules/XPCOMUtils.jsm");
 Components.utils.import("resource://gre/modules/PrivateBrowsingUtils.jsm");
 Components.utils.import("resource:///modules/RecentWindow.jsm");
 
-XPCOMUtils.defineLazyGetter(this, "BROWSER_NEW_TAB_URL", function () {
-  const PREF = "browser.newtab.url";
+XPCOMUtils.defineLazyModuleGetter(this, "NewTabURL",
+  "resource:///modules/NewTabURL.jsm");
 
-  function getNewTabPageURL() {
-    if (PrivateBrowsingUtils.isWindowPrivate(window) &&
-        !PrivateBrowsingUtils.permanentPrivateBrowsing &&
-        !Services.prefs.prefHasUserValue(PREF)) {
-      return "about:privatebrowsing";
-    }
-
-    let url = Services.prefs.getComplexValue(PREF, Ci.nsISupportsString).data;
-    return url || "about:blank";
+this.__defineGetter__("BROWSER_NEW_TAB_URL", () => {
+  if (PrivateBrowsingUtils.isWindowPrivate(window) &&
+      !PrivateBrowsingUtils.permanentPrivateBrowsing &&
+      !NewTabURL.overridden) {
+    return "about:privatebrowsing";
   }
-
-  function update() {
-    BROWSER_NEW_TAB_URL = getNewTabPageURL();
-  }
-
-  Services.prefs.addObserver(PREF, update, false);
-
-  addEventListener("unload", function onUnload() {
-    removeEventListener("unload", onUnload);
-    Services.prefs.removeObserver(PREF, update);
-  });
-
-  return getNewTabPageURL();
+  return NewTabURL.get();
 });
 
 var TAB_DROP_TYPE = "application/x-moz-tabbrowser-tab";
 
 var gBidiUI = false;
 
 /**
  * Determines whether the given url is considered a special URL for new tabs.
--- a/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_urlbarfocus.js
+++ b/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_urlbarfocus.js
@@ -23,19 +23,19 @@ function openNewPrivateWindow() {
 
 add_task(function* () {
   let win = yield openNewPrivateWindow();
   checkUrlbarFocus(win);
   win.close();
 });
 
 add_task(function* () {
-  Services.prefs.setCharPref("browser.newtab.url", "about:blank");
+  NewTabURL.override("about:blank");
   registerCleanupFunction(() => {
-    Services.prefs.clearUserPref("browser.newtab.url");
+    NewTabURL.reset();
   });
 
   let win = yield openNewPrivateWindow();
   checkUrlbarFocus(win);
   win.close();
 
-  Services.prefs.clearUserPref("browser.newtab.url");
+  NewTabURL.reset();
 });
new file mode 100644
--- /dev/null
+++ b/browser/modules/NewTabURL.jsm
@@ -0,0 +1,34 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+"use strict";
+
+let Cc = Components.classes;
+let Ci = Components.interfaces;
+let Cu = Components.utils;
+
+this.EXPORTED_SYMBOLS = [ "NewTabURL" ];
+
+this.NewTabURL = {
+  _url: "about:newtab",
+  _overridden: false,
+
+  get: function() {
+    return this._url;
+  },
+
+  get overridden() {
+    return this._overridden;
+  },
+
+  override: function(newURL) {
+    this._url = newURL;
+    this._overridden = true;
+  },
+
+  reset: function() {
+    this._url = "about:newtab";
+    this._overridden = false;
+  }
+};
--- a/browser/modules/moz.build
+++ b/browser/modules/moz.build
@@ -25,16 +25,17 @@ EXTRA_JS_MODULES += [
     'CustomizationTabPreloader.jsm',
     'DirectoryLinksProvider.jsm',
     'E10SUtils.jsm',
     'Feeds.jsm',
     'FormSubmitObserver.jsm',
     'FormValidationHandler.jsm',
     'HiddenFrame.jsm',
     'NetworkPrioritizer.jsm',
+    'NewTabURL.jsm',
     'offlineAppCache.jsm',
     'PanelFrame.jsm',
     'PluginContent.jsm',
     'ProcessHangMonitor.jsm',
     'ReaderParent.jsm',
     'RecentWindow.jsm',
     'RemotePrompt.jsm',
     'SelfSupportBackend.jsm',
new file mode 100644
--- /dev/null
+++ b/browser/modules/test/xpcshell/test_NewTabURL.js
@@ -0,0 +1,17 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/
+ */
+"use strict";
+
+Components.utils.import("resource:///modules/NewTabURL.jsm");
+
+function run_test() {
+  Assert.equal(NewTabURL.get(), "about:newtab", "Default newtab URL should be about:newtab");
+  let url = "http://example.com/";
+  NewTabURL.override(url);
+  Assert.ok(NewTabURL.overridden, "Newtab URL should be overridden");
+  Assert.equal(NewTabURL.get(), url, "Newtab URL should be the custom URL");
+  NewTabURL.reset();
+  Assert.ok(!NewTabURL.overridden, "Newtab URL should not be overridden");
+  Assert.equal(NewTabURL.get(), "about:newtab", "Newtab URL should be the about:newtab");
+}
--- a/browser/modules/test/xpcshell/xpcshell.ini
+++ b/browser/modules/test/xpcshell/xpcshell.ini
@@ -1,7 +1,8 @@
 [DEFAULT]
 head =
 tail =
 firefox-appdir = browser
 skip-if = toolkit == 'android' || toolkit == 'gonk'
 
 [test_DirectoryLinksProvider.js]
+[test_NewTabURL.js]
--- a/docshell/base/nsDocShell.cpp
+++ b/docshell/base/nsDocShell.cpp
@@ -11727,17 +11727,17 @@ nsDocShell::AddState(JS::Handle<JS::Valu
 bool
 nsDocShell::ShouldAddToSessionHistory(nsIURI* aURI)
 {
   // I believe none of the about: urls should go in the history. But then
   // that could just be me... If the intent is only deny about:blank then we
   // should just do a spec compare, rather than two gets of the scheme and
   // then the path.  -Gagan
   nsresult rv;
-  nsAutoCString buf, pref;
+  nsAutoCString buf;
 
   rv = aURI->GetScheme(buf);
   if (NS_FAILED(rv)) {
     return false;
   }
 
   if (buf.EqualsLiteral("about")) {
     rv = aURI->GetPath(buf);
@@ -11745,26 +11745,27 @@ nsDocShell::ShouldAddToSessionHistory(ns
       return false;
     }
 
     if (buf.EqualsLiteral("blank")) {
       return false;
     }
   }
 
-  rv = Preferences::GetDefaultCString("browser.newtab.url", &pref);
-
-  if (NS_FAILED(rv)) {
-    return true;
-  }
-
-  rv = aURI->GetSpec(buf);
-  NS_ENSURE_SUCCESS(rv, true);
-
-  return !buf.Equals(pref);
+  // Check if the webbrowser chrome wants us to proceed - by default it ensures
+  // aURI is not the newtab URI.
+  nsCOMPtr<nsIWebBrowserChrome3> browserChrome3 = do_GetInterface(mTreeOwner);
+  if (browserChrome3) {
+    bool shouldAdd;
+    rv = browserChrome3->ShouldAddToSessionHistory(this, aURI, &shouldAdd);
+    NS_ENSURE_SUCCESS(rv, true);
+    return shouldAdd;
+  }
+
+  return true;
 }
 
 nsresult
 nsDocShell::AddToSessionHistory(nsIURI* aURI, nsIChannel* aChannel,
                                 nsISupports* aOwner, bool aCloneChildren,
                                 nsISHEntry** aNewEntry)
 {
   NS_PRECONDITION(aURI, "uri is null");
--- a/embedding/browser/nsIWebBrowserChrome3.idl
+++ b/embedding/browser/nsIWebBrowserChrome3.idl
@@ -7,17 +7,17 @@
 #include "nsIDOMNode.idl"
 
 interface nsIDocShell;
 interface nsIInputStream;
 
 /**
  * nsIWebBrowserChrome3 is an extension to nsIWebBrowserChrome2.
  */
-[scriptable, uuid(9e6c2372-5d9d-4ce8-ab9e-c5df1494dc84)]
+[scriptable, uuid(da646a9c-5788-4860-88a4-bd5d0ad853da)]
 interface nsIWebBrowserChrome3 : nsIWebBrowserChrome2
 {
   /**
    * Determines the appropriate target for a link.
    *
    * @param originalTarget
    *        The original link target.
    * @param linkURI
@@ -42,9 +42,12 @@ interface nsIWebBrowserChrome3 : nsIWebB
    * @param aURI
    *        The URI being loaded.
    * @param aReferrer
    *        The referrer of the load.
    */
   bool shouldLoadURI(in nsIDocShell    aDocShell,
                      in nsIURI         aURI,
                      in nsIURI         aReferrer);
+
+  bool shouldAddToSessionHistory(in nsIDocShell aDocShell,
+                                 in nsIURI      aURI);
 };
--- a/testing/talos/talos.json
+++ b/testing/talos/talos.json
@@ -1,16 +1,16 @@
 {
     "talos.zip": {
         "url": "http://talos-bundles.pvt.build.mozilla.org/zips/talos.a6052c33d420.zip",
         "path": ""
     },
     "global": {
         "talos_repo": "https://hg.mozilla.org/build/talos",
-        "talos_revision": "39db01f6dabb"
+        "talos_revision": "33449734f99f"
     },
     "extra_options": {
         "android": [ "--apkPath=%(apk_path)s" ]
     },
     "suites": {
         "chromez": {
             "tests": ["tresize", "tcanvasmark"]
         },
--- a/xpfe/appshell/nsContentTreeOwner.cpp
+++ b/xpfe/appshell/nsContentTreeOwner.cpp
@@ -457,16 +457,34 @@ NS_IMETHODIMP nsContentTreeOwner::Should
 
   if (xulBrowserWindow)
     return xulBrowserWindow->ShouldLoadURI(aDocShell, aURI, aReferrer, _retval);
 
   *_retval = true;
   return NS_OK;
 }
 
+NS_IMETHODIMP
+nsContentTreeOwner::ShouldAddToSessionHistory(nsIDocShell *aDocShell,
+                                              nsIURI *aURI,
+                                              bool *_retval)
+{
+  NS_ENSURE_STATE(mXULWindow);
+
+  nsCOMPtr<nsIXULBrowserWindow> xulBrowserWindow;
+  mXULWindow->GetXULBrowserWindow(getter_AddRefs(xulBrowserWindow));
+
+  if (xulBrowserWindow) {
+    return xulBrowserWindow->ShouldAddToSessionHistory(aDocShell, aURI, _retval);
+  }
+
+  *_retval = true;
+  return NS_OK;
+}
+
 //*****************************************************************************
 // nsContentTreeOwner::nsIWebBrowserChrome2
 //*****************************************************************************   
 
 NS_IMETHODIMP nsContentTreeOwner::SetStatusWithContext(uint32_t aStatusType,
                                                        const nsAString &aStatusText,
                                                        nsISupports *aStatusContext)
 {
--- a/xpfe/appshell/nsIXULBrowserWindow.idl
+++ b/xpfe/appshell/nsIXULBrowserWindow.idl
@@ -14,17 +14,17 @@ interface nsIInputStream;
 interface nsIDocShell;
 interface nsITabParent;
 
 /**
  * The nsIXULBrowserWindow supplies the methods that may be called from the
  * internals of the browser area to tell the containing xul window to update
  * its ui. 
  */
-[scriptable, uuid(db89f748-9736-40b2-a172-3928aa1194b2)]
+[scriptable, uuid(8974a499-d49b-43e1-8b32-c9b3ed81be3f)]
 interface nsIXULBrowserWindow : nsISupports
 {
   /**
    * Sets the status according to JS' version of status.
    */
   void setJSStatus(in AString status);
 
   /**
@@ -57,15 +57,18 @@ interface nsIXULBrowserWindow : nsISuppo
    * @param aURI
    *        The URI being loaded.
    * @param aReferrer
    *        The referrer of the load.
    */
   bool shouldLoadURI(in nsIDocShell    aDocShell,
                      in nsIURI         aURI,
                      in nsIURI         aReferrer);
+
+  bool shouldAddToSessionHistory(in nsIDocShell aDocShell,
+                                 in nsIURI      aURI);
   /**
    * Show/hide a tooltip (when the user mouses over a link, say).
    */
   void showTooltip(in long x, in long y, in AString tooltip);
   void hideTooltip();
 };