| ☠☠ backed out by 78aaa908b43f ☠ ☠ | |
| author | Kris Maglione <maglione.k@gmail.com> |
| Fri, 07 May 2021 01:25:32 +0000 | |
| changeset 578820 | 761fdc9f84d2a79d3da070d8745d8d37821d325f |
| parent 578819 | 640d8a87b172e06d4753e5e535900610d475d7c6 |
| child 578821 | 4a04508cbc613720a945bdaaa1b5cf1723c70eaf |
| push id | 142731 |
| push user | maglione.k@gmail.com |
| push date | Fri, 07 May 2021 01:27:59 +0000 |
| treeherder | autoland@761fdc9f84d2 [default view] [failures only] |
| perfherder | [talos] [build metrics] [platform microbench] (compared to previous push) |
| reviewers | nika |
| bugs | 1702678 |
| milestone | 90.0a1 |
| first release with | nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
|
| last release without | nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
|
--- a/docshell/test/chrome/chrome.ini +++ b/docshell/test/chrome/chrome.ini @@ -79,12 +79,17 @@ skip-if = true # Bug 1026815,Bug 1546159 [test_bug608669.xhtml] [test_bug662200.xhtml] [test_bug690056.xhtml] [test_bug789773.xhtml] [test_bug846906.xhtml] [test_bug89419.xhtml] [test_bug909218.html] [test_bug92598.xhtml] +[test_open_and_immediately_close_opener.html] +# This bug only manifests in the non-e10s window open codepath. The test +# should be updated to make sure it still opens a new window in the parent +# process if and when we start running chrome mochitests with e10s enabled. +skip-if = e10s [test_mozFrameType.xhtml] [test_viewsource_forbidden_in_iframe.xhtml] skip-if = true # bug 1019315 [test_docRedirect.xhtml]
new file mode 100644 --- /dev/null +++ b/docshell/test/chrome/test_open_and_immediately_close_opener.html @@ -0,0 +1,54 @@ +<!DOCTYPE html> +<html> +<head> + <title>Test for Bug 1702678</title> + <meta charset="utf-8"> + <script src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script> + <link rel="stylesheet" type="text/css" href="chrome://mochikit/content/tests/SimpleTest/test.css"/> +</head> +<body> +<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1702678">Mozilla Bug 1702678</a> + +<script type="application/javascript"> +"use strict"; + +const HTML = ` +<!DOCTYPE html> +<html> +<head> + <meta charset="utf-8"> + <script> + // We need to queue a promise reaction job whilch will close the window + // during the nested event loop spun up by the window opening code. + Promise.resolve().then(() => { + window.close(); + }); + window.open("data:text/html,Hello"); + <\/script> +</head> +</html> +`; + +add_task(async function() { + // This bug only manifests when opening tabs in new windows. + await SpecialPowers.pushPrefEnv({ + set: [["browser.link.open_newwindow", 2]], + }); + + // Create a window in a new BrowsingContextGroup so that it will be the last + // window in the group when it closes, and the group will be destroyed. + window.open(`data:text/html,${encodeURIComponent(HTML)}`, "", "noopener"); + + // Make a few trips through the event loop to ensure we've had a chance to + // open and close the relevant windows. + for (let i = 0; i < 10; i++) { + await new Promise(resolve => setTimeout(resolve, 0)); + } + + ok(true, "We didn't crash"); +}); +</script> + +</body> +</html> +
--- a/dom/base/nsFrameLoader.cpp +++ b/dom/base/nsFrameLoader.cpp @@ -283,18 +283,33 @@ static already_AddRefed<BrowsingContext> MOZ_ASSERT(XRE_IsParentProcess()); return do_AddRef( aOpenWindowInfo->GetNextRemoteBrowser()->GetBrowsingContext()); } RefPtr<BrowsingContext> opener; if (aOpenWindowInfo && !aOpenWindowInfo->GetForceNoOpener()) { opener = aOpenWindowInfo->GetParent(); - // Must create BrowsingContext with opener in-process. - MOZ_ASSERT_IF(opener, opener->IsInProcess()); + if (opener) { + // Must create BrowsingContext with opener in-process. + MOZ_ASSERT(opener->IsInProcess()); + + // This can only happen when the opener was closed from a nested event + // loop in the window provider code, and only when the open was triggered + // by a non-e10s tab, and the new tab is being opened in a new browser + // window. Since it is a corner case among corner cases, and the opener + // window will appear to be null to consumers after it is discarded + // anyway, just drop the opener entirely. + if (opener->IsDiscarded()) { + NS_WARNING( + "Opener was closed from a nested event loop in the parent process. " + "Please fix this."); + opener = nullptr; + } + } } RefPtr<nsGlobalWindowInner> parentInner = nsGlobalWindowInner::Cast(aOwner->OwnerDoc()->GetInnerWindow()); if (NS_WARN_IF(!parentInner) || parentInner->IsDying()) { return nullptr; }
--- a/toolkit/components/windowwatcher/nsWindowWatcher.cpp +++ b/toolkit/components/windowwatcher/nsWindowWatcher.cpp @@ -981,18 +981,27 @@ nsresult nsWindowWatcher::OpenWindowInte MOZ_ALWAYS_SUCCEEDS(newBC->SetOnePermittedSandboxedNavigator(parentBC)); } if (!aForceNoOpener && parentBC) { // If we've created a new content window, its opener should have been set // when its BrowsingContext was created, in order to ensure that the context // is loaded within the correct BrowsingContextGroup. if (windowIsNew && newBC->IsContent()) { - MOZ_RELEASE_ASSERT(newBC->GetOpenerId() == parentBC->Id()); - MOZ_RELEASE_ASSERT(!!parentBC == newBC->HadOriginalOpener()); + if (parentBC->IsDiscarded()) { + // If the parent BC was discarded in a nested event loop before we got + // to this point, we can't set it as the opener. Ideally we would still + // set `HadOriginalOpener()` in that case, but that's somewhat + // nontrivial, and not worth the effort given the nature of the corner + // case (see comment in `nsFrameLoader::CreateBrowsingContext`. + MOZ_RELEASE_ASSERT(newBC->GetOpenerId() == 0); + } else { + MOZ_RELEASE_ASSERT(newBC->GetOpenerId() == parentBC->Id()); + MOZ_RELEASE_ASSERT(newBC->HadOriginalOpener()); + } } else { // Update the opener for an existing or chrome BC. newBC->SetOpener(parentBC); } } RefPtr<nsDocShell> newDocShell(nsDocShell::Cast(newBC->GetDocShell()));