bug 1497555 - filter out same-document location changes in nsSecureBrowserUIImpl::OnLocationChange r=Ehsan

If nsSecureBrowserUIImpl::OnLocationChange receives a
LOCATION_CHANGE_SAME_DOCUMENT notification, it doesn't need to (and in fact
shouldn't) update its security state or notify downstream listeners.

Differential Revision: https://phabricator.services.mozilla.com/D8900

--HG--
extra : moz-landing-system : lando

browser/base/content/test/siteIdentity/browser.ini

@@ -109,3 +109,4 @@ support-files =
   iframe_navigation.html
 [browser_navigation_failures.js]
 [browser_secure_transport_insecure_scheme.js]
+[browser_ignore_same_page_navigation.js]

browser/base/content/test/siteIdentity/browser_ignore_same_page_navigation.js

@@ -0,0 +1,41 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=8 sts=2 et sw=2 tw=80: */
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+// Test that the nsISecureBrowserUI implementation doesn't send extraneous OnSecurityChange events
+// when it receives OnLocationChange events with the LOCATION_CHANGE_SAME_DOCUMENT flag set.
+
+add_task(async function() {
+  await BrowserTestUtils.withNewTab("about:blank", async (browser) => {
+    let onLocationChangeCount = 0;
+    let onSecurityChangeCount = 0;
+    let progressListener = {
+      onStateChange() {},
+      onLocationChange() {
+        onLocationChangeCount++;
+      },
+      onSecurityChange() {
+        onSecurityChangeCount++;
+      },
+      onProgressChange() {},
+      onStatusChange() {},
+
+      QueryInterface: ChromeUtils.generateQI([Ci.nsIWebProgressListener,
+                                              Ci.nsISupportsWeakReference]),
+    };
+    browser.addProgressListener(progressListener, Ci.nsIWebProgress.NOTIFY_ALL);
+
+    let uri = getRootDirectory(gTestPath).replace("chrome://mochitests/content",
+                                                  "https://example.com") + "dummy_page.html";
+    BrowserTestUtils.loadURI(browser, uri);
+    await BrowserTestUtils.browserLoaded(browser, false, uri);
+    is(onLocationChangeCount, 1, "should have 1 onLocationChange event");
+    is(onSecurityChangeCount, 1, "should have 1 onSecurityChange event");
+    await ContentTask.spawn(browser, null, async () => {
+      content.history.pushState({}, "", "https://example.com");
+    });
+    is(onLocationChangeCount, 2, "should have 2 onLocationChange events");
+    is(onSecurityChangeCount, 1, "should still have only 1 onSecurityChange event");
+  });
+});

security/manager/ssl/nsSecureBrowserUIImpl.cpp

@@ -396,13 +396,16 @@ nsSecureBrowserUIImpl::OnLocationChange(nsIWebProgress* aWebProgress,
     return NS_OK;
   }
 
-  // Clear any state that varies by location.
-  if (!(aFlags & LOCATION_CHANGE_SAME_DOCUMENT)) {
-    mOldState = 0;
-    mState = 0;
-    mTopLevelSecurityInfo = nullptr;
+  // If this is a same-document location change, we don't need to update our
+  // state or notify anyone.
+  if (aFlags & LOCATION_CHANGE_SAME_DOCUMENT) {
+    return NS_OK;
   }
 
+  mOldState = 0;
+  mState = 0;
+  mTopLevelSecurityInfo = nullptr;
+
   if (aFlags & LOCATION_CHANGE_ERROR_PAGE) {
     mState = STATE_IS_INSECURE;
     mTopLevelSecurityInfo = nullptr;