From 8b4e459999aa9f701b0da7fd126857aeb534b9f9 Mon Sep 17 00:00:00 2001 From: Nika Layzell Date: Tue, 5 Mar 2019 17:33:52 +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 | 64 ++++------------------ 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, 18 insertions(+), 152 deletions(-) diff --git a/docshell/base/BrowsingContext.cpp b/docshell/base/BrowsingContext.cpp index f610abc53127..90064a324144 100644 --- a/docshell/base/BrowsingContext.cpp +++ b/docshell/base/BrowsingContext.cpp @@ -158,11 +158,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) { @@ -275,22 +275,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 @@ -472,14 +456,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() { @@ -489,39 +466,19 @@ 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) NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(BrowsingContext) - NS_IMPL_CYCLE_COLLECTION_UNLINK(mDocShell, mChildren, mParent, mGroup) + NS_IMPL_CYCLE_COLLECTION_UNLINK(mDocShell, mChildren, mParent, mOpener, + mGroup) if (XRE_IsParentProcess()) { CanonicalBrowsingContext::Cast(tmp)->Unlink(); } @@ -529,7 +486,8 @@ NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(BrowsingContext) NS_IMPL_CYCLE_COLLECTION_UNLINK_END NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(BrowsingContext) - NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mDocShell, mChildren, mParent, mGroup) + NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mDocShell, mChildren, mParent, mOpener, + mGroup) if (XRE_IsParentProcess()) { CanonicalBrowsingContext::Cast(tmp)->Traverse(cb); } @@ -606,7 +564,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 3a9fd8a6ed66..fd3ceb0fc2d9 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(); @@ -366,7 +359,6 @@ 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 @@ -374,10 +366,6 @@ class BrowsingContext : public nsWrapperCache, // objectMoved hook and clear it from its finalize hook. JS::Heap mWindowProxy; LocationProxy mLocation; - - // 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 246fd7d1c2fe..cb00510b8d25 100644 --- a/docshell/base/CanonicalBrowsingContext.cpp +++ b/docshell/base/CanonicalBrowsingContext.cpp @@ -89,27 +89,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 5f1db091caeb..d750fca7a82b 100644 --- a/dom/ipc/ContentParent.cpp +++ b/dom/ipc/ContentParent.cpp @@ -5769,46 +5769,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 35465ad78323..d9b2e35f81b5 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 f4aafcf1c5b7..99b68414fcbe 100644 --- a/dom/ipc/PContent.ipdl +++ b/dom/ipc/PContent.ipdl @@ -1228,19 +1228,6 @@ parent: async StoreUserInteractionAsPermission(Principal aPrincipal); - /** - * 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);