From d4ce8fc140acadedf6b3f42540285ef03531a378 Mon Sep 17 00:00:00 2001
From: Dana Keeler <dkeeler@mozilla.com>
Date: Wed, 17 Oct 2018 21:38:24 +0000
Subject: [PATCH] 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
---
 .../content/test/siteIdentity/browser.ini     |  1 +
 .../browser_ignore_same_page_navigation.js    | 41 +++++++++++++++++++
 .../manager/ssl/nsSecureBrowserUIImpl.cpp     | 13 +++---
 3 files changed, 50 insertions(+), 5 deletions(-)
 create mode 100644 browser/base/content/test/siteIdentity/browser_ignore_same_page_navigation.js

diff --git a/browser/base/content/test/siteIdentity/browser.ini b/browser/base/content/test/siteIdentity/browser.ini
index 0b09c03de177..6befca79e77e 100644
--- a/browser/base/content/test/siteIdentity/browser.ini
+++ b/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]
diff --git a/browser/base/content/test/siteIdentity/browser_ignore_same_page_navigation.js b/browser/base/content/test/siteIdentity/browser_ignore_same_page_navigation.js
new file mode 100644
index 000000000000..548ebd780d9d
--- /dev/null
+++ b/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");
+  });
+});
diff --git a/security/manager/ssl/nsSecureBrowserUIImpl.cpp b/security/manager/ssl/nsSecureBrowserUIImpl.cpp
index 1c2e7c341985..6ddda40656a3 100644
--- a/security/manager/ssl/nsSecureBrowserUIImpl.cpp
+++ b/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;