corysanin/tubestation
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Bug 1582832: Part 1 - Make FrameLoader owner rather than DocShell responsible for discarding a BC. r=nika

Browse Source 
There are all sorts of lifecycle issues which arise from making DocShell
responsible for discarding BrowsingContexts. In this particular bug, we tend
to run into them in cases where we create a BrowsingContext for a FrameLoader,
and then never create a DocShell for it, leading to it never being destroyed.
But there are myriad other issues as well.

This patch moves the responsibility for BrowsingContext lifecycle management
to the FrameLoader/FrameLoaderOwner, rather than the DocShell, which makes
things more consistent, and more closely aligns with spec-defined behavior.

Differential Revision: https://phabricator.services.mozilla.com/D59008
This commit is contained in:
Kris Maglione
2020-02-06 19:07:56 +00:00
parent 88c21f2e0a
commit d7e4043bb8
22 changed files with 176 additions and 117 deletions
Download Patch File Download Diff File Expand all files Collapse all files

53
docshell/base/BrowsingContext.cpp
View File

@@ -147,8 +147,10 @@ already_AddRefed<BrowsingContext> BrowsingContext::Create(
RefPtr<BrowsingContext> context;
if (XRE_IsParentProcess()) {
context = new CanonicalBrowsingContext(aParent, group, id,
/* aProcessId */ 0, aType, {});
context =
new CanonicalBrowsingContext(aParent, group, id,
/* aOwnerProcessId */ 0,
/* aEmbedderProcessId */ 0, aType, {});
} else {
context = new BrowsingContext(aParent, group, id, aType, {});
}
@@ -224,9 +226,13 @@ already_AddRefed<BrowsingContext> BrowsingContext::CreateFromIPC(
RefPtr<BrowsingContext> context;
if (XRE_IsParentProcess()) {
context =
new CanonicalBrowsingContext(parent, aGroup, aInit.mId, originId,
Type::Content, std::move(aInit.mFields));
// If the new BrowsingContext has a parent, it is a sub-frame embedded in
// whatever process sent the message. If it doesn't, it is a new window or
// tab, and will be embedded in the parent process.
uint64_t embedderProcessId = parent ? originId : 0;
context = new CanonicalBrowsingContext(parent, aGroup, aInit.mId, originId,
embedderProcessId, Type::Content,
std::move(aInit.mFields));
} else {
context = new BrowsingContext(parent, aGroup, aInit.mId, Type::Content,
std::move(aInit.mFields));
@@ -373,8 +379,6 @@ void BrowsingContext::Detach(bool aFromIPC) {
return;
}
RefPtr<BrowsingContext> self(this);
if (!mGroup->EvictCachedContext(this)) {
Children* children = nullptr;
if (mParent) {
@@ -391,11 +395,31 @@ void BrowsingContext::Detach(bool aFromIPC) {
mChildren.Clear();
}
{
// Hold a strong reference to ourself until the responses comes back to
// ensure we don't die while messages relating to this context are
// in-flight.
RefPtr<BrowsingContext> self(this);
auto callback = [self](auto) {};
if (XRE_IsParentProcess()) {
Group()->EachParent([&](ContentParent* aParent) {
// Only the embedder process is allowed to initiate a BrowsingContext
// detach, so if we've gotten here, the host process already knows we've
// been detached, and there's no need to tell it again.
if (!Canonical()->IsEmbeddedInProcess(aParent->ChildID())) {
aParent->SendDetachBrowsingContext(Id(), callback, callback);
}
});
} else if (!aFromIPC) {
ContentChild::GetSingleton()->SendDetachBrowsingContext(Id(), callback,
callback);
}
}
mGroup->Unregister(this);
mIsDiscarded = true;
nsCOMPtr<nsIObserverService> obs = services::GetObserverService();
if (obs) {
if (nsCOMPtr<nsIObserverService> obs = services::GetObserverService()) {
obs->NotifyObservers(ToSupports(this), "browsing-context-discarded",
nullptr);
}
@@ -414,17 +438,6 @@ void BrowsingContext::Detach(bool aFromIPC) {
if (XRE_IsParentProcess()) {
Canonical()->CanonicalDiscard();
}
if (!aFromIPC && XRE_IsContentProcess()) {
auto cc = ContentChild::GetSingleton();
MOZ_DIAGNOSTIC_ASSERT(cc);
// Tell our parent that the BrowsingContext has been detached. A strong
// reference to this is held until the promise is resolved to ensure it
// doesn't die before the parent receives the message.
auto resolve = [self](bool) {};
auto reject = [self](mozilla::ipc::ResponseRejectReason) {};
cc->SendDetachBrowsingContext(Id(), resolve, reject);
}
}
void BrowsingContext::PrepareForProcessChange() {

28
docshell/base/CanonicalBrowsingContext.cpp
View File

@@ -33,12 +33,14 @@ extern mozilla::LazyLogModule gUserInteractionPRLog;
CanonicalBrowsingContext::CanonicalBrowsingContext(BrowsingContext* aParent,
BrowsingContextGroup* aGroup,
uint64_t aBrowsingContextId,
uint64_t aProcessId,
uint64_t aOwnerProcessId,
uint64_t aEmbedderProcessId,
BrowsingContext::Type aType,
FieldTuple&& aFields)
: BrowsingContext(aParent, aGroup, aBrowsingContextId, aType,
std::move(aFields)),
mProcessId(aProcessId) {
mProcessId(aOwnerProcessId),
mEmbedderProcessId(aEmbedderProcessId) {
// You are only ever allowed to create CanonicalBrowsingContexts in the
// parent process.
MOZ_RELEASE_ASSERT(XRE_IsParentProcess());
@@ -341,28 +343,22 @@ void CanonicalBrowsingContext::PendingRemotenessChange::Complete(
MOZ_DIAGNOSTIC_ASSERT(oldBrowser != embedderBrowser);
MOZ_DIAGNOSTIC_ASSERT(oldBrowser->GetBrowserBridgeParent());
oldBrowser->SendSkipBrowsingContextDetach(
[resetInFlightId](bool aSuccess) { resetInFlightId(); },
[resetInFlightId](mozilla::ipc::ResponseRejectReason aReason) {
resetInFlightId();
});
auto callback = [resetInFlightId](auto) { resetInFlightId(); };
oldBrowser->SendWillChangeProcess(callback, callback);
oldBrowser->Destroy();
}
// Tell the embedder process a remoteness change is in-process. When this is
// acknowledged, reset the in-flight ID if it used to be an in-process load.
embedderWindow->SendMakeFrameRemote(
target, std::move(endpoint), tabId,
[wasRemote, resetInFlightId](bool aSuccess) {
{
auto callback = [wasRemote, resetInFlightId](auto) {
if (!wasRemote) {
resetInFlightId();
}
},
[wasRemote, resetInFlightId](mozilla::ipc::ResponseRejectReason aReason) {
if (!wasRemote) {
resetInFlightId();
};
embedderWindow->SendMakeFrameRemote(target, std::move(endpoint), tabId,
callback, callback);
}
});
// FIXME: We should get the correct principal for the to-be-created window so
// we can avoid creating unnecessary extra windows in the new process.
@@ -468,7 +464,7 @@ CanonicalBrowsingContext::ChangeFrameRemoteness(const nsAString& aRemoteType,
RefPtr<CanonicalBrowsingContext> target(this);
SetInFlightProcessId(OwnerProcessId());
oldBrowser->SendSkipBrowsingContextDetach(
oldBrowser->SendWillChangeProcess(
[target](bool aSuccess) { target->SetInFlightProcessId(0); },
[target](mozilla::ipc::ResponseRejectReason aReason) {
target->SetInFlightProcessId(0);

13
docshell/base/CanonicalBrowsingContext.h
View File

@@ -41,7 +41,11 @@ class CanonicalBrowsingContext final : public BrowsingContext {
bool IsOwnedByProcess(uint64_t aProcessId) const {
return mProcessId == aProcessId;
}
bool IsEmbeddedInProcess(uint64_t aProcessId) const {
return mEmbedderProcessId == aProcessId;
}
uint64_t OwnerProcessId() const { return mProcessId; }
uint64_t EmbedderProcessId() const { return mEmbedderProcessId; }
ContentParent* GetContentParent() const;
void GetCurrentRemoteType(nsAString& aRemoteType, ErrorResult& aRv) const;
@@ -113,8 +117,10 @@ class CanonicalBrowsingContext final : public BrowsingContext {
using Type = BrowsingContext::Type;
CanonicalBrowsingContext(BrowsingContext* aParent,
BrowsingContextGroup* aGroup,
uint64_t aBrowsingContextId, uint64_t aProcessId,
Type aType, FieldTuple&& aFields);
uint64_t aBrowsingContextId,
uint64_t aOwnerProcessId,
uint64_t aEmbedderProcessId, Type aType,
FieldTuple&& aFields);
private:
friend class BrowsingContext;
@@ -149,6 +155,9 @@ class CanonicalBrowsingContext final : public BrowsingContext {
// Indicates which process owns the docshell.
uint64_t mProcessId;
// Indicates which process owns the embedder element.
uint64_t mEmbedderProcessId;
// The ID of the former owner process during an ownership change, which may
// have in-flight messages that assume it is still the owner.
uint64_t mInFlightProcessId = 0;

14
docshell/base/nsDocShell.cpp
View File

@@ -379,7 +379,7 @@ nsDocShell::nsDocShell(BrowsingContext* aBrowsingContext,
mBlankTiming(false),
mTitleValidForCurrentURI(false),
mIsFrame(false),
mSkipBrowsingContextDetachOnDestroy(false),
mWillChangeProcess(false),
mWatchedByDevtools(false),
mIsNavigating(false) {
AssertOriginAttributesMatchPrivateBrowsing();
@@ -4556,7 +4556,7 @@ nsDocShell::Destroy() {
mCurrentURI = nullptr;
if (mScriptGlobal) {
mScriptGlobal->DetachFromDocShell(!mSkipBrowsingContextDetachOnDestroy);
mScriptGlobal->DetachFromDocShell(!mWillChangeProcess);
mScriptGlobal = nullptr;
}
@@ -4568,12 +4568,8 @@ nsDocShell::Destroy() {
mSessionHistory = nullptr;
}
// Either `Detach` our BrowsingContext if this window is closing, or prepare
// the BrowsingContext for the switch to continue.
if (mSkipBrowsingContextDetachOnDestroy) {
if (mWillChangeProcess) {
mBrowsingContext->PrepareForProcessChange();
} else {
mBrowsingContext->Detach();
}
SetTreeOwner(nullptr);
@@ -7263,8 +7259,8 @@ nsresult nsDocShell::RestoreFromHistory() {
// Order the mContentViewer setup just like Embed does.
mContentViewer = nullptr;
if (!mSkipBrowsingContextDetachOnDestroy) {
// Move the browsing ontext's children to the cache. If we're
if (!mWillChangeProcess) {
// Move the browsing context's children to the cache. If we're
// detaching them, we'll detach them from there.
mBrowsingContext->CacheChildren();
}

11
docshell/base/nsDocShell.h
View File

@@ -477,9 +477,7 @@ class nsDocShell final : public nsDocLoader,
// Clear the document's storage access flag if needed.
void MaybeClearStorageAccessFlag();
void SkipBrowsingContextDetach() {
mSkipBrowsingContextDetachOnDestroy = true;
}
void SetWillChangeProcess() { mWillChangeProcess = true; }
// Create a content viewer within this nsDocShell for the given
// `WindowGlobalChild` actor.
@@ -1355,10 +1353,9 @@ class nsDocShell final : public nsDocLoader,
bool mIsFrame : 1;
// If mSkipBrowsingContextDetachOnDestroy is set to true, then when the
// docshell is destroyed, the browsing context will not be detached. This is
// for cases where we want to preserve the BC for future use.
bool mSkipBrowsingContextDetachOnDestroy : 1;
// If mWillChangeProcess is set to true, then when the docshell is destroyed,
// we prepare the browsing context to change process.
bool mWillChangeProcess : 1;
// Set when activity in this docshell is being watched by the developer tools.
bool mWatchedByDevtools : 1;

48
dom/base/nsFrameLoader.cpp
View File

@@ -178,6 +178,7 @@ nsFrameLoader::nsFrameLoader(Element* aOwner, BrowsingContext* aBrowsingContext,
mLoadingOriginalSrc(false),
mRemoteBrowserShown(false),
mIsRemoteFrame(!aRemoteType.IsEmpty()),
mWillChangeProcess(false),
mObservingOwnerContent(false),
mTabProcessCrashFired(false) {}
@@ -1898,6 +1899,10 @@ void nsFrameLoader::DestroyDocShell() {
GetDocShell()->Destroy();
}
if (!mWillChangeProcess) {
mBrowsingContext->Detach();
}
mBrowsingContext = nullptr;
mDocShell = nullptr;
@@ -2620,6 +2625,15 @@ bool nsFrameLoader::TryRemoteBrowserInternal() {
if (!mRemoteBrowser) {
return false;
}
// If we were given a remote tab ID, we may be attaching to an existing remote
// browser, which already has its own BrowsingContext. If so, we need to
// detach our original BC and take ownership of the one from the remote
// browser.
if (mBrowsingContext != mRemoteBrowser->GetBrowsingContext()) {
MOZ_DIAGNOSTIC_ASSERT(nextRemoteTabId);
mBrowsingContext->Detach();
mBrowsingContext = mRemoteBrowser->GetBrowsingContext();
}
mRemoteBrowser->GetBrowsingContext()->Embed();
@@ -2991,6 +3005,7 @@ void nsFrameLoader::InitializeFromBrowserParent(BrowserParent* aBrowserParent) {
MOZ_ASSERT(!mRemoteBrowser);
mIsRemoteFrame = true;
mRemoteBrowser = new BrowserHost(aBrowserParent);
mBrowsingContext = aBrowserParent->GetBrowsingContext();
mChildID = aBrowserParent ? aBrowserParent->Manager()->ChildID() : 0;
MaybeUpdatePrimaryBrowserParent(eBrowserParentChanged);
ReallyLoadFrameScripts();
@@ -3220,13 +3235,14 @@ already_AddRefed<BrowsingContext> nsFrameLoader::GetBrowsingContext() {
}
already_AddRefed<BrowsingContext> nsFrameLoader::GetExtantBrowsingContext() {
RefPtr<BrowsingContext> browsingContext;
if (mRemoteBrowser) {
return do_AddRef(mRemoteBrowser->GetBrowsingContext());
browsingContext = mRemoteBrowser->GetBrowsingContext();
} else if (mDocShell) {
browsingContext = mDocShell->GetBrowsingContext();
}
if (mDocShell) {
return do_AddRef(mDocShell->GetBrowsingContext());
}
return nullptr;
MOZ_ASSERT_IF(browsingContext, browsingContext == mBrowsingContext);
return browsingContext.forget();
}
void nsFrameLoader::InitializeBrowserAPI() {
@@ -3412,11 +3428,12 @@ JSObject* nsFrameLoader::WrapObject(JSContext* cx,
return result;
}
void nsFrameLoader::SkipBrowsingContextDetach() {
void nsFrameLoader::SetWillChangeProcess() {
mWillChangeProcess = true;
if (IsRemoteFrame()) {
// OOP Browser - Go directly over Browser Parent
if (auto* browserParent = GetBrowserParent()) {
RefPtr<BrowsingContext> bc(GetBrowsingContext());
// We're going to be synchronously changing the owner of the
// BrowsingContext in the parent process while the current owner may still
// have in-flight requests which only the owner is allowed to make. Those
@@ -3432,17 +3449,16 @@ void nsFrameLoader::SkipBrowsingContextDetach() {
// resilient. For the moment, though, the surrounding process switch code
// is enough in flux that we're better off with a workable interim
// solution.
bc->Canonical()->SetInFlightProcessId(
browserParent->Manager()->ChildID());
browserParent->SendSkipBrowsingContextDetach(
[bc](bool aSuccess) { bc->Canonical()->SetInFlightProcessId(0); },
[bc](mozilla::ipc::ResponseRejectReason aReason) {
bc->Canonical()->SetInFlightProcessId(0);
});
MOZ_DIAGNOSTIC_ASSERT(mBrowsingContext ==
RefPtr<BrowsingContext>(GetBrowsingContext()));
RefPtr<CanonicalBrowsingContext> bc(mBrowsingContext->Canonical());
bc->SetInFlightProcessId(browserParent->Manager()->ChildID());
auto callback = [bc](auto) { bc->SetInFlightProcessId(0); };
browserParent->SendWillChangeProcess(callback, callback);
}
// OOP IFrame - Through Browser Bridge Parent, set on browser child
else if (auto* browserBridgeChild = GetBrowserBridgeChild()) {
Unused << browserBridgeChild->SendSkipBrowsingContextDetach();
Unused << browserBridgeChild->SendWillChangeProcess();
}
return;
}
@@ -3450,7 +3466,7 @@ void nsFrameLoader::SkipBrowsingContextDetach() {
// In process
RefPtr<nsDocShell> docshell = GetDocShell();
MOZ_ASSERT(docshell);
docshell->SkipBrowsingContextDetach();
docshell->SetWillChangeProcess();
}
void nsFrameLoader::MaybeNotifyCrashed(BrowsingContext* aBrowsingContext,

5
dom/base/nsFrameLoader.h
View File

@@ -396,7 +396,7 @@ class nsFrameLoader final : public nsStubMutationObserver,
virtual JSObject* WrapObject(JSContext* cx,
JS::Handle<JSObject*> aGivenProto) override;
void SkipBrowsingContextDetach();
void SetWillChangeProcess();
void MaybeNotifyCrashed(mozilla::dom::BrowsingContext* aBrowsingContext,
mozilla::ipc::MessageChannel* aChannel);
@@ -527,6 +527,9 @@ class nsFrameLoader final : public nsStubMutationObserver,
bool mRemoteBrowserShown : 1;
bool mIsRemoteFrame : 1;
// If true, the FrameLoader will be re-created with the same BrowsingContext,
// but for a different process, after it is destroyed.
bool mWillChangeProcess : 1;
bool mObservingOwnerContent : 1;
// When an out-of-process nsFrameLoader crashes, an event is fired on the

2
dom/base/nsFrameLoaderOwner.cpp
View File

@@ -102,7 +102,7 @@ void nsFrameLoaderOwner::ChangeRemotenessCommon(
if (mFrameLoader) {
if (aPreserveContext) {
bc = mFrameLoader->GetBrowsingContext();
mFrameLoader->SkipBrowsingContextDetach();
mFrameLoader->SetWillChangeProcess();
}
// Preserve the networkCreated status, as nsDocShells created after a

4
dom/ipc/BrowserBridgeParent.cpp
View File

@@ -189,8 +189,8 @@ IPCResult BrowserBridgeParent::RecvDispatchSynthesizedMouseEvent(
return IPC_OK();
}
IPCResult BrowserBridgeParent::RecvSkipBrowsingContextDetach() {
Unused << mBrowserParent->SendSkipBrowsingContextDetach();
IPCResult BrowserBridgeParent::RecvWillChangeProcess() {
Unused << mBrowserParent->SendWillChangeProcess();
return IPC_OK();
}

2
dom/ipc/BrowserBridgeParent.h
View File

@@ -77,7 +77,7 @@ class BrowserBridgeParent : public PBrowserBridgeParent {
mozilla::ipc::IPCResult RecvDispatchSynthesizedMouseEvent(
const WidgetMouseEvent& aEvent);
mozilla::ipc::IPCResult RecvSkipBrowsingContextDetach();
mozilla::ipc::IPCResult RecvWillChangeProcess();
mozilla::ipc::IPCResult RecvActivate();

6
dom/ipc/BrowserChild.cpp
View File

@@ -1043,11 +1043,11 @@ BrowserChild::~BrowserChild() {
mozilla::DropJSObjects(this);
}
mozilla::ipc::IPCResult BrowserChild::RecvSkipBrowsingContextDetach(
SkipBrowsingContextDetachResolver&& aResolve) {
mozilla::ipc::IPCResult BrowserChild::RecvWillChangeProcess(
WillChangeProcessResolver&& aResolve) {
if (nsCOMPtr<nsIDocShell> docShell = do_GetInterface(WebNavigation())) {
RefPtr<nsDocShell> docshell = nsDocShell::Cast(docShell);
docshell->SkipBrowsingContextDetach();
docshell->SetWillChangeProcess();
}
aResolve(true);
return IPC_OK();

4
dom/ipc/BrowserChild.h
View File

@@ -525,8 +525,8 @@ class BrowserChild final : public nsMessageManagerScriptExecutor,
mozilla::ipc::IPCResult RecvUpdateNativeWindowHandle(
const uintptr_t& aNewHandle);
mozilla::ipc::IPCResult RecvSkipBrowsingContextDetach(
SkipBrowsingContextDetachResolver&& aResolve);
mozilla::ipc::IPCResult RecvWillChangeProcess(
WillChangeProcessResolver&& aResolve);
/**
* Native widget remoting protocol for use with windowed plugins with e10s.
*/

34
dom/ipc/ContentParent.cpp
View File

@@ -6150,6 +6150,17 @@ bool ContentParent::CheckBrowsingContextOwnership(
return true;
}
bool ContentParent::CheckBrowsingContextEmbedder(BrowsingContext* aBC,
const char* aOperation) const {
if (!aBC->Canonical()->IsEmbeddedInProcess(ChildID())) {
MOZ_LOG(BrowsingContext::GetLog(), LogLevel::Warning,
("ParentIPC: Trying to %s out of process context 0x%08" PRIx64,
aOperation, aBC->Id()));
return false;
}
return true;
}
mozilla::ipc::IPCResult ContentParent::RecvDetachBrowsingContext(
uint64_t aContextId, DetachBrowsingContextResolver&& aResolve) {
// NOTE: Immediately resolve the promise, as we've received the message. This
@@ -6165,24 +6176,12 @@ mozilla::ipc::IPCResult ContentParent::RecvDetachBrowsingContext(
return IPC_OK();
}
if (!CheckBrowsingContextOwnership(context, "detach")) {
// We're trying to detach a child BrowsingContext in another child
// process. This is illegal since the owner of the BrowsingContext
// is the proccess with the in-process docshell, which is tracked
// by OwnerProcessId.
return IPC_OK();
if (!CheckBrowsingContextEmbedder(context, "detach")) {
return IPC_FAIL(this, "Illegal Detach() attempt");
}
context->Detach(/* aFromIPC */ true);
context->Group()->EachOtherParent(this, [&](ContentParent* aParent) {
// Hold a reference to `context` until the response comes back to ensure it
// doesn't die while messages relating to this context are in-flight.
auto resolve = [context](bool) {};
auto reject = [context](ResponseRejectReason) {};
aParent->SendDetachBrowsingContext(context->Id(), resolve, reject);
});
return IPC_OK();
}
@@ -6320,6 +6319,13 @@ mozilla::ipc::IPCResult ContentParent::RecvWindowPostMessage(
return IPC_OK();
}
if (aData.source() && aData.source()->IsDiscarded()) {
MOZ_LOG(
BrowsingContext::GetLog(), LogLevel::Debug,
("ParentIPC: Trying to send a message from dead or detached context"));
return IPC_OK();
}
RefPtr<ContentParent> cp = aContext->Canonical()->GetContentParent();
if (!cp) {
MOZ_LOG(BrowsingContext::GetLog(), LogLevel::Debug,

2
dom/ipc/ContentParent.h
View File

@@ -673,6 +673,8 @@ class ContentParent final
protected:
bool CheckBrowsingContextOwnership(BrowsingContext* aBC,
const char* aOperation) const;
bool CheckBrowsingContextEmbedder(BrowsingContext* aBC,
const char* aOperation) const;
void OnChannelConnected(int32_t pid) override;

2
dom/ipc/PBrowser.ipdl
View File

@@ -1016,7 +1016,7 @@ child:
*/
async SetWidgetNativeData(WindowsHandle aHandle);
async SkipBrowsingContextDetach() returns (bool success);
async WillChangeProcess() returns (bool success);
parent:
/**

2
dom/ipc/PBrowserBridge.ipdl
View File

@@ -103,7 +103,7 @@ parent:
async SetIsUnderHiddenEmbedderElement(bool aIsUnderHiddenEmbedderElement);
async SkipBrowsingContextDetach();
async WillChangeProcess();
#ifdef ACCESSIBILITY
/**

2
dom/ipc/WindowGlobalParent.cpp
View File

@@ -109,7 +109,9 @@ void WindowGlobalParent::Init(const WindowGlobalInit& aInit) {
// If there is no current window global, assume we're about to become it
// optimistically.
if (!mBrowsingContext->IsDiscarded()) {
mBrowsingContext->SetCurrentInnerWindowId(aInit.innerWindowId());
}
nsCOMPtr<nsIObserverService> obs = services::GetObserverService();
if (obs) {

8
dom/ipc/tests/test_window_open_discarded_bc.html
View File

@@ -22,7 +22,13 @@ add_task(async function() {
// BrowsingContext to actually become discarded after we call it, to
// make sure that the window provider actually has a discarded BC at the
// end of its own nested event loop.
SpecialPowers.Services.tm.spinEventLoopUntil(() => !win.opener);
SpecialPowers.Services.tm.spinEventLoopUntil(() => {
try {
return !win.opener;
} catch (e) {
return false;
}
});
}
}
SpecialPowers.addObserver(observer, TOPIC);

3
dom/media/test/file_access_controls.html
View File

@@ -2,7 +2,7 @@
<head>
<script type="text/javascript" src="manifest.js"></script>
</head>
<body onload="setTimeout(load, 0);" onunload="done()">
<body onload="setTimeout(load, 0);">
<script>
// Page URL: http://example.org/tests/dom/media/test/file_access_controls.html
@@ -115,6 +115,7 @@ function nextTest() {
} else {
//dump("Exiting...\n");
// We're done, exit the test.
done();
window.close();
return;
}

7
layout/printing/nsPrintObject.cpp
View File

@@ -49,10 +49,9 @@ nsPrintObject::~nsPrintObject() {
DestroyPresentation();
if (mDidCreateDocShell && mDocShell) {
nsCOMPtr<nsIBaseWindow> baseWin(do_QueryInterface(mDocShell));
if (baseWin) {
baseWin->Destroy();
}
RefPtr<BrowsingContext> bc(mDocShell->GetBrowsingContext());
nsDocShell::Cast(mDocShell)->Destroy();
bc->Detach();
}
mDocShell = nullptr;
mTreeOwner = nullptr; // mTreeOwner must be released after mDocShell;

2
xpfe/appshell/AppWindow.cpp
View File

@@ -692,8 +692,10 @@ NS_IMETHODIMP AppWindow::Destroy() {
mDOMWindow = nullptr;
if (mDocShell) {
RefPtr<BrowsingContext> bc(mDocShell->GetBrowsingContext());
nsCOMPtr<nsIBaseWindow> shellAsWin(do_QueryInterface(mDocShell));
shellAsWin->Destroy();
bc->Detach();
mDocShell = nullptr; // this can cause reentrancy of this function
}

31
xpfe/appshell/nsAppShellService.cpp
View File

@@ -321,11 +321,25 @@ class BrowserDestroyer final : public Runnable {
mBrowser(aBrowser),
mContainer(aContainer) {}
static nsresult Destroy(nsIWebBrowser* aBrowser) {
RefPtr<BrowsingContext> bc;
if (nsCOMPtr<nsIDocShell> docShell = do_GetInterface(aBrowser)) {
bc = docShell->GetBrowsingContext();
}
nsCOMPtr<nsIBaseWindow> window(do_QueryInterface(aBrowser));
nsresult rv = window->Destroy();
MOZ_ASSERT(bc);
if (bc) {
bc->Detach();
}
return rv;
}
NS_IMETHOD
Run() override {
// Explicitly destroy the browser, in case this isn't the last reference.
nsCOMPtr<nsIBaseWindow> window = do_QueryInterface(mBrowser);
return window->Destroy();
return Destroy(mBrowser);
}
protected:
@@ -354,8 +368,8 @@ class WindowlessBrowser final : public nsIWindowlessBrowser,
NS_FORWARD_SAFE_NSIWEBNAVIGATION(mWebNavigation)
NS_FORWARD_SAFE_NSIINTERFACEREQUESTOR(mInterfaceRequestor)
protected:
virtual ~WindowlessBrowser() {
private:
~WindowlessBrowser() {
if (mClosed) {
return;
}
@@ -366,11 +380,10 @@ class WindowlessBrowser final : public nsIWindowlessBrowser,
// when it's safe to run scripts. If this was triggered by GC, it may
// not always be safe to run scripts, in which cases we need to delay
// destruction until it is.
nsCOMPtr<nsIRunnable> runnable = new BrowserDestroyer(mBrowser, mContainer);
nsContentUtils::AddScriptRunner(runnable);
auto runnable = MakeRefPtr<BrowserDestroyer>(mBrowser, mContainer);
nsContentUtils::AddScriptRunner(runnable.forget());
}
private:
nsCOMPtr<nsIWebBrowser> mBrowser;
nsCOMPtr<nsIWebNavigation> mWebNavigation;
nsCOMPtr<nsIInterfaceRequestor> mInterfaceRequestor;
@@ -393,9 +406,7 @@ WindowlessBrowser::Close() {
mWebNavigation = nullptr;
mInterfaceRequestor = nullptr;
nsCOMPtr<nsIBaseWindow> window = do_QueryInterface(mBrowser);
return window->Destroy();
return BrowserDestroyer::Destroy(mBrowser);
}
NS_IMETHODIMP