Bug 1118285 - The browser.newtab.url preference is abused and should be removed.
☠☠ backed out by 0e92a104e351 ☠ ☠
authorNihanth Subramanya <nhnt11@.gmail.com>
Wed, 27 May 2015 17:35:03 -0700
changeset 247958 3f9e71094c020d4a0447a8e2534629c7bf0c3083
parent 247957 73d0aad18e5fc40df2a845c69edb2c088d03e1c4
child 247959 612e73e50be6899bb48c25211ed1dca17b51163e
push id28887
push userkwierso@gmail.com
push dateThu, 11 Jun 2015 01:10:53 +0000
treeherdermozilla-central@1fd19d8fc936 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1118285
milestone41.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 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
xpfe/appshell/nsContentTreeOwner.cpp
xpfe/appshell/nsIXULBrowserWindow.idl
--- a/browser/app/profile/firefox.js
+++ b/browser/app/profile/firefox.js
@@ -1644,18 +1644,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);
@@ -4088,16 +4090,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
@@ -1600,17 +1600,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
@@ -24,16 +24,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
@@ -11733,17 +11733,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);
@@ -11751,26 +11751,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/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();
 };