Bug 1530550 - Synchronize mOpener and mIsActivatedByUserGesture with SYNCED_BC_FIELD, r=farre
authorNika Layzell <nika@thelayzells.com>
Thu, 14 Mar 2019 18:50:55 +0000
changeset 521931 42b3b54ac6f7
parent 521930 2e9f31ee6571
child 521932 84d076d2163d
push id10870
push usernbeleuzu@mozilla.com
push dateFri, 15 Mar 2019 20:00:07 +0000
treeherdermozilla-beta@c594aee5b7a4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfarre
bugs1530550
milestone67.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
Bug 1530550 - Synchronize mOpener and mIsActivatedByUserGesture with SYNCED_BC_FIELD, r=farre Depends on D22764 Differential Revision: https://phabricator.services.mozilla.com/D21134
docshell/base/BrowsingContext.cpp
docshell/base/BrowsingContext.h
docshell/base/CanonicalBrowsingContext.cpp
dom/ipc/ContentParent.cpp
dom/ipc/ContentParent.h
dom/ipc/PContent.ipdl
--- a/docshell/base/BrowsingContext.cpp
+++ b/docshell/base/BrowsingContext.cpp
@@ -153,23 +153,24 @@ already_AddRefed<BrowsingContext> Browsi
 }
 
 BrowsingContext::BrowsingContext(BrowsingContext* aParent,
                                  BrowsingContext* aOpener,
                                  const nsAString& aName,
                                  uint64_t aBrowsingContextId, Type aType)
     : mName(aName),
       mClosed(false),
+      mOpener(aOpener),
+      mIsActivatedByUserGesture(false),
       mType(aType),
       mBrowsingContextId(aBrowsingContextId),
       mParent(aParent),
