From b9e168fca3138a7fd3c7c7ed7cd250da483562f0 Mon Sep 17 00:00:00 2001 From: Nika Layzell Date: Wed, 27 Feb 2019 22:46:41 +0000 Subject: [PATCH] Bug 1530550 - Synchronize mOpener and mIsActivatedByUserGesture with SYNCED_BC_FIELD, r=farre Differential Revision: https://phabricator.services.mozilla.com/D21134 --HG-- extra : moz-landing-system : lando --- docshell/base/BrowsingContext.cpp | 58 +++------------------- docshell/base/BrowsingContext.h | 26 +++------- docshell/base/CanonicalBrowsingContext.cpp | 21 -------- dom/ipc/ContentParent.cpp | 40 --------------- dom/ipc/ContentParent.h | 6 --- dom/ipc/PContent.ipdl | 13 ----- 6 files changed, 14 insertions(+), 150 deletions(-) diff --git a/docshell/base/BrowsingContext.cpp b/docshell/base/BrowsingContext.cpp index 69c6571cbdda..519fa46e1e43 100644 --- a/docshell/base/BrowsingContext.cpp +++ b/docshell/base/BrowsingContext.cpp @@ -169,11 +169,11 @@ BrowsingContext::BrowsingContext(BrowsingContext* aParent, uint64_t aBrowsingContextId, Type aType) : mName(aName), mClosed(false), + mOpener(aOpener), + mIsActivatedByUserGesture(false), mType(aType), mBrowsingContextId(aBrowsingContextId), - mParent(aParent), - mOpener(aOpener), - mIsActivatedByUserGesture(false) { + mParent(aParent) { // Specify our group in our constructor. We will explicitly join the group // when we are registered, as doing so will take a reference. if (mParent) { @@ -280,22 +280,6 @@ void BrowsingContext::GetChildren( MOZ_ALWAYS_TRUE(aChildren.AppendElements(mChildren)); } -void BrowsingContext::SetOpener(BrowsingContext* aOpener) { - if (mOpener == aOpener) { - return; - } - - mOpener = aOpener; - - if (!XRE_IsContentProcess()) { - return; - } - - auto cc = ContentChild::GetSingleton(); - MOZ_DIAGNOSTIC_ASSERT(cc); - cc->SendSetOpenerBrowsingContext(this, aOpener); -} - // FindWithName follows the rules for choosing a browsing context, // with the exception of sandboxing for iframes. The implementation // for arbitrarily choosing between two browsing contexts with the @@ -477,14 +461,7 @@ void BrowsingContext::NotifyUserGestureActivation() { RefPtr topLevelBC = TopLevelBrowsingContext(); USER_ACTIVATION_LOG("Get top level browsing context 0x%08" PRIx64, topLevelBC->Id()); - topLevelBC->SetUserGestureActivation(); - - if (!XRE_IsContentProcess()) { - return; - } - auto cc = ContentChild::GetSingleton(); - MOZ_ASSERT(cc); - cc->SendSetUserGestureActivation(topLevelBC, true); + topLevelBC->SetIsActivatedByUserGesture(true); } void BrowsingContext::NotifyResetUserGestureActivation() { @@ -494,33 +471,12 @@ void BrowsingContext::NotifyResetUserGestureActivation() { RefPtr topLevelBC = TopLevelBrowsingContext(); USER_ACTIVATION_LOG("Get top level browsing context 0x%08" PRIx64, topLevelBC->Id()); - topLevelBC->ResetUserGestureActivation(); - - if (!XRE_IsContentProcess()) { - return; - } - auto cc = ContentChild::GetSingleton(); - MOZ_ASSERT(cc); - cc->SendSetUserGestureActivation(topLevelBC, false); -} - -void BrowsingContext::SetUserGestureActivation() { - MOZ_ASSERT(!mParent, "Set user activation flag on non top-level context!"); - USER_ACTIVATION_LOG( - "Set user gesture activation for browsing context 0x%08" PRIx64, Id()); - mIsActivatedByUserGesture = true; + topLevelBC->SetIsActivatedByUserGesture(false); } bool BrowsingContext::GetUserGestureActivation() { RefPtr topLevelBC = TopLevelBrowsingContext(); - return topLevelBC->mIsActivatedByUserGesture; -} - -void BrowsingContext::ResetUserGestureActivation() { - MOZ_ASSERT(!mParent, "Clear user activation flag on non top-level context!"); - USER_ACTIVATION_LOG( - "Reset user gesture activation for browsing context 0x%08" PRIx64, Id()); - mIsActivatedByUserGesture = false; + return topLevelBC->GetIsActivatedByUserGesture(); } NS_IMPL_CYCLE_COLLECTION_CLASS(BrowsingContext) @@ -577,7 +533,7 @@ Nullable BrowsingContext::GetTop(ErrorResult& aError) { void BrowsingContext::GetOpener(JSContext* aCx, JS::MutableHandle aOpener, ErrorResult& aError) const { - auto* opener = GetOpener(); + BrowsingContext* opener = GetOpener(); if (!opener) { aOpener.setNull(); return; diff --git a/docshell/base/BrowsingContext.h b/docshell/base/BrowsingContext.h index 04edb5369a2b..d1892fd3029d 100644 --- a/docshell/base/BrowsingContext.h +++ b/docshell/base/BrowsingContext.h @@ -66,9 +66,11 @@ class WindowProxyHolder; // acts as a sentinel for callers of MOZ_FOR_EACH_SYNCED_FIELD. // clang-format off -#define MOZ_FOR_EACH_SYNCED_BC_FIELD(declare, ...) \ - declare(Name, nsString, nsAString) \ - declare(Closed, bool, bool) \ +#define MOZ_FOR_EACH_SYNCED_BC_FIELD(declare, ...) \ + declare(Name, nsString, nsAString) \ + declare(Closed, bool, bool) \ + declare(Opener, RefPtr, BrowsingContext*) \ + declare(IsActivatedByUserGesture, bool, bool) \ __VA_ARGS__ // clang-format on @@ -76,9 +78,9 @@ class WindowProxyHolder; #define MOZ_SYNCED_BC_FIELD_ARGUMENT(name, type, atype) \ transaction->MOZ_SYNCED_BC_FIELD_NAME(name), #define MOZ_SYNCED_BC_FIELD_GETTER(name, type, atype) \ - const type& Get##name() const { return MOZ_SYNCED_BC_FIELD_NAME(name); } + type const& Get##name() const { return MOZ_SYNCED_BC_FIELD_NAME(name); } #define MOZ_SYNCED_BC_FIELD_SETTER(name, type, atype) \ - void Set##name(const atype& aValue) { \ + void Set##name(atype const& aValue) { \ Transaction t; \ t.MOZ_SYNCED_BC_FIELD_NAME(name).emplace(aValue); \ t.Commit(this); \ @@ -204,10 +206,6 @@ class BrowsingContext : public nsWrapperCache, void GetChildren(nsTArray>& aChildren); - BrowsingContext* GetOpener() const { return mOpener; } - - void SetOpener(BrowsingContext* aOpener); - BrowsingContextGroup* Group() { return mGroup; } // Using the rules for choosing a browsing context we try to find @@ -241,11 +239,6 @@ class BrowsingContext : public nsWrapperCache, // activation flag of the top level browsing context. void NotifyResetUserGestureActivation(); - // These functions would only be called in the top level browsing context. - // They would set/reset the user gesture activation flag. - void SetUserGestureActivation(); - void ResetUserGestureActivation(); - // Return true if it corresponding document is activated by user gesture. bool GetUserGestureActivation(); @@ -338,17 +331,12 @@ class BrowsingContext : public nsWrapperCache, RefPtr mGroup; RefPtr mParent; Children mChildren; - WeakPtr mOpener; nsCOMPtr mDocShell; // This is not a strong reference, but using a JS::Heap for that should be // fine. The JSObject stored in here should be a proxy with a // nsOuterWindowProxy handler, which will update the pointer from its // objectMoved hook and clear it from its finalize hook. JS::Heap mWindowProxy; - - // This flag is only valid in the top level browsing context, it indicates - // whether the corresponding document has been activated by user gesture. - bool mIsActivatedByUserGesture; }; /** diff --git a/docshell/base/CanonicalBrowsingContext.cpp b/docshell/base/CanonicalBrowsingContext.cpp index 71a88ce2c75d..b32f33ab9bbf 100644 --- a/docshell/base/CanonicalBrowsingContext.cpp +++ b/docshell/base/CanonicalBrowsingContext.cpp @@ -87,27 +87,6 @@ JSObject* CanonicalBrowsingContext::WrapObject( return CanonicalBrowsingContext_Binding::Wrap(aCx, this, aGivenProto); } -void CanonicalBrowsingContext::NotifySetUserGestureActivationFromIPC( - bool aIsUserGestureActivation) { - if (!mCurrentWindowGlobal) { - return; - } - - if (aIsUserGestureActivation) { - SetUserGestureActivation(); - } else { - ResetUserGestureActivation(); - } - - USER_ACTIVATION_LOG("Chrome browsing context 0x%08" PRIx64 - " would notify other browsing contexts for updating " - "user gesture activation flag.", - Id()); - // XXX(alwu) : we need to sync the flag to other browsing contexts which are - // not in the same child process where the flag was set. Will implement that - // in bug1519229. -} - void CanonicalBrowsingContext::Traverse( nsCycleCollectionTraversalCallback& cb) { CanonicalBrowsingContext* tmp = this; diff --git a/dom/ipc/ContentParent.cpp b/dom/ipc/ContentParent.cpp index 48630ff422f5..87ebe0b59b58 100644 --- a/dom/ipc/ContentParent.cpp +++ b/dom/ipc/ContentParent.cpp @@ -5727,46 +5727,6 @@ mozilla::ipc::IPCResult ContentParent::RecvDetachBrowsingContext( return IPC_OK(); } -mozilla::ipc::IPCResult ContentParent::RecvSetOpenerBrowsingContext( - BrowsingContext* aContext, BrowsingContext* aOpener) { - if (!aContext) { - MOZ_LOG(BrowsingContext::GetLog(), LogLevel::Debug, - ("ParentIPC: Trying to set opener already detached")); - return IPC_OK(); - } - - if (!aContext->Canonical()->IsOwnedByProcess(ChildID())) { - // Where trying to set opener on 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. - - // TODO(farre): To crash or not to crash. Same reasoning as in - // above TODO. [Bug 1471598] - MOZ_LOG(BrowsingContext::GetLog(), LogLevel::Warning, - ("ParentIPC: Trying to set opener on out of process context " - "0x%08" PRIx64, - aContext->Id())); - return IPC_OK(); - } - - aContext->SetOpener(aOpener); - - return IPC_OK(); -} - -mozilla::ipc::IPCResult ContentParent::RecvSetUserGestureActivation( - BrowsingContext* aContext, bool aNewValue) { - if (!aContext) { - MOZ_LOG(BrowsingContext::GetLog(), LogLevel::Debug, - ("ParentIPC: Trying to activate wrong context")); - return IPC_OK(); - } - - aContext->Canonical()->NotifySetUserGestureActivationFromIPC(aNewValue); - return IPC_OK(); -} - void ContentParent::RegisterRemoteWorkerActor() { ++mRemoteWorkerActors; } void ContentParent::UnregisterRemoveWorkerActor() { diff --git a/dom/ipc/ContentParent.h b/dom/ipc/ContentParent.h index 8d212cddfa0b..c48a07475be2 100644 --- a/dom/ipc/ContentParent.h +++ b/dom/ipc/ContentParent.h @@ -618,9 +618,6 @@ class ContentParent final : public PContentParent, mozilla::ipc::IPCResult RecvDetachBrowsingContext(BrowsingContext* aContext, bool aMoveToBFCache); - mozilla::ipc::IPCResult RecvSetOpenerBrowsingContext( - BrowsingContext* aContext, BrowsingContext* aOpener); - mozilla::ipc::IPCResult RecvWindowClose(BrowsingContext* aContext, bool aTrustedCaller); mozilla::ipc::IPCResult RecvWindowFocus(BrowsingContext* aContext); @@ -629,9 +626,6 @@ class ContentParent final : public PContentParent, BrowsingContext* aContext, const ClonedMessageData& aMessage, const PostMessageData& aData); - mozilla::ipc::IPCResult RecvSetUserGestureActivation( - BrowsingContext* aContext, bool aNewValue); - FORWARD_SHMEM_ALLOCATOR_TO(PContentParent) protected: diff --git a/dom/ipc/PContent.ipdl b/dom/ipc/PContent.ipdl index 69524aea4111..a4979bb8e2bb 100644 --- a/dom/ipc/PContent.ipdl +++ b/dom/ipc/PContent.ipdl @@ -1253,19 +1253,6 @@ parent: async DetachBrowsingContext(BrowsingContext aContext, bool aMoveToBFCache); - /** - * Set the opener of browsing context 'aContext' to the browsing context - * with id 'aOpenerId'. - */ - async SetOpenerBrowsingContext(BrowsingContext aContext, - BrowsingContext aOpenerContext); - - /** - * Notify parent to update user gesture activation flag. - */ - async SetUserGestureActivation(BrowsingContext aContext, - bool aNewValue); - both: async CommitBrowsingContextTransaction(BrowsingContext aContext, BrowsingContextTransaction aTransaction);