Skip to content

Commit

Permalink
Bug 1624550 - P4: Cleanup APIs for setting BrowsingContext::UseGlobal…
Browse files Browse the repository at this point in the history
…History. 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
  • Loading branch information
djg committed May 8, 2020
1 parent 260bf24 commit dd7f585
Show file tree
Hide file tree
Showing 19 changed files with 39 additions and 88 deletions.
8 changes: 2 additions & 6 deletions docshell/base/BrowsingContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -323,11 +323,6 @@ already_AddRefed<BrowsingContext> BrowsingContext::CreateIndependent(
return bc.forget();
}

void BrowsingContext::SetWindowless() {
MOZ_DIAGNOSTIC_ASSERT(!mEverAttached);
mWindowless = true;
}

void BrowsingContext::EnsureAttached() {
if (!mEverAttached) {
Register(this);
Expand Down Expand Up @@ -1963,7 +1958,8 @@ void BrowsingContext::DidSet(FieldIndex<IDX_DefaultLoadFlags>) {
bool BrowsingContext::CanSet(FieldIndex<IDX_UseGlobalHistory>,
const bool& aUseGlobalHistory,
ContentParent* aSource) {
// TODO: Allow access from all
// Should only be set in the parent process.
// return XRE_IsParentProcess() && !aSource;
return true;
}

Expand Down
3 changes: 2 additions & 1 deletion docshell/base/BrowsingContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down
22 changes: 4 additions & 18 deletions docshell/base/nsDocShell.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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<IHistory> 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<nsIDocShellTreeItem> root;
Expand Down
5 changes: 0 additions & 5 deletions docshell/base/nsIDocShell.idl
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
5 changes: 5 additions & 0 deletions dom/chrome-webidl/BrowsingContext.webidl
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
15 changes: 0 additions & 15 deletions dom/ipc/BrowserChild.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2285,21 +2285,6 @@ mozilla::ipc::IPCResult BrowserChild::RecvHandleAccessKey(
return IPC_OK();
}

mozilla::ipc::IPCResult BrowserChild::RecvSetUseGlobalHistory(
const bool& aUse) {
nsCOMPtr<nsIDocShell> 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
Expand Down
2 changes: 0 additions & 2 deletions dom/ipc/BrowserChild.h
Original file line number Diff line number Diff line change
Expand Up @@ -525,8 +525,6 @@ class BrowserChild final : public nsMessageManagerScriptExecutor,
mozilla::ipc::IPCResult RecvHandleAccessKey(const WidgetKeyboardEvent& aEvent,
nsTArray<uint32_t>&& aCharCodes);

mozilla::ipc::IPCResult RecvSetUseGlobalHistory(const bool& aUse);

mozilla::ipc::IPCResult RecvHandledWindowedPluginKeyEvent(
const mozilla::NativeEventData& aKeyEventData, const bool& aIsConsumed);

Expand Down
6 changes: 0 additions & 6 deletions dom/ipc/BrowserParent.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
7 changes: 0 additions & 7 deletions dom/ipc/PBrowser.ipdl
Original file line number Diff line number Diff line change
Expand Up @@ -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().
Expand Down
2 changes: 1 addition & 1 deletion testing/specialpowers/content/SpecialPowersParent.jsm
Original file line number Diff line number Diff line change
Expand Up @@ -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,
});
Expand Down
17 changes: 1 addition & 16 deletions toolkit/components/browser/nsWebBrowser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,7 @@ nsIWidget* nsWebBrowser::EnsureWidget() {
already_AddRefed<nsWebBrowser> 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);

Expand Down Expand Up @@ -157,13 +156,6 @@ already_AddRefed<nsWebBrowser> nsWebBrowser::Create(

docShell->InitSessionHistory();

if (XRE_IsParentProcess() && !aDisableHistory) {
// Hook up global history. Do not fail if we can't - just warn.
DebugOnly<nsresult> 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
Expand Down Expand Up @@ -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);
Expand Down
4 changes: 1 addition & 3 deletions toolkit/components/browser/nsWebBrowser.h
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,7 @@ class nsWebBrowser final : public nsIWebBrowser,
static already_AddRefed<nsWebBrowser> Create(
nsIWebBrowserChrome* aContainerWindow, nsIWidget* aParentWidget,
mozilla::dom::BrowsingContext* aBrowsingContext,
mozilla::dom::WindowGlobalChild* aInitialWindowChild,
bool aDisableHistory = false);
mozilla::dom::WindowGlobalChild* aInitialWindowChild);

protected:
virtual ~nsWebBrowser();
Expand All @@ -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();

Expand Down
2 changes: 1 addition & 1 deletion toolkit/components/extensions/ExtensionParent.jsm
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
});
Expand Down
2 changes: 1 addition & 1 deletion toolkit/components/extensions/ExtensionXPCShellUtils.jsm
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ class ContentPage {
);

chromeShell.createAboutBlankContentViewer(system, system);
chromeShell.useGlobalHistory = false;
this.windowlessBrowser.browsingContext.useGlobalHistory = false;
let loadURIOptions = {
triggeringPrincipal: system,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
}
Expand Down
2 changes: 1 addition & 1 deletion toolkit/content/widgets/browser-custom-element.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
3 changes: 2 additions & 1 deletion toolkit/modules/HiddenFrame.jsm
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down
12 changes: 10 additions & 2 deletions xpfe/appshell/nsAppShellService.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -408,6 +408,15 @@ WindowlessBrowser::Close() {
return BrowserDestroyer::Destroy(mBrowser);
}

NS_IMETHODIMP
WindowlessBrowser::GetBrowsingContext(BrowsingContext** aBrowsingContext) {
nsCOMPtr<nsIDocShellTreeItem> docShellTreeItem = do_QueryInterface(mBrowser);
if (!docShellTreeItem) {
return NS_ERROR_NOT_INITIALIZED;
}
return docShellTreeItem->GetBrowsingContextXPCOM(aBrowsingContext);
}

NS_IMETHODIMP
WindowlessBrowser::GetDocShell(nsIDocShell** aDocShell) {
nsCOMPtr<nsIDocShell> docShell = do_GetInterface(mInterfaceRequestor);
Expand Down Expand Up @@ -457,8 +466,7 @@ nsAppShellService::CreateWindowlessBrowser(bool aIsChrome,
* an associated doc shell, which is what we're interested in.
*/
nsCOMPtr<nsIWebBrowser> 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!");
Expand Down
8 changes: 7 additions & 1 deletion xpfe/appshell/nsIWindowlessBrowser.idl
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include "nsIWebNavigation.idl"

interface nsIDocShell;
webidl BrowsingContext;

/**
* This interface represents a nsIWebBrowser instance with no associated OS
Expand All @@ -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;
};

0 comments on commit dd7f585

Please sign in to comment.