Bug 1510569 - Port onStateChange notifications inside WebProgressChild.jsm to C++ r=baku,kmag

We now also only access the document when the state is
nsIWebProgress::STATE_STOP. The comments in the previous code indicated that
touching the document inside the event handler when the state is not STATE_STOP
would result in the content creating a new about:blank document to retrieve the
values from. However, it then went on to do this in another location, causing a
document to be created whenever we received an onStateChange event. This should
no longer occur.

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

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

devtools/client/responsive.html/browser/tunnel.js

@@ -436,7 +436,6 @@ MessageManagerTunnel.prototype = {
     "Content:LoadURIResult",
     "Content:LocationChange",
     "Content:SecurityChange",
-    "Content:StateChange",
     // Messages sent to browser.js
     "DOMTitleChanged",
     "ImageDocumentLoaded",

dom/interfaces/base/nsIBrowser.idl

@@ -5,6 +5,7 @@
 
 interface nsIContentSecurityPolicy;
 interface nsIPrincipal;
+interface nsIURI;
 interface nsIWebProgress;
 
 webidl FrameLoader;
@@ -86,4 +87,26 @@ interface nsIBrowser : nsISupports
 
   readonly attribute nsIPrincipal contentPrincipal;
   readonly attribute nsIContentSecurityPolicy csp;
+
+  /**
+   * Whether or not the browser is in the process of an nsIWebNavigation
+   * navigation method.
+   */
+  attribute boolean isNavigating;
+
+  /**
+   * Whether or not the character encoding menu may be enabled.
+   */
+  attribute boolean mayEnableCharacterEncodingMenu;
+
+  /**
+   * Called by Gecko to update the browser when its state changes.
+   *
+   * @param aCharset the new character set of the document
+   * @param aDocumentURI the URI of the current document
+   * @param aContentType the content type of the document
+   */
+  void updateForStateChange(in AString aCharset,
+                            in nsIURI aDocumentURI,
+                            in AString aContentType);
 };

dom/ipc/BrowserChild.cpp

@@ -131,6 +131,7 @@
 #include "nsWebBrowser.h"
 #include "mozilla/dom/WindowGlobalChild.h"
 #include "MMPrinter.h"
+#include "mozilla/ResultExtensions.h"
 
 #ifdef XP_WIN
 #  include "mozilla/plugins/PluginWidgetChild.h"
