Bug 1419007 - ensure errors FireOnLocationChange after documentURI has changed in docshell. r=sawang r=francois
authorJonathan Kingston <jkt@mozilla.com>
Tue, 21 Nov 2017 11:40:01 +0000
changeset 396691 0fc5c1fe3fea2be37226fc4cc41179f18909e8ab
parent 396690 12981d0630956bff62378600e011d3a396969590
child 396692 724d02b6417b65bf7495962246c0a5953d50dda7
push id98342
push userarchaeopteryx@coole-files.de
push dateMon, 18 Dec 2017 14:42:06 +0000
treeherdermozilla-inbound@724d02b6417b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssawang, francois
bugs1419007
milestone59.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 1419007 - ensure errors FireOnLocationChange after documentURI has changed in docshell. r=sawang r=francois MozReview-Commit-ID: AaZEIOI4sW1
browser/base/content/test/siteIdentity/browser_check_identity_state.js
browser/base/content/test/siteIdentity/dummy_page.html
docshell/base/nsDocShell.cpp
docshell/test/browser/browser_uriFixupAlternateRedirects.js
toolkit/components/url-classifier/nsUrlClassifierUtils.cpp
--- a/browser/base/content/test/siteIdentity/browser_check_identity_state.js
+++ b/browser/base/content/test/siteIdentity/browser_check_identity_state.js
@@ -1,15 +1,15 @@
 /*
  * Test the identity mode UI for a variety of page types
  */
 
 "use strict";
 