-      mIsInProcess(false),
-      mOpener(aOpener),
-      mIsActivatedByUserGesture(false) {
+      mIsInProcess(false) {
   mCrossOriginPolicy = nsILoadInfo::CROSS_ORIGIN_POLICY_NULL;
+
   // 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) {
     mGroup = mParent->Group();
     mCrossOriginPolicy = mParent->CrossOriginPolicy();
   } else if (mOpener) {
     mGroup = mOpener->Group();
     mCrossOriginPolicy = mOpener->CrossOriginPolicy();
@@ -280,32 +281,16 @@ void BrowsingContext::CacheChildren(bool
 
 bool BrowsingContext::IsCached() { return sCachedBrowsingContexts->has(Id()); }
 
 void BrowsingContext::GetChildren(
     nsTArray<RefPtr<BrowsingContext>>& aChildren) {
   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
 // same name is as follows:
 //
 // 1) The start browsing context, i.e. 'this'
 // 2) Descendants in insertion order
 // 3) The parent
@@ -477,74 +462,46 @@ JSObject* BrowsingContext::WrapObject(JS
 
 void BrowsingContext::NotifyUserGestureActivation() {
   // We would set the user gesture activation flag on the top level browsing
   // context, which would automatically be sync to other top level browsing
   // contexts which are in the different process.
   RefPtr<BrowsingContext> 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() {
   // We would reset the user gesture activation flag on the top level browsing
   // context, which would automatically be sync to other top level browsing
   // contexts which are in the different process.
   RefPtr<BrowsingContext> 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<BrowsingContext> 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();
   }
   NS_IMPL_CYCLE_COLLECTION_UNLINK_PRESERVED_WRAPPER
 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);
   }
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 
 NS_IMPL_CYCLE_COLLECTION_TRACE_WRAPPERCACHE(BrowsingContext)
 
 NS_IMPL_CYCLE_COLLECTION_ROOT_NATIVE(BrowsingContext, AddRef)
@@ -620,17 +577,17 @@ Nullable<WindowProxyHolder> BrowsingCont
   // We never return null or throw an error, but the implementation in
   // nsGlobalWindow does and we need to use the same signature.
   return WindowProxyHolder(TopLevelBrowsingContext());
 }
 
 void BrowsingContext::GetOpener(JSContext* aCx,
                                 JS::MutableHandle<JS::Value> aOpener,
                                 ErrorResult& aError) const {
-  auto* opener = GetOpener();
+  BrowsingContext* opener = GetOpener();
   if (!opener) {
     aOpener.setNull();
     return;
   }
 
   if (!ToJSValue(aCx, WindowProxyHolder(opener), aOpener)) {
     aError.NoteJSContextException(aCx);
   }
--- a/docshell/base/BrowsingContext.h
+++ b/docshell/base/BrowsingContext.h
@@ -62,30 +62,32 @@ class WindowProxyHolder;
 // supplied name prepended with 'm'. If the field needs to be
 // initialized in the constructor, then that will have to be done
 // manually, and of course keeping the same order as below.
 //
 // At all times the last line below should be __VA_ARGS__, since that
 // 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(CrossOriginPolicy, nsILoadInfo::CrossOriginPolicy, nsILoadInfo::CrossOriginPolicy) \
+  declare(Opener, RefPtr<BrowsingContext>, BrowsingContext*) \
+  declare(IsActivatedByUserGesture, bool, bool)              \
   __VA_ARGS__
 // clang-format on
 
 #define MOZ_SYNCED_BC_FIELD_NAME(name, ...) m##name
 #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);                                   \
   }
 #define MOZ_SYNCED_BC_FIELD_MEMBER(name, type, atype) \
   type MOZ_SYNCED_BC_FIELD_NAME(name);
 #define MOZ_SYNCED_BC_FIELD_MAYBE_MEMBER(name, type, atype) \
   mozilla::Maybe<type> MOZ_SYNCED_BC_FIELD_NAME(name);
@@ -206,20 +208,16 @@ class BrowsingContext : public nsWrapper
   bool IsContent() const { return mType == Type::Content; }
 
   uint64_t Id() const { return mBrowsingContextId; }
 
   BrowsingContext* GetParent() { return mParent; }
 
   void GetChildren(nsTArray<RefPtr<BrowsingContext>>& 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
   // the browsing context with the given name in the set of
   // transitively reachable browsing contexts. Performs access control
   // with regards to this.
   // See
   // https://html.spec.whatwg.org/multipage/browsers.html#the-rules-for-choosing-a-browsing-context-given-a-browsing-context-name.
@@ -243,21 +241,16 @@ class BrowsingContext : public nsWrapper
   // by user gesture, and we would set the flag in the top level browsing
   // context.
   void NotifyUserGestureActivation();
 
   // This function would be called when we want to reset the user gesture
   // 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();
 
   // Return the window proxy object that corresponds to this browsing context.
   inline JSObject* GetWindowProxy() const { return mWindowProxy; }
   // Set the window proxy object that corresponds to this browsing context.
   void SetWindowProxy(JS::Handle<JSObject*> aWindowProxy) {
     mWindowProxy = aWindowProxy;
@@ -372,30 +365,25 @@ class BrowsingContext : public nsWrapper
   const Type mType;
 
   // Unique id identifying BrowsingContext
   const uint64_t mBrowsingContextId;
 
   RefPtr<BrowsingContextGroup> mGroup;
   RefPtr<BrowsingContext> mParent;
   Children mChildren;
-  WeakPtr<BrowsingContext> mOpener;
   nsCOMPtr<nsIDocShell> 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<JSObject*> 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;
-
   // Is the most recent Document in this BrowsingContext loaded within this
   // process? This may be true with a null mDocShell after the Window has been
   // closed.
   bool mIsInProcess : 1;
 };
 
 /**
  * Gets a WindowProxy object for a BrowsingContext that lives in a different
--- a/docshell/base/CanonicalBrowsingContext.cpp
+++ b/docshell/base/CanonicalBrowsingContext.cpp
@@ -95,37 +95,16 @@ void CanonicalBrowsingContext::SetCurren
   mCurrentWindowGlobal = aGlobal;
 }
 
 JSObject* CanonicalBrowsingContext::WrapObject(
     JSContext* aCx, JS::Handle<JSObject*> aGivenProto) {
   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;
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mWindowGlobals);
 }
 
 void CanonicalBrowsingContext::Unlink() {
   CanonicalBrowsingContext* tmp = this;
--- a/dom/ipc/ContentParent.cpp
+++ b/dom/ipc/ContentParent.cpp
@@ -5767,56 +5767,16 @@ mozilla::ipc::IPCResult ContentParent::R
     aContext->CacheChildren(/* aFromIPC */ true);
   } else {
     aContext->Detach(/* aFromIPC */ true);
   }
 
   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() {
   MOZ_ASSERT(NS_IsMainThread());
 
   if (--mRemoteWorkerActors) {
     return;
   }
--- a/dom/ipc/ContentParent.h
+++ b/dom/ipc/ContentParent.h
@@ -611,30 +611,24 @@ class ContentParent final : public PCont
 
   mozilla::ipc::IPCResult RecvAttachBrowsingContext(
       BrowsingContext* aParentContext, BrowsingContext* aOpener,
       BrowsingContextId aContextId, const nsString& aName);
 
   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);
   mozilla::ipc::IPCResult RecvWindowBlur(BrowsingContext* aContext);
   mozilla::ipc::IPCResult RecvWindowPostMessage(
       BrowsingContext* aContext, const ClonedMessageData& aMessage,
       const PostMessageData& aData);
 
-  mozilla::ipc::IPCResult RecvSetUserGestureActivation(
-      BrowsingContext* aContext, bool aNewValue);
-
   FORWARD_SHMEM_ALLOCATOR_TO(PContentParent)
 
  protected:
   void OnChannelConnected(int32_t pid) override;
 
   virtual void ActorDestroy(ActorDestroyReason why) override;
 
   bool ShouldContinueFromReplyTimeout() override;
--- a/dom/ipc/PContent.ipdl
+++ b/dom/ipc/PContent.ipdl
@@ -1219,29 +1219,16 @@ parent:
                                                   Principal aTrackingPrincipal,
                                                   nsCString aTrackingOrigin,
                                                   nsCString aGrantedOrigin,
                                                   int aAllowMode)
           returns (bool unused);
 
     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);
 
     async AsyncMessage(nsString aMessage, CpowEntry[] aCpows,
                        Principal aPrincipal, ClonedMessageData aData);
 
     /**