@@ -543,8 +544,9 @@ nsresult BrowserChild::Init(mozIDOMWindowProxy* aParent) {
   MOZ_ASSERT(docShell);
 
   const uint32_t notifyMask =
-      nsIWebProgress::NOTIFY_PROGRESS | nsIWebProgress::NOTIFY_STATUS |
-      nsIWebProgress::NOTIFY_REFRESH | nsIWebProgress::NOTIFY_CONTENT_BLOCKING;
+      nsIWebProgress::NOTIFY_STATE_ALL | nsIWebProgress::NOTIFY_PROGRESS |
+      nsIWebProgress::NOTIFY_STATUS | nsIWebProgress::NOTIFY_REFRESH |
+      nsIWebProgress::NOTIFY_CONTENT_BLOCKING;
 
   mStatusFilter = new nsBrowserStatusFilter();
 
@@ -3436,7 +3438,52 @@ NS_IMETHODIMP BrowserChild::OnStateChange(nsIWebProgress* aWebProgress,
                                           nsIRequest* aRequest,
                                           uint32_t aStateFlags,
                                           nsresult aStatus) {
-  return NS_ERROR_NOT_IMPLEMENTED;
+  if (!IPCOpen()) {
+    return NS_OK;
+  }
+
+  nsCOMPtr<nsIDocShell> docShell = do_GetInterface(WebNavigation());
+  if (!docShell) {
+    return NS_OK;
+  }
+
+  RefPtr<Document> document;
+  if (nsCOMPtr<nsPIDOMWindowOuter> outerWindow = do_GetInterface(docShell)) {
+    document = outerWindow->GetExtantDoc();
+  } else {
+    return NS_OK;
+  }
+
+  Maybe<WebProgressData> webProgressData;
+  Maybe<WebProgressStateChangeData> stateChangeData;
+  RequestData requestData;
+
+  MOZ_TRY(PrepareProgressListenerData(aWebProgress, aRequest, webProgressData,
+                                      requestData));
+
+  if (webProgressData->isTopLevel()) {
+    stateChangeData.emplace();
+
+    stateChangeData->isNavigating() = docShell->GetIsNavigating();
+    stateChangeData->mayEnableCharacterEncodingMenu() =
+        docShell->GetMayEnableCharacterEncodingMenu();
+
+    if (aStateFlags & nsIWebProgressListener::STATE_STOP) {
+      MOZ_ASSERT(document);
+
+      document->GetContentType(stateChangeData->contentType());
+      document->GetCharacterSet(stateChangeData->charset());
+      stateChangeData->documentURI() = document->GetDocumentURIObject();
+    } else {
+      stateChangeData->contentType().SetIsVoid(true);
+      stateChangeData->charset().SetIsVoid(true);
+    }
+  }
+
+  Unused << SendOnStateChange(webProgressData, requestData, aStateFlags,
+                              aStatus, stateChangeData);
+
+  return NS_OK;
 }
 
 NS_IMETHODIMP BrowserChild::OnProgressChange(nsIWebProgress* aWebProgress,

dom/ipc/BrowserParent.cpp

@@ -2273,6 +2273,54 @@ mozilla::ipc::IPCResult BrowserParent::RecvRegisterProtocolHandler(
   return IPC_OK();
 }
 
+mozilla::ipc::IPCResult BrowserParent::RecvOnStateChange(
+    const Maybe<WebProgressData>& aWebProgressData,
+    const RequestData& aRequestData, const uint32_t aStateFlags,
+    const nsresult aStatus,
+    const Maybe<WebProgressStateChangeData>& aStateChangeData) {
+  nsCOMPtr<nsIBrowser> browser =
+      mFrameElement ? mFrameElement->AsBrowser() : nullptr;
+
+  if (!browser) {
+    return IPC_OK();
+  }
+
+  nsCOMPtr<nsIWebProgress> manager;
+  nsresult rv = browser->GetRemoteWebProgressManager(getter_AddRefs(manager));
+  NS_ENSURE_SUCCESS(rv, IPC_OK());
+
+  nsCOMPtr<nsIWebProgressListener> managerAsListener =
+      do_QueryInterface(manager);
+
+  if (!managerAsListener) {
+    // We are no longer remote, so we cannot propagate this message.
+    return IPC_OK();
+  }
+
+  nsCOMPtr<nsIWebProgress> webProgress;
+  nsCOMPtr<nsIRequest> request;
+  ReconstructWebProgressAndRequest(manager, aWebProgressData, aRequestData,
+                                   webProgress, request);
+
+  if (aWebProgressData && aWebProgressData->isTopLevel()) {
+    Unused << browser->SetIsNavigating(aStateChangeData->isNavigating());
+    Unused << browser->SetMayEnableCharacterEncodingMenu(
+        aStateChangeData->mayEnableCharacterEncodingMenu());
+    Unused << browser->UpdateForStateChange(aStateChangeData->charset(),
+                                            aStateChangeData->documentURI(),
+                                            aStateChangeData->contentType());
+  } else if (aStateChangeData.isSome()) {
+    return IPC_FAIL(
+        this,
+        "Unexpected WebProgressStateChangeData for non-top-level WebProgress");
+  }
+
+  Unused << managerAsListener->OnStateChange(webProgress, request, aStateFlags,
+                                             aStatus);
+
+  return IPC_OK();
+}
+
 mozilla::ipc::IPCResult BrowserParent::RecvOnProgressChange(
     const Maybe<WebProgressData>& aWebProgressData,
     const RequestData& aRequestData, const int32_t aCurSelfProgress,

dom/ipc/BrowserParent.h

@@ -170,6 +170,12 @@ class BrowserParent final : public PBrowserParent,
                                                       const nsString& aTitle,
                                                       nsIURI* aDocURI);
 
+  mozilla::ipc::IPCResult RecvOnStateChange(
+      const Maybe<WebProgressData>& awebProgressData,
+      const RequestData& aRequestData, const uint32_t aStateFlags,
+      const nsresult aStatus,
+      const Maybe<WebProgressStateChangeData>& aStateChangeData);
+
   mozilla::ipc::IPCResult RecvOnProgressChange(
       const Maybe<WebProgressData>& aWebProgressData,
       const RequestData& aRequestData, const int32_t aCurSelfProgress,

dom/ipc/PBrowser.ipdl

@@ -118,6 +118,18 @@ struct RequestData
   nsCString matchedList;
 };
 
+struct WebProgressStateChangeData
+{
+  bool isNavigating;
+  bool mayEnableCharacterEncodingMenu;
+
+  // The following fields are only set when the aStateFlags param passed with
+  // this struct is |nsIWebProgress.STATE_STOP|.
+  nsString contentType;
+  nsString charset;
+  nsIURI documentURI;
+};
+
 nested(upto inside_cpow) sync protocol PBrowser
 {
     manager PContent;
@@ -537,6 +549,11 @@ parent:
     async RegisterProtocolHandler(nsString scheme, nsIURI handlerURI, nsString title,
                                   nsIURI documentURI);
 
+    async OnStateChange(WebProgressData? aWebProgressData,
+                        RequestData aRequestData, uint32_t aStateFlags,
+                        nsresult aStatus,
+                        WebProgressStateChangeData? aStateChangeData);
+
     async OnProgressChange(WebProgressData? aWebProgressData,
                            RequestData aRequestData, int32_t aCurSelfProgress,
                            int32_t aMaxSelfProgress, int32_t aCurTotalProgress,

toolkit/content/widgets/browser-custom-element.js

@@ -42,6 +42,12 @@ class MozBrowser extends MozElements.MozElementMixin(XULFrameElement) {
 
     this.onPageHide = this.onPageHide.bind(this);
 
+    this.isNavigating = false;
+
+    this._documentURI = null;
+    this._characterSet = null;
+    this._documentContentType = null;
+
     /**
      * These are managed by the tabbrowser:
      */
@@ -352,6 +358,16 @@ class MozBrowser extends MozElements.MozElementMixin(XULFrameElement) {
     return this.contentDocument ? this.contentDocument.contentType : null;
   }
 
+  set documentContentType(aContentType) {
+    if (aContentType != null) {
+      if (this.isRemoteBrowser) {
+        this._documentContentType = aContentType;
+      } else {
+        this.contentDocument.documentContentType = aContentType;
+      }
+    }
+  }
+
   set sameProcessAsFrameLoader(val) {
     this._sameProcessAsFrameLoader = Cu.getWeakReference(val);
   }
@@ -594,6 +610,12 @@ class MozBrowser extends MozElements.MozElementMixin(XULFrameElement) {
     return this.isRemoteBrowser ? this._mayEnableCharacterEncodingMenu : this.docShell.mayEnableCharacterEncodingMenu;
   }
 
+  set mayEnableCharacterEncodingMenu(aMayEnable) {
+    if (this.isRemoteBrowser) {
+      this._mayEnableCharacterEncodingMenu = aMayEnable;
+    }
+  }
+
   get contentPrincipal() {
     return this.isRemoteBrowser ? this._contentPrincipal : this.contentDocument.nodePrincipal;
   }
@@ -1370,6 +1392,22 @@ class MozBrowser extends MozElements.MozElementMixin(XULFrameElement) {
     return this._remoteWebProgressManager;
   }
 
+  updateForStateChange(aCharset, aDocumentURI, aContentType) {
+    if (this.isRemoteBrowser && this.messageManager) {
+      if (aCharset != null) {
+        this._characterSet = aCharset;
+      }
+
+      if (aDocumentURI != null) {
+        this._documentURI = aDocumentURI;
+      }
+
+      if (aContentType != null) {
+        this._documentContentType = aContentType;
+      }
+    }
+  }
+
   purgeSessionHistory() {
     if (this.isRemoteBrowser) {
       try {

toolkit/modules/RemoteWebProgress.jsm

@@ -28,7 +28,6 @@ class RemoteWebProgressManager {
 
   swapBrowser(aBrowser) {
     if (this._messageManager) {
-      this._messageManager.removeMessageListener("Content:StateChange", this);
       this._messageManager.removeMessageListener("Content:LocationChange", this);
       this._messageManager.removeMessageListener("Content:SecurityChange", this);
       this._messageManager.removeMessageListener("Content:LoadURIResult", this);
@@ -36,7 +35,6 @@ class RemoteWebProgressManager {
 
     this._browser = aBrowser;
     this._messageManager = aBrowser.messageManager;
-    this._messageManager.addMessageListener("Content:StateChange", this);
     this._messageManager.addMessageListener("Content:LocationChange", this);
     this._messageManager.addMessageListener("Content:SecurityChange", this);
     this._messageManager.addMessageListener("Content:LoadURIResult", this);
@@ -203,13 +201,6 @@ class RemoteWebProgressManager {
     }
 
     switch (aMessage.name) {
-    case "Content:StateChange":
-      if (isTopLevel) {
-        this._browser._documentURI = Services.io.newURI(json.documentURI);
-      }
-      this.onStateChange(webProgress, request, json.stateFlags, json.status);
-      break;
-
     case "Content:LocationChange":
       let location = Services.io.newURI(json.location);
       let flags = json.flags;

toolkit/modules/WebProgressChild.jsm

@@ -25,9 +25,10 @@ class WebProgressChild {
   constructor(mm) {
     this.mm = mm;
 
-    // NOTIFY_PROGRESS, NOTIFY_STATUS, NOTIFY_REFRESH, and
+    // NOTIFY_PROGRESS, NOTIFY_STATE_ALL, NOTIFY_STATUS, NOTIFY_REFRESH, and
     // NOTIFY_CONTENT_BLOCKING are handled by PBrowser.
     let notifyCode = Ci.nsIWebProgress.NOTIFY_ALL &
+                        ~Ci.nsIWebProgress.NOTIFY_STATE_ALL &
                         ~Ci.nsIWebProgress.NOTIFY_PROGRESS &
                         ~Ci.nsIWebProgress.NOTIFY_STATUS &
                         ~Ci.nsIWebProgress.NOTIFY_REFRESH &
@@ -99,26 +100,6 @@ class WebProgressChild {
     this.mm.sendAsyncMessage(name, data);
   }
 
-  onStateChange(aWebProgress, aRequest, aStateFlags, aStatus) {
-    let json = this._setupJSON(aWebProgress, aRequest, aStateFlags);
-
-    json.stateFlags = aStateFlags;
-    json.status = aStatus;
-
-    // It's possible that this state change was triggered by
-    // loading an internal error page, for which the parent
-    // will want to know some details, so we'll update it with
-    // the documentURI.
-    if (aWebProgress && aWebProgress.isTopLevel) {
-      json.documentURI = this.mm.content.document.documentURIObject.spec;
-      json.charset = this.mm.content.document.characterSet;
-      json.mayEnableCharacterEncodingMenu = this.mm.docShell.mayEnableCharacterEncodingMenu;
-      json.isNavigating = this.mm.docShell.isNavigating;
-    }
-
-    this._send("Content:StateChange", json);
-  }
-
   onLocationChange(aWebProgress, aRequest, aLocationURI, aFlags) {
     let json = this._setupJSON(aWebProgress, aRequest);