Bug 1530550 - Synchronize mOpener and mIsActivatedByUserGesture with SYNCED_BC_FIELD, r=farre
☠☠ backed out by 67213a91036c ☠ ☠
authorNika Layzell <nika@thelayzells.com>
Wed, 27 Feb 2019 22:46:41 +0000
changeset 519429 c2091f595c72666fdc160828a3577aac7974f1ba
parent 519428 2f27f2c7a443b3c798c0722379328e6ba6c3ef5d
child 519430 8e58294baf24a304a10a1ac46607abc7711ad0a3
push id10862
push userffxbld-merge
push dateMon, 11 Mar 2019 13:01:11 +0000
treeherdermozilla-beta@a2e7f5c935da [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 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
@@ -164,21 +164,21 @@ CanonicalBrowsingContext* BrowsingContex
 }
 
 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),
-      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) {
     mGroup = mParent->Group();
   } else if (mOpener) {
     mGroup = mOpener->Group();
   } else {
     // To ensure the group has a unique ID, we will use our ID, as the founder
@@ -275,32 +275,16 @@ void BrowsingContext::CacheChildren() {
 
 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
@@ -472,60 +456,32 @@ 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)
   if (XRE_IsParentProcess()) {
     CanonicalBrowsingContext::Cast(tmp)->Unlink();
@@ -572,17 +528,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
@@ -61,29 +61,31 @@ 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(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);
@@ -199,20 +201,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.
@@ -236,21 +234,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;
@@ -333,27 +326,22 @@ 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;
-
-  // 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;
 };
 
 /**
  * Gets a WindowProxy object for a BrowsingContext that lives in a different
  * process (creating the object if it doesn't already exist). The WindowProxy
  * object will be in the compartment that aCx is currently in. This should only
  * be called if aContext doesn't hold a docshell, otherwise the BrowsingContext
  * lives in this process, and a same-process WindowProxy should be used (see
--- a/docshell/base/CanonicalBrowsingContext.cpp
+++ b/docshell/base/CanonicalBrowsingContext.cpp
@@ -82,37 +82,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
@@ -5722,56 +5722,16 @@ mozilla::ipc::IPCResult ContentParent::R
     aContext->CacheChildren();
   } else {
     aContext->Detach();
   }
 
   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
@@ -613,30 +613,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
@@ -1248,29 +1248,16 @@ parent:
      * BrowsingContext belonging to an already detached subtree. The
      * 'aMoveToBFCache' paramater controls if detaching a BrowsingContext
      * should move it to the bfcache allowing it to be re-attached if navigated
      * to.
      */
     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);
 
     async AsyncMessage(nsString aMessage, CpowEntry[] aCpows,
                        Principal aPrincipal, ClonedMessageData aData);
 
     /**