Bug 1746383 - Set URLBar pageProxyState to invalid for onLocationChange triggered by SessionStore. r=nika,dao

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

browser/base/content/browser.js

@@ -5363,11 +5363,15 @@ var XULBrowserWindow = {
       this.reloadCommand.removeAttribute("disabled");
     }
 
+    let isSessionRestore = !!(
+      aFlags & Ci.nsIWebProgressListener.LOCATION_CHANGE_SESSION_STORE
+    );
+
     // We want to update the popup visibility if we received this notification
     // via simulated locationchange events such as switching between tabs, however
     // if this is a document navigation then PopupNotifications will be updated
     // via TabsProgressListener.onLocationChange and we do not want it called twice
-    gURLBar.setURI(aLocationURI, aIsSimulated);
+    gURLBar.setURI(aLocationURI, aIsSimulated, isSessionRestore);
 
     BookmarkingUI.onLocationChange();
     // If we've actually changed document, update the toolbar visibility.

browser/components/sessionstore/ContentRestore.jsm

@@ -122,7 +122,7 @@ ContentRestoreInternal.prototype = {
     let activePageData = tabData.entries[activeIndex] || {};
     let uri = activePageData.url || null;
     if (uri && !loadArguments) {
-      webNavigation.setCurrentURI(Services.io.newURI(uri));
+      webNavigation.setCurrentURIForSessionStore(Services.io.newURI(uri));
     }
 
     SessionHistory.restore(this.docShell, tabData);
@@ -178,7 +178,9 @@ ContentRestoreInternal.prototype = {
     // load happens. Don't bother doing this if we're restoring immediately
     // due to a process switch.
     if (!isRemotenessUpdate) {
-      webNavigation.setCurrentURI(Services.io.newURI("about:blank"));
+      webNavigation.setCurrentURIForSessionStore(
+        Services.io.newURI("about:blank")
+      );
     }
 
     try {
@@ -362,7 +364,9 @@ HistoryListener.prototype = {
 
     // Reset the tab's URL to what it's actually showing. Without this loadURI()
     // would use the current document and change the displayed URL only.
-    this.webNavigation.setCurrentURI(Services.io.newURI("about:blank"));
+    this.webNavigation.setCurrentURIForSessionStore(
+      Services.io.newURI("about:blank")
+    );
 
     // Kick off a new load so that we navigate away from about:blank to the
     // new URL that was passed to loadURI(). The new load will cause a

browser/components/sessionstore/test/browser_async_window_flushing.js

@@ -115,7 +115,7 @@ add_task(async function test_remove_uninteresting_window() {
   // history entries and make itself seem uninteresting.
   await SpecialPowers.spawn(browser, [], async function() {
     // Epic hackery to make this browser seem suddenly boring.
-    docShell.setCurrentURI(Services.io.newURI("about:blank"));
+    docShell.setCurrentURIForSessionStore(Services.io.newURI("about:blank"));
 
     if (!SpecialPowers.Services.appinfo.sessionHistoryInParent) {
       let { sessionHistory } = docShell.QueryInterface(Ci.nsIWebNavigation);

browser/components/urlbar/UrlbarInput.jsm

@@ -321,8 +321,11 @@ class UrlbarInput {
    * @param {boolean} [dueToTabSwitch]
    *        True if this is being called due to switching tabs and false
    *        otherwise.
+   * @param {boolean} [dueToSessionRestore]
+   *        True if this is being called due to session restore and false
+   *        otherwise.
    */
-  setURI(uri = null, dueToTabSwitch = false) {
+  setURI(uri = null, dueToTabSwitch = false, dueToSessionRestore = false) {
     let value = this.window.gBrowser.userTypedValue;
     let valid = false;
 
@@ -355,9 +358,13 @@ class UrlbarInput {
           value = "about:blank";
         }
       }
-
+      // If we update the URI while restoring a session, set the proxyState to
+      // invalid, because we don't have a valid security state to show via site
+      // identity yet. See Bug 1746383.
       valid =
-        !this.window.isBlankPageURL(uri.spec) || uri.schemeIs("moz-extension");
+        !dueToSessionRestore &&
+        (!this.window.isBlankPageURL(uri.spec) ||
+          uri.schemeIs("moz-extension"));
     } else if (
       this.window.isInitialPage(value) &&
       BrowserUIUtils.checkEmptyPageOrigin(this.window.gBrowser.selectedBrowser)

docshell/base/nsDocShell.cpp

@@ -1572,12 +1572,16 @@ nsDocShell::GetChromeEventHandler(EventTarget** aChromeEventHandler) {
 }
 
 NS_IMETHODIMP
-nsDocShell::SetCurrentURI(nsIURI* aURI) {
+nsDocShell::SetCurrentURIForSessionStore(nsIURI* aURI) {
   // Note that securityUI will set STATE_IS_INSECURE, even if
   // the scheme of |aURI| is "https".
-  SetCurrentURI(aURI, nullptr, /* aFireOnLocationChange */ true,
-                /* aIsInitialAboutBlank */ false,
-                /* aLocationFlags */ 0);
+  SetCurrentURI(aURI, nullptr,
+                /* aFireOnLocationChange */
+                true,
+                /* aIsInitialAboutBlank */
+                false,
+                /* aLocationFlags */
+                nsIWebProgressListener::LOCATION_CHANGE_SESSION_STORE);
   return NS_OK;
 }
 

docshell/base/nsIDocShell.idl

@@ -102,11 +102,10 @@ interface nsIDocShell : nsIDocShellTreeItem
   void prepareForNewContentModel();
 
   /**
-   * For editors and suchlike who wish to change the URI associated with the
-   * document. Note if you want to get the current URI, use the read-only
-   * property on nsIWebNavigation.
+   * Helper for the session store to change the URI associated with the
+   * document.
    */
-  void setCurrentURI(in nsIURI aURI);
+  void setCurrentURIForSessionStore(in nsIURI aURI);
 
   /**
    * Notify the associated content viewer and all child docshells that they are

toolkit/components/sessionstore/SessionStoreUtils.cpp

@@ -1489,7 +1489,7 @@ void SessionStoreUtils::RestoreDocShellState(
     nsIDocShell* aDocShell, const DocShellRestoreState& aState) {
   if (aDocShell) {
     if (aState.URI()) {
-      aDocShell->SetCurrentURI(aState.URI());
+      aDocShell->SetCurrentURIForSessionStore(aState.URI());
     }
     RestoreDocShellCapabilities(aDocShell, aState.docShellCaps());
   }

uriloader/base/nsIWebProgressListener.idl

@@ -450,11 +450,16 @@ interface nsIWebProgressListener : nsISupports
    *   This flag is on for same-document location changes where only the URI's
    *   reference fragment has changed. This flag implies
    *   LOCATION_CHANGE_SAME_DOCUMENT.
+   *
+   * LOCATION_CHANGE_SESSION_STORE
+   *   This flag is on for location changes that are the result of the session
+   *   store updating the URI of aWebProgress without actually navigating.
    */
   const unsigned long LOCATION_CHANGE_SAME_DOCUMENT   = 0x00000001;
   const unsigned long LOCATION_CHANGE_ERROR_PAGE      = 0x00000002;
   const unsigned long LOCATION_CHANGE_RELOAD          = 0x00000004;
   const unsigned long LOCATION_CHANGE_HASHCHANGE      = 0x00000008;
+  const unsigned long LOCATION_CHANGE_SESSION_STORE   = 0x00000010;
 
   /**
    * Called when the location of the window being watched changes.  This is not