-const DUMMY = "browser/browser/base/content/test/general/dummy_page.html";
+const DUMMY = "browser/browser/base/content/test/siteIdentity/dummy_page.html";
 const INSECURE_ICON_PREF = "security.insecure_connection_icon.enabled";
 const INSECURE_PBMODE_ICON_PREF = "security.insecure_connection_icon.pbmode.enabled";
 
 function loadNewTab(url) {
   return BrowserTestUtils.openNewForegroundTab(gBrowser, url, true);
 }
 
 function getIdentityMode(aWindow = window) {
@@ -228,16 +228,44 @@ async function noCertErrorTest(secureChe
   await SpecialPowers.popPrefEnv();
 }
 
 add_task(async function test_about_net_error_uri() {
   await noCertErrorTest(true);
   await noCertErrorTest(false);
 });
 
+async function noCertErrorFromNavigationTest(secureCheck) {
+  await SpecialPowers.pushPrefEnv({set: [[INSECURE_ICON_PREF, secureCheck]]});
+  let newTab = await loadNewTab("http://example.com/" + DUMMY);
+
+  let promise = BrowserTestUtils.waitForErrorPage(gBrowser.selectedBrowser);
+  await ContentTask.spawn(gBrowser.selectedBrowser, {}, function() {
+    content.document.getElementById("no-cert").click();
+  });
+  await promise;
+  await ContentTask.spawn(gBrowser.selectedBrowser, {}, function() {
+    is(content.window.location.href, "https://nocert.example.com/", "Should be the cert error URL");
+  });
+
+
+  is(newTab.linkedBrowser.documentURI.spec.startsWith("about:certerror?"), true, "Should be an about:certerror");
+  is(getIdentityMode(), "unknownIdentity", "Identity should be unknown");
+  is(getConnectionState(), "not-secure", "Connection should be file");
+
+  gBrowser.removeTab(newTab);
+
+  await SpecialPowers.popPrefEnv();
+}
+
+add_task(async function test_about_net_error_uri_from_navigation_tab() {
+  await noCertErrorFromNavigationTest(true);
+  await noCertErrorFromNavigationTest(false);
+});
+
 async function aboutUriTest(secureCheck) {
   let oldTab = gBrowser.selectedTab;
   await SpecialPowers.pushPrefEnv({set: [[INSECURE_ICON_PREF, secureCheck]]});
   let aboutURI = "about:robots";
 
   let newTab = await loadNewTab(aboutURI);
   is(getConnectionState(), "file", "Connection should be file");
 
--- a/browser/base/content/test/siteIdentity/dummy_page.html
+++ b/browser/base/content/test/siteIdentity/dummy_page.html
@@ -1,9 +1,10 @@
 <html>
 <head>
 <title>Dummy test page</title>
 <meta http-equiv="Content-Type" content="text/html;charset=utf-8"></meta>
 </head>
 <body>
+  <a href="https://nocert.example.com" id="no-cert">No Cert page</a>
 <p>Dummy test page</p>
 </body>
 </html>
--- a/docshell/base/nsDocShell.cpp
+++ b/docshell/base/nsDocShell.cpp
@@ -9389,37 +9389,38 @@ nsDocShell::CreateContentViewer(const ns
   // Set mFiredUnloadEvent = false so that the unload handler for the
   // *new* document will fire.
   mFiredUnloadEvent = false;
 
   // we've created a new document so go ahead and call
   // OnLoadingSite(), but don't fire OnLocationChange()
   // notifications before we've called Embed(). See bug 284993.
   mURIResultedInDocument = true;
+  bool errorOnLocationChangeNeeded = false;
+  nsCOMPtr<nsIChannel> failedChannel = mFailedChannel;
+  nsCOMPtr<nsIURI> failedURI;
 
   if (mLoadType == LOAD_ERROR_PAGE) {
     // We need to set the SH entry and our current URI here and not
     // at the moment we load the page. We want the same behavior
     // of Stop() as for a normal page load. See bug 514232 for details.
 
     // Revert mLoadType to load type to state the page load failed,
     // following function calls need it.
     mLoadType = mFailedLoadType;
 
-    nsCOMPtr<nsIChannel> failedChannel = mFailedChannel;
 
     nsIDocument* doc = viewer->GetDocument();
     if (doc) {
       doc->SetFailedChannel(failedChannel);
     }
 
-    // Make sure we have a URI to set currentURI.
-    nsCOMPtr<nsIURI> failedURI;
     nsCOMPtr<nsIPrincipal> triggeringPrincipal;
     if (failedChannel) {
+      // Make sure we have a URI to set currentURI.
       NS_GetFinalChannelURI(failedChannel, getter_AddRefs(failedURI));
     }
      else {
        // if there is no failed channel we have to explicitly provide
        // a triggeringPrincipal for the history entry.
        triggeringPrincipal = nsContentUtils::GetSystemPrincipal();
     }
 
@@ -9435,24 +9436,19 @@ nsDocShell::CreateContentViewer(const ns
     // bug 291876.
     MOZ_ASSERT(failedURI, "We don't have a URI for history APIs.");
 
     mFailedChannel = nullptr;
     mFailedURI = nullptr;
 
     // Create an shistory entry for the old load.
     if (failedURI) {
-      bool errorOnLocationChangeNeeded = OnNewURI(
+      errorOnLocationChangeNeeded = OnNewURI(
         failedURI, failedChannel, triggeringPrincipal,
         nullptr, mLoadType, false, false, false);
-
-      if (errorOnLocationChangeNeeded) {
-        FireOnLocationChange(this, failedChannel, failedURI,
-                             LOCATION_CHANGE_ERROR_PAGE);
-      }
     }
 
     // Be sure to have a correct mLSHE, it may have been cleared by
     // EndPageLoad. See bug 302115.
     if (mSessionHistory && !mLSHE) {
       int32_t idx;
       mSessionHistory->GetRequestedIndex(&idx);
       if (idx == -1) {
@@ -9533,17 +9529,20 @@ nsDocShell::CreateContentViewer(const ns
   // performance over normal native event dispatch priorities.
   if (++gNumberOfDocumentsLoading == 1) {
     // Hint to favor performance for the plevent notification mechanism.
     // We want the pages to load as fast as possible even if its means
     // native messages might be starved.
     FavorPerformanceHint(true);
   }
 
-  if (onLocationChangeNeeded) {
+  if (errorOnLocationChangeNeeded){
+    FireOnLocationChange(this, failedChannel, failedURI,
+                         LOCATION_CHANGE_ERROR_PAGE);
+  } else if (onLocationChangeNeeded) {
     FireOnLocationChange(this, aRequest, mCurrentURI, 0);
   }
 
   return NS_OK;
 }
 
 nsresult
 nsDocShell::NewContentViewerObj(const nsACString& aContentType,
--- a/docshell/test/browser/browser_uriFixupAlternateRedirects.js
+++ b/docshell/test/browser/browser_uriFixupAlternateRedirects.js
@@ -11,14 +11,19 @@ add_task(async function() {
   await errorPageLoaded;
   let [contentURL, originalURL] = await ContentTask.spawn(tab.linkedBrowser, null, () => {
     return [
       content.document.documentURI,
       content.document.mozDocumentURIIfNotForErrorPages.spec,
     ];
   });
   info("Page that loaded: " + contentURL);
-  ok(contentURL.startsWith("about:neterror?"), "Should be on an error page");
+  const errorURI = "about:neterror?";
+  ok(contentURL.startsWith(errorURI), "Should be on an error page");
+
+  const contentPrincipal = tab.linkedBrowser.contentPrincipal;
+  ok(contentPrincipal.URI.spec.startsWith(errorURI), "Principal should be for the error page");
+
   originalURL = new URL(originalURL);
   is(originalURL.host, "example", "Should be an error for http://example, not http://www.example.com/");
 
   await BrowserTestUtils.removeTab(tab);
 });
--- a/toolkit/components/url-classifier/nsUrlClassifierUtils.cpp
+++ b/toolkit/components/url-classifier/nsUrlClassifierUtils.cpp
@@ -507,17 +507,17 @@ AddThreatSourceFromChannel(ThreatHit& aH
   }
 
   nsresult rv;
 
   auto matchingSource = aHit.add_resources();
   matchingSource->set_type(aType);
 
   nsCOMPtr<nsIURI> uri;
-  rv = aChannel->GetURI(getter_AddRefs(uri));
+  rv = NS_GetFinalChannelURI(aChannel, getter_AddRefs(uri));
   NS_ENSURE_SUCCESS(rv, rv);
 
   nsCString spec;
   rv = GetSpecWithoutSensitiveData(uri, spec);
   NS_ENSURE_SUCCESS(rv, rv);
   matchingSource->set_url(spec.get());
 
   nsCOMPtr<nsIHttpChannel> httpChannel =
@@ -629,22 +629,22 @@ AddTabThreatSources(ThreatHit& aHit, nsI
   if (NS_SUCCEEDED(rv) && !isTopUri) {
     nsCOMPtr<nsILoadInfo> loadInfo = aChannel->GetLoadInfo();
     if (loadInfo && loadInfo->RedirectChain().Length()) {
       AddThreatSourceFromRedirectEntry(aHit, loadInfo->RedirectChain()[0],
                                        ThreatHit_ThreatSourceType_TAB_RESOURCE);
     }
   }
 
-  // Set top level tab_url threatshource
+  // Set top level tab_url threat source
   rv = AddThreatSourceFromChannel(aHit, topChannel,
                                   ThreatHit_ThreatSourceType_TAB_URL);
   Unused << NS_WARN_IF(NS_FAILED(rv));
 
-  // Set tab_redirect threatshources if there's any
+  // Set tab_redirect threat sources if there's any
   nsCOMPtr<nsILoadInfo> topLoadInfo = topChannel->GetLoadInfo();
   if (!topLoadInfo) {
     return NS_OK;
   }
 
   nsIRedirectHistoryEntry* redirectEntry;
   size_t length = topLoadInfo->RedirectChain().Length();
   for (size_t i = 0; i < length; i++) {