Bug 776167 - allow going back to about:newtab, r?ursula,smaug draft
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Thu, 10 Aug 2017 10:46:19 +0100
changeset 647490 d762678c86560b989dfdd15b7787d0ecadb6a650
parent 646498 564e82f0f289af976da01c2d50507017bbc152b5
child 726517 5230481f482ccff3c83b0942dbcdb3a8ff8299b3
push id74420
push userbmo:gijskruitbosch+bugs@gmail.com
push dateWed, 16 Aug 2017 11:36:10 +0000
reviewersursula, smaug
bugs776167
milestone57.0a1
Bug 776167 - allow going back to about:newtab, r?ursula,smaug MozReview-Commit-ID: 8FTPI30RqYL
browser/base/content/test/general/browser_bug724239.js
docshell/base/nsDocShell.cpp
docshell/base/nsDocShell.h
--- a/browser/base/content/test/general/browser_bug724239.js
+++ b/browser/base/content/test/general/browser_bug724239.js
@@ -1,11 +1,36 @@
 /* Any copyright is dedicated to the Public Domain.
  * http://creativecommons.org/publicdomain/zero/1.0/ */
 
-add_task(async function test() {
+add_task(async function test_blank() {
   await BrowserTestUtils.withNewTab({ gBrowser, url: "about:blank" },
                                     async function(browser) {
     BrowserTestUtils.loadURI(browser, "http://example.com");
     await BrowserTestUtils.browserLoaded(browser);
-    ok(!gBrowser.canGoBack, "about:newtab wasn't added to the session history");
+    ok(!gBrowser.canGoBack, "about:blank wasn't added to session history");
   });
 });
+
+add_task(async function test_newtab() {
+  await SpecialPowers.pushPrefEnv({set: [["browser.newtabpage.activity-stream.enabled", true]]});
+  await BrowserTestUtils.withNewTab({ gBrowser, url: "about:blank" },
+                                    async function(browser) {
+    // Can't load it directly because that'll use a preloaded tab if present.
+    BrowserTestUtils.loadURI(browser, "about:newtab");
+    await BrowserTestUtils.browserLoaded(browser);
+
+    BrowserTestUtils.loadURI(browser, "http://example.com");
+    await BrowserTestUtils.browserLoaded(browser);
+    is(gBrowser.canGoBack, true, "about:newtab was added to the session history when AS was enabled.");
+  });
+  await SpecialPowers.pushPrefEnv({set: [["browser.newtabpage.activity-stream.enabled", false]]});
+  await BrowserTestUtils.withNewTab({ gBrowser, url: "about:blank" },
+                                    async function(browser) {
+    // Can't load it directly because that'll use a preloaded tab if present.
+    BrowserTestUtils.loadURI(browser, "about:newtab");
+    await BrowserTestUtils.browserLoaded(browser);
+
+    BrowserTestUtils.loadURI(browser, "http://example.com");
+    await BrowserTestUtils.browserLoaded(browser);
+    is(gBrowser.canGoBack, false, "about:newtab was not added to the session history when AS was disabled.");
+  });
+});
--- a/docshell/base/nsDocShell.cpp
+++ b/docshell/base/nsDocShell.cpp
@@ -12363,17 +12363,17 @@ nsDocShell::SetCurrentScrollRestorationI
   if (mOSHE) {
     mOSHE->SetScrollRestorationIsManual(aIsManual);
   }
 
   return NS_OK;
 }
 
 bool
-nsDocShell::ShouldAddToSessionHistory(nsIURI* aURI)
+nsDocShell::ShouldAddToSessionHistory(nsIURI* aURI, nsIChannel* aChannel)
 {
   // 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;
 
@@ -12383,19 +12383,29 @@ nsDocShell::ShouldAddToSessionHistory(ns
   }
 
   if (buf.EqualsLiteral("about")) {
     rv = aURI->GetPathQueryRef(buf);
     if (NS_FAILED(rv)) {
       return false;
     }
 
-    if (buf.EqualsLiteral("blank") || buf.EqualsLiteral("newtab")) {
+    if (buf.EqualsLiteral("blank")) {
       return false;
     }
+    // We only want to add about:newtab if it's not privileged:
+    if (buf.EqualsLiteral("newtab")) {
+      NS_ENSURE_TRUE(aChannel, false);
+      nsCOMPtr<nsIPrincipal> resultPrincipal;
+      rv = nsContentUtils::GetSecurityManager()->
+             GetChannelResultPrincipal(aChannel,
+                                       getter_AddRefs(resultPrincipal));
+      NS_ENSURE_SUCCESS(rv, false);
+      return !nsContentUtils::IsSystemPrincipal(resultPrincipal);
+    }
   }
 
   return true;
 }
 
 nsresult
 nsDocShell::AddToSessionHistory(nsIURI* aURI, nsIChannel* aChannel,
                                 nsIPrincipal* aTriggeringPrincipal,
@@ -12418,19 +12428,16 @@ nsDocShell::AddToSessionHistory(nsIURI* 
     MOZ_LOG(gDocShellLog, LogLevel::Debug,
             ("nsDocShell[%p]::AddToSessionHistory(\"%s\", [%s])\n",
              this, aURI->GetSpecOrDefault().get(), chanName.get()));
   }
 #endif
 
   nsresult rv = NS_OK;
   nsCOMPtr<nsISHEntry> entry;
-  bool shouldPersist;
-
-  shouldPersist = ShouldAddToSessionHistory(aURI);
 
   // Get a handle to the root docshell
   nsCOMPtr<nsIDocShellTreeItem> root;
   GetSameTypeRootTreeItem(getter_AddRefs(root));
   /*
    * If this is a LOAD_FLAGS_REPLACE_HISTORY in a subframe, we use
    * the existing SH entry in the page and replace the url and
    * other vitalities.
@@ -12622,16 +12629,18 @@ nsDocShell::AddToSessionHistory(nsIURI* 
     }
 
     if (addToSHistory) {
       // Add to session history
       nsCOMPtr<nsISHistoryInternal> shPrivate =
         do_QueryInterface(mSessionHistory);
       NS_ENSURE_TRUE(shPrivate, NS_ERROR_FAILURE);
       mSessionHistory->GetIndex(&mPreviousTransIndex);
+
+      bool shouldPersist = ShouldAddToSessionHistory(aURI, aChannel);
       rv = shPrivate->AddEntry(entry, shouldPersist);
       mSessionHistory->GetIndex(&mLoadedTransIndex);
 #ifdef DEBUG_PAGE_CACHE
       printf("Previous index: %d, Loaded index: %d\n\n",
              mPreviousTransIndex, mLoadedTransIndex);
 #endif
     }
   } else {
--- a/docshell/base/nsDocShell.h
+++ b/docshell/base/nsDocShell.h
@@ -442,17 +442,17 @@ protected:
                 bool aFireOnLocationChange,
                 bool aAddToGlobalHistory,
                 bool aCloneSHChildren);
 
   void SetReferrerURI(nsIURI* aURI);
   void SetReferrerPolicy(uint32_t aReferrerPolicy);
 
   // Session History
-  bool ShouldAddToSessionHistory(nsIURI* aURI);
+  bool ShouldAddToSessionHistory(nsIURI* aURI, nsIChannel* aChannel);
   // Either aChannel or aOwner must be null. If aChannel is
   // present, the owner should be gotten from it.
   // If aCloneChildren is true, then our current session history's
   // children will be cloned onto the new entry. This should be
   // used when we aren't actually changing the document while adding
   // the new session history entry.
   nsresult AddToSessionHistory(nsIURI* aURI, nsIChannel* aChannel,
                                nsIPrincipal* aTriggeringPrincipal,