From 93a50953e0e69ec91f152d32dbb532ceefff676e Mon Sep 17 00:00:00 2001 From: Barret Rennie Date: Thu, 2 May 2019 23:36:24 +0000 Subject: [PATCH] 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 --- .../client/responsive.html/browser/tunnel.js | 1 - dom/interfaces/base/nsIBrowser.idl | 23 ++++++++ dom/ipc/BrowserChild.cpp | 53 +++++++++++++++++-- dom/ipc/BrowserParent.cpp | 48 +++++++++++++++++ dom/ipc/BrowserParent.h | 6 +++ dom/ipc/PBrowser.ipdl | 17 ++++++ .../content/widgets/browser-custom-element.js | 38 +++++++++++++ toolkit/modules/RemoteWebProgress.jsm | 9 ---- toolkit/modules/WebProgressChild.jsm | 23 +------- 9 files changed, 184 insertions(+), 34 deletions(-) diff --git a/devtools/client/responsive.html/browser/tunnel.js b/devtools/client/responsive.html/browser/tunnel.js index ac0898c0ffbe..187dc0d582f0 100644 --- a/devtools/client/responsive.html/browser/tunnel.js +++ b/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", diff --git a/dom/interfaces/base/nsIBrowser.idl b/dom/interfaces/base/nsIBrowser.idl index 7137ca8fde53..c271bac0b97c 100644 --- a/dom/interfaces/base/nsIBrowser.idl +++ b/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); }; diff --git a/dom/ipc/BrowserChild.cpp b/dom/ipc/BrowserChild.cpp index 5a7cd9d8fb7e..208149aa7bea 100644 --- a/dom/ipc/BrowserChild.cpp +++ b/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 docShell = do_GetInterface(WebNavigation()); + if (!docShell) { + return NS_OK; + } + + RefPtr document; + if (nsCOMPtr outerWindow = do_GetInterface(docShell)) { + document = outerWindow->GetExtantDoc(); + } else { + return NS_OK; + } + + Maybe webProgressData; + Maybe 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, diff --git a/dom/ipc/BrowserParent.cpp b/dom/ipc/BrowserParent.cpp index 9c04fe627b7a..cf4aacc5b38e 100644 --- a/dom/ipc/BrowserParent.cpp +++ b/dom/ipc/BrowserParent.cpp @@ -2273,6 +2273,54 @@ mozilla::ipc::IPCResult BrowserParent::RecvRegisterProtocolHandler( return IPC_OK(); } +mozilla::ipc::IPCResult BrowserParent::RecvOnStateChange( + const Maybe& aWebProgressData, + const RequestData& aRequestData, const uint32_t aStateFlags, + const nsresult aStatus, + const Maybe& aStateChangeData) { + nsCOMPtr browser = + mFrameElement ? mFrameElement->AsBrowser() : nullptr; + + if (!browser) { + return IPC_OK(); + } + + nsCOMPtr manager; + nsresult rv = browser->GetRemoteWebProgressManager(getter_AddRefs(manager)); + NS_ENSURE_SUCCESS(rv, IPC_OK()); + + nsCOMPtr managerAsListener = + do_QueryInterface(manager); + + if (!managerAsListener) { + // We are no longer remote, so we cannot propagate this message. + return IPC_OK(); + } + + nsCOMPtr webProgress; + nsCOMPtr 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& aWebProgressData, const RequestData& aRequestData, const int32_t aCurSelfProgress, diff --git a/dom/ipc/BrowserParent.h b/dom/ipc/BrowserParent.h index a12aafe38d5f..32c3f025fc40 100644 --- a/dom/ipc/BrowserParent.h +++ b/dom/ipc/BrowserParent.h @@ -170,6 +170,12 @@ class BrowserParent final : public PBrowserParent, const nsString& aTitle, nsIURI* aDocURI); + mozilla::ipc::IPCResult RecvOnStateChange( + const Maybe& awebProgressData, + const RequestData& aRequestData, const uint32_t aStateFlags, + const nsresult aStatus, + const Maybe& aStateChangeData); + mozilla::ipc::IPCResult RecvOnProgressChange( const Maybe& aWebProgressData, const RequestData& aRequestData, const int32_t aCurSelfProgress, diff --git a/dom/ipc/PBrowser.ipdl b/dom/ipc/PBrowser.ipdl index 5e57adec95c6..b411cb643bb1 100644 --- a/dom/ipc/PBrowser.ipdl +++ b/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, diff --git a/toolkit/content/widgets/browser-custom-element.js b/toolkit/content/widgets/browser-custom-element.js index 2682f54dc159..da902cd6e057 100644 --- a/toolkit/content/widgets/browser-custom-element.js +++ b/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 { diff --git a/toolkit/modules/RemoteWebProgress.jsm b/toolkit/modules/RemoteWebProgress.jsm index ceb7b786ad5a..d37faeae5732 100644 --- a/toolkit/modules/RemoteWebProgress.jsm +++ b/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; diff --git a/toolkit/modules/WebProgressChild.jsm b/toolkit/modules/WebProgressChild.jsm index 591fc85a3c31..1c526bae7815 100644 --- a/toolkit/modules/WebProgressChild.jsm +++ b/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);