From dd7f585ea3089b2f018a60347fbda6bbce0b4746 Mon Sep 17 00:00:00 2001 From: Dan Glastonbury Date: Fri, 8 May 2020 03:28:44 +0000 Subject: [PATCH] Bug 1624550 - P4: Cleanup APIs for setting BrowsingContext::UseGlobalHistory. r=farre This value is determined in Parent process and passed down to nsDocShell. Delete the messages to pass the setting down and set it on the BrowsingContext in the Parent process. Refactor the code that determines to opt-out of using global history. Code inspection determines that windowless browsing contexts want to opt-out as well as any frame with `disableglobalhistory` attribute set on it. Differential Revision: https://phabricator.services.mozilla.com/D72279 --- docshell/base/BrowsingContext.cpp | 8 ++----- docshell/base/BrowsingContext.h | 3 ++- docshell/base/nsDocShell.cpp | 22 ++++--------------- docshell/base/nsIDocShell.idl | 5 ----- dom/chrome-webidl/BrowsingContext.webidl | 5 +++++ dom/ipc/BrowserChild.cpp | 15 ------------- dom/ipc/BrowserChild.h | 2 -- dom/ipc/BrowserParent.cpp | 6 ----- dom/ipc/PBrowser.ipdl | 7 ------ .../content/SpecialPowersParent.jsm | 2 +- toolkit/components/browser/nsWebBrowser.cpp | 17 +------------- toolkit/components/browser/nsWebBrowser.h | 4 +--- .../components/extensions/ExtensionParent.jsm | 2 +- .../extensions/ExtensionXPCShellUtils.jsm | 2 +- .../chrome/browser_disableglobalhistory.xhtml | 2 +- .../content/widgets/browser-custom-element.js | 2 +- toolkit/modules/HiddenFrame.jsm | 3 ++- xpfe/appshell/nsAppShellService.cpp | 12 ++++++++-- xpfe/appshell/nsIWindowlessBrowser.idl | 8 ++++++- 19 files changed, 39 insertions(+), 88 deletions(-) diff --git a/docshell/base/BrowsingContext.cpp b/docshell/base/BrowsingContext.cpp index 32f720e05e4d..c5159f65d9ca 100644 --- a/docshell/base/BrowsingContext.cpp +++ b/docshell/base/BrowsingContext.cpp @@ -323,11 +323,6 @@ already_AddRefed BrowsingContext::CreateIndependent( return bc.forget(); } -void BrowsingContext::SetWindowless() { - MOZ_DIAGNOSTIC_ASSERT(!mEverAttached); - mWindowless = true; -} - void BrowsingContext::EnsureAttached() { if (!mEverAttached) { Register(this); @@ -1963,7 +1958,8 @@ void BrowsingContext::DidSet(FieldIndex) { bool BrowsingContext::CanSet(FieldIndex, const bool& aUseGlobalHistory, ContentParent* aSource) { - // TODO: Allow access from all + // Should only be set in the parent process. + // return XRE_IsParentProcess() && !aSource; return true; } diff --git a/docshell/base/BrowsingContext.h b/docshell/base/BrowsingContext.h index d26c5396ff6c..6f92c882ee89 100644 --- a/docshell/base/BrowsingContext.h +++ b/docshell/base/BrowsingContext.h @@ -220,7 +220,6 @@ class BrowsingContext : public nsILoadContext, public nsWrapperCache { bool IsDiscarded() const { return mIsDiscarded; } bool Windowless() const { return mWindowless; } - void SetWindowless(); // Get the DocShell for this BrowsingContext if it is in-process, or // null if it's not. @@ -386,6 +385,8 @@ class BrowsingContext : public nsILoadContext, public nsWrapperCache { float FullZoom() const { return GetFullZoom(); } float TextZoom() const { return GetTextZoom(); } + bool UseGlobalHistory() const { return GetUseGlobalHistory(); } + bool IsLoading(); // ScreenOrientation related APIs diff --git a/docshell/base/nsDocShell.cpp b/docshell/base/nsDocShell.cpp index 17620a2ccf69..585ea1d7e7a4 100644 --- a/docshell/base/nsDocShell.cpp +++ b/docshell/base/nsDocShell.cpp @@ -2830,7 +2830,10 @@ nsDocShell::AddChild(nsIDocShellTreeItem* aChild) { /* Set the child's global history if the parent has one */ if (mBrowsingContext->GetUseGlobalHistory()) { - childDocShell->SetUseGlobalHistory(true); + // childDocShell->SetUseGlobalHistory(true); + // this should be set through BC inherit + MOZ_ASSERT(nsDocShell::Cast(childDocShell) + ->mBrowsingContext->GetUseGlobalHistory()); } if (aChild->ItemType() != mItemType) { @@ -3006,23 +3009,6 @@ nsresult nsDocShell::AddChildSHEntryToParent(nsISHEntry* aNewEntry, return rv; } -NS_IMETHODIMP -nsDocShell::SetUseGlobalHistory(bool aUseGlobalHistory) { - mBrowsingContext->SetUseGlobalHistory(aUseGlobalHistory); - if (!aUseGlobalHistory) { - return NS_OK; - } - - nsCOMPtr history = services::GetHistoryService(); - return history ? NS_OK : NS_ERROR_FAILURE; -} - -NS_IMETHODIMP -nsDocShell::GetUseGlobalHistory(bool* aUseGlobalHistory) { - *aUseGlobalHistory = mBrowsingContext->GetUseGlobalHistory(); - return NS_OK; -} - NS_IMETHODIMP nsDocShell::RemoveFromSessionHistory() { nsCOMPtr root; diff --git a/docshell/base/nsIDocShell.idl b/docshell/base/nsIDocShell.idl index dbed1c61ab28..65217ac03737 100644 --- a/docshell/base/nsIDocShell.idl +++ b/docshell/base/nsIDocShell.idl @@ -788,11 +788,6 @@ interface nsIDocShell : nsIDocShellTreeItem in unsigned long aLoadType, in boolean aCloneChilden); - /** - * Whether this docshell should save entries in global history. - */ - attribute boolean useGlobalHistory; - /** * Removes nsISHEntry objects related to this docshell from session history. * Use this only with subdocuments, like iframes. diff --git a/dom/chrome-webidl/BrowsingContext.webidl b/dom/chrome-webidl/BrowsingContext.webidl index 731660a20c1a..82b4e152fbcd 100644 --- a/dom/chrome-webidl/BrowsingContext.webidl +++ b/dom/chrome-webidl/BrowsingContext.webidl @@ -88,6 +88,11 @@ interface BrowsingContext { attribute float textZoom; + /** + * Whether this docshell should save entries in global history. + */ + attribute boolean useGlobalHistory; + // Extension to give chrome JS the ability to set the window screen // orientation while in RDM. void setRDMPaneOrientation(OrientationType type, float rotationAngle); diff --git a/dom/ipc/BrowserChild.cpp b/dom/ipc/BrowserChild.cpp index 9260c1eac816..6274617d4561 100644 --- a/dom/ipc/BrowserChild.cpp +++ b/dom/ipc/BrowserChild.cpp @@ -2285,21 +2285,6 @@ mozilla::ipc::IPCResult BrowserChild::RecvHandleAccessKey( return IPC_OK(); } -mozilla::ipc::IPCResult BrowserChild::RecvSetUseGlobalHistory( - const bool& aUse) { - nsCOMPtr docShell = do_GetInterface(WebNavigation()); - if (!docShell) { - return IPC_OK(); - } - - nsresult rv = docShell->SetUseGlobalHistory(aUse); - if (NS_FAILED(rv)) { - NS_WARNING("Failed to set UseGlobalHistory on BrowserChild docShell"); - } - - return IPC_OK(); -} - mozilla::ipc::IPCResult BrowserChild::RecvPrint(const uint64_t& aOuterWindowID, const PrintData& aPrintData) { #ifdef NS_PRINTING diff --git a/dom/ipc/BrowserChild.h b/dom/ipc/BrowserChild.h index e682b4a8b49e..436b31bb001f 100644 --- a/dom/ipc/BrowserChild.h +++ b/dom/ipc/BrowserChild.h @@ -525,8 +525,6 @@ class BrowserChild final : public nsMessageManagerScriptExecutor, mozilla::ipc::IPCResult RecvHandleAccessKey(const WidgetKeyboardEvent& aEvent, nsTArray&& aCharCodes); - mozilla::ipc::IPCResult RecvSetUseGlobalHistory(const bool& aUse); - mozilla::ipc::IPCResult RecvHandledWindowedPluginKeyEvent( const mozilla::NativeEventData& aKeyEventData, const bool& aIsConsumed); diff --git a/dom/ipc/BrowserParent.cpp b/dom/ipc/BrowserParent.cpp index d8b49679f54e..72a710ca027e 100644 --- a/dom/ipc/BrowserParent.cpp +++ b/dom/ipc/BrowserParent.cpp @@ -501,12 +501,6 @@ void BrowserParent::SetOwnerElement(Element* aElement) { newTopLevelWin->AddBrowser(mBrowserHost); } - if (mFrameElement) { - bool useGlobalHistory = !mFrameElement->HasAttr( - kNameSpaceID_None, nsGkAtoms::disableglobalhistory); - Unused << SendSetUseGlobalHistory(useGlobalHistory); - } - #if defined(XP_WIN) && defined(ACCESSIBILITY) if (!mIsDestroyed) { uintptr_t newWindowHandle = 0; diff --git a/dom/ipc/PBrowser.ipdl b/dom/ipc/PBrowser.ipdl index 2528426f3d37..5e16545c3e1b 100644 --- a/dom/ipc/PBrowser.ipdl +++ b/dom/ipc/PBrowser.ipdl @@ -963,13 +963,6 @@ child: async HandleAccessKey(WidgetKeyboardEvent event, uint32_t[] charCodes); - /** - * Tells the root child docShell whether or not to use - * global history. This is sent right after the PBrowser - * is bound to a frameloader element. - */ - async SetUseGlobalHistory(bool aUse); - /** * HandledWindowedPluginKeyEvent() is always called after posting a native * key event with OnWindowedPluginKeyEvent(). diff --git a/testing/specialpowers/content/SpecialPowersParent.jsm b/testing/specialpowers/content/SpecialPowersParent.jsm index 371bac0c8e53..fd1ac873108b 100644 --- a/testing/specialpowers/content/SpecialPowersParent.jsm +++ b/testing/specialpowers/content/SpecialPowersParent.jsm @@ -95,7 +95,7 @@ async function createWindowlessBrowser({ isPrivate = false } = {}) { const system = Services.scriptSecurityManager.getSystemPrincipal(); chromeShell.createAboutBlankContentViewer(system, system); - chromeShell.useGlobalHistory = false; + windowlessBrowser.browsingContext.useGlobalHistory = false; chromeShell.loadURI("chrome://extensions/content/dummy.xhtml", { triggeringPrincipal: system, }); diff --git a/toolkit/components/browser/nsWebBrowser.cpp b/toolkit/components/browser/nsWebBrowser.cpp index 58dec07a2e41..b6fca23cbc83 100644 --- a/toolkit/components/browser/nsWebBrowser.cpp +++ b/toolkit/components/browser/nsWebBrowser.cpp @@ -98,8 +98,7 @@ nsIWidget* nsWebBrowser::EnsureWidget() { already_AddRefed nsWebBrowser::Create( nsIWebBrowserChrome* aContainerWindow, nsIWidget* aParentWidget, dom::BrowsingContext* aBrowsingContext, - dom::WindowGlobalChild* aInitialWindowChild, - bool aDisableHistory /* = false */) { + dom::WindowGlobalChild* aInitialWindowChild) { MOZ_ASSERT_IF(aInitialWindowChild, aInitialWindowChild->BrowsingContext() == aBrowsingContext); @@ -157,13 +156,6 @@ already_AddRefed nsWebBrowser::Create( docShell->InitSessionHistory(); - if (XRE_IsParentProcess() && !aDisableHistory) { - // Hook up global history. Do not fail if we can't - just warn. - DebugOnly rv = - browser->EnableGlobalHistory(browser->mShouldEnableHistory); - NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), "EnableGlobalHistory() failed"); - } - NS_ENSURE_SUCCESS(docShellAsWin->Create(), nullptr); // Hook into the OnSecurityChange() notification for lock/unlock icon @@ -263,13 +255,6 @@ nsWebBrowser::GetInterface(const nsIID& aIID, void** aSink) { // nsWebBrowser::nsIWebBrowser //***************************************************************************** -NS_IMETHODIMP -nsWebBrowser::EnableGlobalHistory(bool aEnable) { - NS_ENSURE_STATE(mDocShell); - - return mDocShell->SetUseGlobalHistory(aEnable); -} - NS_IMETHODIMP nsWebBrowser::GetContainerWindow(nsIWebBrowserChrome** aTopWindow) { NS_ENSURE_ARG_POINTER(aTopWindow); diff --git a/toolkit/components/browser/nsWebBrowser.h b/toolkit/components/browser/nsWebBrowser.h index c41383028436..189749461ba3 100644 --- a/toolkit/components/browser/nsWebBrowser.h +++ b/toolkit/components/browser/nsWebBrowser.h @@ -109,8 +109,7 @@ class nsWebBrowser final : public nsIWebBrowser, static already_AddRefed Create( nsIWebBrowserChrome* aContainerWindow, nsIWidget* aParentWidget, mozilla::dom::BrowsingContext* aBrowsingContext, - mozilla::dom::WindowGlobalChild* aInitialWindowChild, - bool aDisableHistory = false); + mozilla::dom::WindowGlobalChild* aInitialWindowChild); protected: virtual ~nsWebBrowser(); @@ -119,7 +118,6 @@ class nsWebBrowser final : public nsIWebBrowser, // XXXbz why are these NS_IMETHOD? They're not interface methods! NS_IMETHOD SetDocShell(nsIDocShell* aDocShell); NS_IMETHOD EnsureDocShellTreeOwner(); - NS_IMETHOD EnableGlobalHistory(bool aEnable); nsIWidget* EnsureWidget(); diff --git a/toolkit/components/extensions/ExtensionParent.jsm b/toolkit/components/extensions/ExtensionParent.jsm index 1f4bc1e43c04..1a41901e7294 100644 --- a/toolkit/components/extensions/ExtensionParent.jsm +++ b/toolkit/components/extensions/ExtensionParent.jsm @@ -1187,7 +1187,7 @@ class HiddenXULWindow { chromeShell.setOriginAttributes(attrs); } - chromeShell.useGlobalHistory = false; + windowlessBrowser.browsingContext.useGlobalHistory = false; chromeShell.loadURI("chrome://extensions/content/dummy.xhtml", { triggeringPrincipal: Services.scriptSecurityManager.getSystemPrincipal(), }); diff --git a/toolkit/components/extensions/ExtensionXPCShellUtils.jsm b/toolkit/components/extensions/ExtensionXPCShellUtils.jsm index efb533398884..977365f64574 100644 --- a/toolkit/components/extensions/ExtensionXPCShellUtils.jsm +++ b/toolkit/components/extensions/ExtensionXPCShellUtils.jsm @@ -207,7 +207,7 @@ class ContentPage { ); chromeShell.createAboutBlankContentViewer(system, system); - chromeShell.useGlobalHistory = false; + this.windowlessBrowser.browsingContext.useGlobalHistory = false; let loadURIOptions = { triggeringPrincipal: system, }; diff --git a/toolkit/components/places/tests/chrome/browser_disableglobalhistory.xhtml b/toolkit/components/places/tests/chrome/browser_disableglobalhistory.xhtml index e04d21cdbdf1..047d807cc115 100644 --- a/toolkit/components/places/tests/chrome/browser_disableglobalhistory.xhtml +++ b/toolkit/components/places/tests/chrome/browser_disableglobalhistory.xhtml @@ -21,7 +21,7 @@ let browser = document.getElementById(id); /* eslint-disable-next-line no-shadow */ return ContentTask.spawn(browser, {id, expected}, function({id, expected}) { - Assert.equal(docShell.useGlobalHistory, expected, + Assert.equal(docShell.browsingContext.useGlobalHistory, expected, "Got the right useGlobalHistory state in the docShell of " + id); }); } diff --git a/toolkit/content/widgets/browser-custom-element.js b/toolkit/content/widgets/browser-custom-element.js index 2b88c36562c2..e14f023eafbc 100644 --- a/toolkit/content/widgets/browser-custom-element.js +++ b/toolkit/content/widgets/browser-custom-element.js @@ -1274,7 +1274,7 @@ !this.isRemoteBrowser ) { try { - this.docShell.useGlobalHistory = true; + this.docShell.browsingContext.useGlobalHistory = true; } catch (ex) { // This can occur if the Places database is locked Cu.reportError("Error enabling browser global history: " + ex); diff --git a/toolkit/modules/HiddenFrame.jsm b/toolkit/modules/HiddenFrame.jsm index 66749472c9bc..437289534c15 100644 --- a/toolkit/modules/HiddenFrame.jsm +++ b/toolkit/modules/HiddenFrame.jsm @@ -113,7 +113,8 @@ HiddenFrame.prototype = { let docShell = this._browser.docShell; let systemPrincipal = Services.scriptSecurityManager.getSystemPrincipal(); docShell.createAboutBlankContentViewer(systemPrincipal, systemPrincipal); - docShell.useGlobalHistory = false; + let browsingContext = this._browser.browsingContext; + browsingContext.useGlobalHistory = false; let loadURIOptions = { triggeringPrincipal: systemPrincipal, }; diff --git a/xpfe/appshell/nsAppShellService.cpp b/xpfe/appshell/nsAppShellService.cpp index b4e2a48a2177..c024db0d1f86 100644 --- a/xpfe/appshell/nsAppShellService.cpp +++ b/xpfe/appshell/nsAppShellService.cpp @@ -408,6 +408,15 @@ WindowlessBrowser::Close() { return BrowserDestroyer::Destroy(mBrowser); } +NS_IMETHODIMP +WindowlessBrowser::GetBrowsingContext(BrowsingContext** aBrowsingContext) { + nsCOMPtr docShellTreeItem = do_QueryInterface(mBrowser); + if (!docShellTreeItem) { + return NS_ERROR_NOT_INITIALIZED; + } + return docShellTreeItem->GetBrowsingContextXPCOM(aBrowsingContext); +} + NS_IMETHODIMP WindowlessBrowser::GetDocShell(nsIDocShell** aDocShell) { nsCOMPtr docShell = do_GetInterface(mInterfaceRequestor); @@ -457,8 +466,7 @@ nsAppShellService::CreateWindowlessBrowser(bool aIsChrome, * an associated doc shell, which is what we're interested in. */ nsCOMPtr browser = nsWebBrowser::Create( - stub, widget, browsingContext, nullptr /* initialWindowChild */, - true /* disable history */); + stub, widget, browsingContext, nullptr /* initialWindowChild */); if (NS_WARN_IF(!browser)) { NS_ERROR("Couldn't create instance of nsWebBrowser!"); diff --git a/xpfe/appshell/nsIWindowlessBrowser.idl b/xpfe/appshell/nsIWindowlessBrowser.idl index 7204a9fc5c38..65bee2dbec39 100644 --- a/xpfe/appshell/nsIWindowlessBrowser.idl +++ b/xpfe/appshell/nsIWindowlessBrowser.idl @@ -7,6 +7,7 @@ #include "nsIWebNavigation.idl" interface nsIDocShell; +webidl BrowsingContext; /** * This interface represents a nsIWebBrowser instance with no associated OS @@ -31,5 +32,10 @@ interface nsIWindowlessBrowser : nsIWebNavigation * navigated when the browser's nsIWebNavigation interface is used. */ readonly attribute nsIDocShell docShell; -}; + /** + * Get the Browsing Context for this browser. This is the Browsing Context + * that owns the docshell used for navigation. + */ + readonly attribute BrowsingContext browsingContext; +};