From 3e35e21538d228d354b0307f43df2bfd1479b5cf Mon Sep 17 00:00:00 2001 From: Dorel Luca Date: Tue, 9 Jun 2020 03:39:22 +0300 Subject: [PATCH] Backed out 2 changesets (bug 1580766) for XPCshell failures in unit/test_browsing_context_structured_clone.js. CLOSED TREE Backed out changeset 6b9926a5ab88 (bug 1580766) Backed out changeset 3722901b6309 (bug 1580766) --- .../test/general/browser_tab_dragdrop.js | 78 +++---------------- docshell/base/BrowsingContext.cpp | 47 ++--------- docshell/base/BrowsingContext.h | 14 +--- .../browser/browser_browsingContext-02.js | 52 +------------ dom/base/nsContentUtils.cpp | 8 -- dom/base/nsContentUtils.h | 5 -- dom/base/nsFrameLoader.cpp | 45 +---------- dom/base/nsFrameLoaderOwner.h | 5 -- dom/chrome-webidl/BrowsingContext.webidl | 12 +-- dom/chrome-webidl/XULFrameElement.webidl | 4 +- dom/ipc/ContentChild.cpp | 3 +- dom/xul/XULFrameElement.h | 1 - 12 files changed, 25 insertions(+), 249 deletions(-) diff --git a/browser/base/content/test/general/browser_tab_dragdrop.js b/browser/base/content/test/general/browser_tab_dragdrop.js index a26a8f355cd5..e662c0db9977 100644 --- a/browser/base/content/test/general/browser_tab_dragdrop.js +++ b/browser/base/content/test/general/browser_tab_dragdrop.js @@ -1,35 +1,7 @@ -// Swaps the content of tab a into tab b and then closes tab a. function swapTabsAndCloseOther(a, b) { gBrowser.swapBrowsersAndCloseOther(gBrowser.tabs[b], gBrowser.tabs[a]); } -// Mirrors the effect of the above function on an array. -function swapArrayContentsAndRemoveOther(arr, a, b) { - arr[b] = arr[a]; - arr.splice(a, 1); -} - -function checkBrowserIds(expected) { - is( - gBrowser.tabs.length, - expected.length, - "Should have the right number of tabs." - ); - - for (let [i, tab] of gBrowser.tabs.entries()) { - is( - tab.linkedBrowser.browserId, - expected[i], - `Tab ${i} should have the right browser ID.` - ); - is( - tab.linkedBrowser.browserId, - tab.linkedBrowser.browsingContext.browserId, - `Browser for tab ${i} has the same browserId as its BrowsingContext` - ); - } -} - var getClicks = function(tab) { return SpecialPowers.spawn(tab.linkedBrowser, [], function() { return content.wrappedJSObject.clicks; @@ -140,40 +112,23 @@ add_task(async function() { ); await BrowserTestUtils.switchTab(gBrowser, tabs[3]); - let browserIds = tabs.map(t => t.linkedBrowser.browserId); - checkBrowserIds(browserIds); - - is(gBrowser.tabs[1], tabs[1], "tab1"); - is(gBrowser.tabs[2], tabs[2], "tab2"); - is(gBrowser.tabs[3], tabs[3], "tab3"); - is(gBrowser.tabs[4], tabs[4], "tab4"); - swapTabsAndCloseOther(2, 3); // now: 0 1 2 4 - // Tab 2 is gone (what was tab 3 is displaying its content). - tabs.splice(2, 1); - swapArrayContentsAndRemoveOther(browserIds, 2, 3); - is(gBrowser.tabs[1], tabs[1], "tab1"); - is(gBrowser.tabs[2], tabs[2], "tab2"); - is(gBrowser.tabs[3], tabs[3], "tab4"); - - checkBrowserIds(browserIds); + is(gBrowser.tabs[2], tabs[3], "tab3"); + is(gBrowser.tabs[3], tabs[4], "tab4"); + delete tabs[2]; info("about to cacheObjectValue"); - await cacheObjectValue(tabs[3].linkedBrowser); + await cacheObjectValue(tabs[4].linkedBrowser); info("just finished cacheObjectValue"); swapTabsAndCloseOther(3, 2); // now: 0 1 4 - tabs.splice(3, 1); - swapArrayContentsAndRemoveOther(browserIds, 3, 2); - is( Array.prototype.indexOf.call(gBrowser.tabs, gBrowser.selectedTab), 2, "The third tab should be selected" ); - - checkBrowserIds(browserIds); + delete tabs[4]; ok( await checkObjectValue(gBrowser.tabs[2].linkedBrowser), @@ -181,19 +136,15 @@ add_task(async function() { ); is(gBrowser.tabs[1], tabs[1], "tab1"); - is(gBrowser.tabs[2], tabs[2], "tab4"); + is(gBrowser.tabs[2], tabs[3], "tab4"); let clicks = await getClicks(gBrowser.tabs[2]); is(clicks, 0, "no click on BODY so far"); await clickTest(gBrowser.tabs[2]); swapTabsAndCloseOther(2, 1); // now: 0 4 - tabs.splice(2, 1); - swapArrayContentsAndRemoveOther(browserIds, 2, 1); - - is(gBrowser.tabs[1], tabs[1], "tab4"); - - checkBrowserIds(browserIds); + is(gBrowser.tabs[1], tabs[1], "tab1"); + delete tabs[3]; ok( await checkObjectValue(gBrowser.tabs[1].linkedBrowser), @@ -224,14 +175,9 @@ add_task(async function() { await loadURI(tabs[1], "about:blank"); let key = tabs[1].linkedBrowser.permanentKey; - checkBrowserIds(browserIds); - let win = gBrowser.replaceTabWithWindow(tabs[1]); await new Promise(resolve => whenDelayedStartupFinished(win, resolve)); - - let newWinBrowserId = browserIds[1]; - browserIds.splice(1, 1); - checkBrowserIds(browserIds); + delete tabs[1]; // Verify that the original window now only has the initial tab left in it. is(gBrowser.tabs[0], tabs[0], "tab0"); @@ -239,12 +185,6 @@ add_task(async function() { let tab = win.gBrowser.tabs[0]; is(tab.linkedBrowser.permanentKey, key, "Should have kept the key"); - is(tab.linkedBrowser.browserId, newWinBrowserId, "Should have kept the ID"); - is( - tab.linkedBrowser.browserId, - tab.linkedBrowser.browsingContext.browserId, - "Should have kept the ID" - ); let awaitPageShow = BrowserTestUtils.waitForContentEvent( tab.linkedBrowser, diff --git a/docshell/base/BrowsingContext.cpp b/docshell/base/BrowsingContext.cpp index 7fa63dde0bc0..93817166c3b5 100644 --- a/docshell/base/BrowsingContext.cpp +++ b/docshell/base/BrowsingContext.cpp @@ -29,7 +29,6 @@ #include "mozilla/dom/WindowGlobalParent.h" #include "mozilla/dom/WindowProxyHolder.h" #include "mozilla/dom/SyncedContextInlines.h" -#include "mozilla/dom/XULFrameElement.h" #include "mozilla/net/DocumentLoadListener.h" #include "mozilla/net/RequestContextService.h" #include "mozilla/Assertions.h" @@ -50,7 +49,6 @@ #include "nsGlobalWindowOuter.h" #include "nsIObserverService.h" #include "nsContentUtils.h" -#include "nsQueryObject.h" #include "nsSandboxFlags.h" #include "nsScriptError.h" #include "nsThreadUtils.h" @@ -188,14 +186,10 @@ bool BrowsingContext::SameOriginWithTop() { /* static */ already_AddRefed BrowsingContext::CreateDetached( nsGlobalWindowInner* aParent, BrowsingContext* aOpener, - const nsAString& aName, Type aType, uint64_t aBrowserId) { - if (aParent) { - MOZ_DIAGNOSTIC_ASSERT(aParent->GetBrowsingContext()->mType == aType); - MOZ_DIAGNOSTIC_ASSERT(aParent->GetWindowContext()); - MOZ_DIAGNOSTIC_ASSERT(aParent->GetBrowsingContext()->GetBrowserId() == 0 || - aParent->GetBrowsingContext()->GetBrowserId() == - aBrowserId); - } + const nsAString& aName, Type aType) { + MOZ_DIAGNOSTIC_ASSERT(!aParent || + aParent->GetBrowsingContext()->mType == aType); + MOZ_DIAGNOSTIC_ASSERT(!aParent || aParent->GetWindowContext()); MOZ_DIAGNOSTIC_ASSERT(aType != Type::Chrome || XRE_IsParentProcess()); @@ -237,7 +231,6 @@ already_AddRefed BrowsingContext::CreateDetached( context->mFields.SetWithoutSyncing(aOpener->Id()); context->mFields.SetWithoutSyncing(true); } - if (aParent) { MOZ_DIAGNOSTIC_ASSERT(parentBC->Group() == context->Group()); MOZ_DIAGNOSTIC_ASSERT(parentBC->mType == context->mType); @@ -250,8 +243,6 @@ already_AddRefed BrowsingContext::CreateDetached( context->mFields.SetWithoutSyncing( nsILoadInfo::OPENER_POLICY_UNSAFE_NONE); - context->mFields.SetWithoutSyncing(aBrowserId); - if (aOpener && aOpener->SameOriginWithTop()) { // We inherit the opener policy if there is a creator and if the creator's // origin is same origin with the creator's top-level origin. @@ -332,7 +323,7 @@ already_AddRefed BrowsingContext::CreateDetached( already_AddRefed BrowsingContext::CreateIndependent( Type aType) { RefPtr bc( - CreateDetached(nullptr, nullptr, EmptyString(), aType, 0)); + CreateDetached(nullptr, nullptr, EmptyString(), aType)); bc->mWindowless = bc->IsContent(); bc->EnsureAttached(); return bc.forget(); @@ -487,28 +478,6 @@ void BrowsingContext::SetEmbedderElement(Element* aEmbedder) { if (aEmbedder) { Transaction txn; txn.SetEmbedderElementType(Some(aEmbedder->LocalName())); - - // We don't care about browser Ids for chrome-type BrowsingContexts. - if (RefPtr owner = do_QueryObject(aEmbedder); - !IsChrome() && owner) { - uint64_t browserId = GetBrowserId(); - uint64_t frameBrowserId = owner->GetBrowserId(); - MOZ_DIAGNOSTIC_ASSERT(browserId != 0); - - if (frameBrowserId == 0) { - // We'll arrive here if we're a top-level BrowsingContext for a window - // or tab that was opened in a content process. There should be no - // children to update at this point. This Id was generated in - // ContentChild::ProvideWindowCommon. - MOZ_DIAGNOSTIC_ASSERT(IsTopContent()); - MOZ_DIAGNOSTIC_ASSERT(Children().IsEmpty()); - owner->SetBrowserId(browserId); - } else { - // We would've inherited or generated an Id in CreateBrowsingContext. - MOZ_DIAGNOSTIC_ASSERT(browserId == frameBrowserId); - } - } - if (nsCOMPtr inner = do_QueryInterface(aEmbedder->GetOwnerGlobal())) { txn.SetEmbedderInnerWindowId(inner->WindowID()); @@ -2398,12 +2367,6 @@ void BrowsingContext::DidSet(FieldIndex, CreateChildSHistory(); } -bool BrowsingContext::CanSet(FieldIndex, const uint32_t& aValue, - ContentParent* aSource) { - // Should only be able to set if the ID is not already set. - return GetBrowserId() == 0 && IsTop() && Children().IsEmpty(); -} - } // namespace dom namespace ipc { diff --git a/docshell/base/BrowsingContext.h b/docshell/base/BrowsingContext.h index 06391592e492..32c4494dc7fa 100644 --- a/docshell/base/BrowsingContext.h +++ b/docshell/base/BrowsingContext.h @@ -107,13 +107,6 @@ class WindowProxyHolder; FIELD(FeaturePolicy, RefPtr) \ /* See nsSandboxFlags.h for the possible flags. */ \ FIELD(SandboxFlags, uint32_t) \ - /* A unique identifier for the browser element that is hosting this \ - * BrowsingContext tree. Every BrowsingContext in the element's tree will \ - * return the same ID in all processes and it will remain stable \ - * regardless of process changes. When a browser element's frameloader is \ - * switched to another browser element this ID will remain the same but \ - * hosted under the under the new browser element. */ \ - FIELD(BrowserId, uint64_t) \ FIELD(HistoryID, nsID) \ FIELD(InRDMPane, bool) \ FIELD(Loading, bool) \ @@ -204,7 +197,7 @@ class BrowsingContext : public nsILoadContext, public nsWrapperCache { // DocShell, BrowserParent, or BrowserBridgeChild. static already_AddRefed CreateDetached( nsGlobalWindowInner* aParent, BrowsingContext* aOpener, - const nsAString& aName, Type aType, uint64_t aBrowserId); + const nsAString& aName, Type aType); void EnsureAttached(); @@ -400,8 +393,6 @@ class BrowsingContext : public nsILoadContext, public nsWrapperCache { bool UseGlobalHistory() const { return GetUseGlobalHistory(); } - uint64_t BrowserId() const { return GetBrowserId(); } - bool IsLoading(); // ScreenOrientation related APIs @@ -767,9 +758,6 @@ class BrowsingContext : public nsILoadContext, public nsWrapperCache { void DidSet(FieldIndex, bool aOldValue); - bool CanSet(FieldIndex, const uint32_t& aValue, - ContentParent* aSource); - template bool CanSet(FieldIndex, const T&, ContentParent*) { return true; diff --git a/docshell/test/browser/browser_browsingContext-02.js b/docshell/test/browser/browser_browsingContext-02.js index f4be161fba35..12d4d6d71a2f 100644 --- a/docshell/test/browser/browser_browsingContext-02.js +++ b/docshell/test/browser/browser_browsingContext-02.js @@ -28,7 +28,7 @@ add_task(async function() { true, true ); - let browserIds = await SpecialPowers.spawn( + await SpecialPowers.spawn( browser, [{ base1: BASE1, base2: BASE2 }], async function({ base1, base2 }) { @@ -172,58 +172,10 @@ add_task(async function() { } await unreachable(start, seventh); } - - let topBrowserId = topBC.browserId; - ok(topBrowserId > 0, "Should have a browser ID."); - for (let [name, bc] of Object.entries({ - first, - second, - third, - fourth, - fifth, - })) { - is( - bc.browserId, - topBrowserId, - `${name} frame should have the same browserId as top.` - ); - } - - ok(sixth.browserId > 0, "sixth should have a browserId."); - isnot( - sixth.browserId, - topBrowserId, - "sixth frame should have a different browserId to top." - ); - - return [topBrowserId, sixth.browserId]; } ); - [sixth, seventh] = await Promise.all([sixth, seventh]); - - is( - browser.browserId, - browserIds[0], - "browser should have the right browserId." - ); - is( - browser.browsingContext.browserId, - browserIds[0], - "browser's BrowsingContext should have the right browserId." - ); - is( - sixth.linkedBrowser.browserId, - browserIds[1], - "sixth should have the right browserId." - ); - is( - sixth.linkedBrowser.browsingContext.browserId, - browserIds[1], - "sixth's BrowsingContext should have the right browserId." - ); - - for (let tab of [sixth, seventh]) { + for (let tab of await Promise.all([sixth, seventh])) { BrowserTestUtils.removeTab(tab); } } diff --git a/dom/base/nsContentUtils.cpp b/dom/base/nsContentUtils.cpp index a3ed4b5e91b9..09b211f2cf7d 100644 --- a/dom/base/nsContentUtils.cpp +++ b/dom/base/nsContentUtils.cpp @@ -9858,14 +9858,6 @@ uint64_t nsContentUtils::GenerateTabId() { return GenerateProcessSpecificId(++gNextTabId); } -// Next process-local Browser ID. -static uint64_t gNextBrowserId = 0; - -/* static */ -uint64_t nsContentUtils::GenerateBrowserId() { - return GenerateProcessSpecificId(++gNextBrowserId); -} - // Next process-local Browsing Context ID. static uint64_t gNextBrowsingContextId = 0; diff --git a/dom/base/nsContentUtils.h b/dom/base/nsContentUtils.h index 3c18535fa38f..3fe23fd94568 100644 --- a/dom/base/nsContentUtils.h +++ b/dom/base/nsContentUtils.h @@ -3143,11 +3143,6 @@ class nsContentUtils { */ static uint64_t GenerateTabId(); - /** - * Compose a browser id with process id and a serial number. - */ - static uint64_t GenerateBrowserId(); - /** * Generate an id for a BrowsingContext using a range of serial * numbers reserved for the current process. diff --git a/dom/base/nsFrameLoader.cpp b/dom/base/nsFrameLoader.cpp index 1b9166b89bb7..8d4a4d373471 100644 --- a/dom/base/nsFrameLoader.cpp +++ b/dom/base/nsFrameLoader.cpp @@ -83,7 +83,6 @@ #include "mozilla/dom/MozFrameLoaderOwnerBinding.h" #include "mozilla/dom/SessionStoreListener.h" #include "mozilla/dom/WindowGlobalParent.h" -#include "mozilla/dom/XULFrameElement.h" #include "mozilla/gfx/CrossProcessPaint.h" #include "nsGenericHTMLFrameElement.h" #include "GeckoProfiler.h" @@ -295,10 +294,6 @@ static already_AddRefed CreateBrowsingContext( nsAutoString frameName; GetFrameName(aOwner, frameName); - // By default we just use the same browserId as the parent. - uint64_t browserId = parentBC->GetBrowserId(); - RefPtr owner = do_QueryObject(aOwner); - // Create our BrowsingContext without immediately attaching it. It's possible // that no DocShell or remote browser will ever be created for this // FrameLoader, particularly if the document that we were created for is not @@ -306,38 +301,16 @@ static already_AddRefed CreateBrowsingContext( // it will wind up attached as a child of the currently active inner window // for the BrowsingContext, and cause no end of trouble. if (IsTopContent(parentBC, aOwner)) { - if (owner && owner->GetBrowserId() != 0) { - // This frame has already been assigned an Id. This can happen for example - // if a frame is re-inserted into the DOM (i.e. on a remoteness change). - browserId = owner->GetBrowserId(); - - // This implies that we do not support changing a frame's "type" - // attribute. Doing so would mean needing to change the browser Id for the - // frame and the intent is to never change this. - MOZ_DIAGNOSTIC_ASSERT(browserId != parentBC->GetBrowserId()); - } else { - browserId = nsContentUtils::GenerateBrowserId(); - if (owner) { - owner->SetBrowserId(browserId); - } - } - // Create toplevel content without a parent & as Type::Content. - return BrowsingContext::CreateDetached( - nullptr, opener, frameName, BrowsingContext::Type::Content, browserId); + return BrowsingContext::CreateDetached(nullptr, opener, frameName, + BrowsingContext::Type::Content); } MOZ_ASSERT(!aOpenWindowInfo, "Can't have openWindowInfo for non-toplevel context"); - if (owner) { - MOZ_DIAGNOSTIC_ASSERT(owner->GetBrowserId() == 0 || - owner->GetBrowserId() == browserId); - owner->SetBrowserId(browserId); - } - return BrowsingContext::CreateDetached(parentInner, nullptr, frameName, - parentBC->GetType(), browserId); + parentBC->GetType()); } static bool InitialLoadIsRemote(Element* aOwner) { @@ -1283,12 +1256,6 @@ nsresult nsFrameLoader::SwapWithOtherRemoteLoader( MaybeUpdatePrimaryBrowserParent(eBrowserParentRemoved); aOther->MaybeUpdatePrimaryBrowserParent(eBrowserParentRemoved); - // Setting the owner content does checks that the browsing context's browser - // ID matches the element it is being attached to so swap that here. - uint64_t ourBrowserId = aThisOwner->GetBrowserId(); - aThisOwner->SetBrowserId(aOtherOwner->GetBrowserId()); - aOtherOwner->SetBrowserId(ourBrowserId); - SetOwnerContent(otherContent); aOther->SetOwnerContent(ourContent); @@ -1704,12 +1671,6 @@ nsresult nsFrameLoader::SwapWithOtherLoader(nsFrameLoader* aOther, otherDocshell, ourOwner, ourBc->IsContent() ? ourChromeEventHandler.get() : nullptr); - // Setting the owner content does checks that the browsing context's browser - // ID matches the element it is being attached to so swap that here. - uint64_t ourBrowserId = aThisOwner->GetBrowserId(); - aThisOwner->SetBrowserId(aOtherOwner->GetBrowserId()); - aOtherOwner->SetBrowserId(ourBrowserId); - // Switch the owner content before we start calling AddTreeItemToTreeOwner. // Note that we rely on this to deal with setting mObservingOwnerContent to // false and calling RemoveMutationObserver as needed. diff --git a/dom/base/nsFrameLoaderOwner.h b/dom/base/nsFrameLoaderOwner.h index 2186204bf734..bf55ef356ec3 100644 --- a/dom/base/nsFrameLoaderOwner.h +++ b/dom/base/nsFrameLoaderOwner.h @@ -58,9 +58,6 @@ class nsFrameLoaderOwner : public nsISupports { void SubframeCrashed(); - uint64_t GetBrowserId() { return mBrowserId; } - void SetBrowserId(uint64_t aBrowserId) { mBrowserId = aBrowserId; } - private: bool UseRemoteSubframes(); bool ShouldPreserveBrowsingContext( @@ -89,8 +86,6 @@ class nsFrameLoaderOwner : public nsISupports { protected: virtual ~nsFrameLoaderOwner() = default; RefPtr mFrameLoader; - - uint64_t mBrowserId = 0; }; NS_DEFINE_STATIC_IID_ACCESSOR(nsFrameLoaderOwner, NS_FRAMELOADEROWNER_IID) diff --git a/dom/chrome-webidl/BrowsingContext.webidl b/dom/chrome-webidl/BrowsingContext.webidl index 072dcc861dca..8ada4591ade4 100644 --- a/dom/chrome-webidl/BrowsingContext.webidl +++ b/dom/chrome-webidl/BrowsingContext.webidl @@ -102,15 +102,9 @@ interface BrowsingContext { // while in RDM. void setRDMPaneMaxTouchPoints(octet maxTouchPoints); - /** - * A unique identifier for the browser element that is hosting this - * BrowsingContext tree. Every BrowsingContext in the element's tree will - * return the same ID in all processes and it will remain stable regardless of - * process changes. When a browser element's frameloader is switched to - * another browser element this ID will remain the same but hosted under the - * under the new browser element. - */ - attribute unsigned long long browserId; + // The watchedByDevTools flag indicates whether or not DevTools are currently + // debugging this browsing context. + [SetterThrows] attribute boolean watchedByDevTools; }; BrowsingContext includes LoadContextMixin; diff --git a/dom/chrome-webidl/XULFrameElement.webidl b/dom/chrome-webidl/XULFrameElement.webidl index 3e4d92a10d54..07f9c2934b83 100644 --- a/dom/chrome-webidl/XULFrameElement.webidl +++ b/dom/chrome-webidl/XULFrameElement.webidl @@ -16,9 +16,7 @@ interface XULFrameElement : XULElement readonly attribute nsIWebNavigation? webNavigation; readonly attribute WindowProxy? contentWindow; - readonly attribute Document? contentDocument; - - readonly attribute unsigned long long browserId; + readonly attribute Document? contentDocument; }; XULFrameElement includes MozFrameLoaderOwner; diff --git a/dom/ipc/ContentChild.cpp b/dom/ipc/ContentChild.cpp index d72ef4968b8f..c258084c8162 100644 --- a/dom/ipc/ContentChild.cpp +++ b/dom/ipc/ContentChild.cpp @@ -952,9 +952,8 @@ nsresult ContentChild::ProvideWindowCommon( openerBC = parent; } - uint64_t browserId(nsContentUtils::GenerateBrowserId()); RefPtr browsingContext = BrowsingContext::CreateDetached( - nullptr, openerBC, aName, BrowsingContext::Type::Content, browserId); + nullptr, openerBC, aName, BrowsingContext::Type::Content); MOZ_ALWAYS_SUCCEEDS(browsingContext->SetRemoteTabs(true)); MOZ_ALWAYS_SUCCEEDS(browsingContext->SetRemoteSubframes(useRemoteSubframes)); MOZ_ALWAYS_SUCCEEDS(browsingContext->SetOriginAttributes( diff --git a/dom/xul/XULFrameElement.h b/dom/xul/XULFrameElement.h index 5f5e5d65229f..f5a28b94cd47 100644 --- a/dom/xul/XULFrameElement.h +++ b/dom/xul/XULFrameElement.h @@ -39,7 +39,6 @@ class XULFrameElement final : public nsXULElement, public nsFrameLoaderOwner { already_AddRefed GetWebNavigation(); Nullable GetContentWindow(); Document* GetContentDocument(); - uint64_t BrowserId() { return GetBrowserId(); } void SwapFrameLoaders(mozilla::dom::HTMLIFrameElement& aOtherLoaderOwner, mozilla::ErrorResult& rv);