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

@@ -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<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() {
@@ -494,33 +471,12 @@ void BrowsingContext::NotifyResetUserGestureActivation() {
   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)
@@ -577,7 +533,7 @@ Nullable<WindowProxyHolder> BrowsingContext::GetTop(ErrorResult& aError) {
 void BrowsingContext::GetOpener(JSContext* aCx,
                                 JS::MutableHandle<JS::Value> aOpener,
                                 ErrorResult& aError) const {
-  auto* opener = GetOpener();
+  BrowsingContext* opener = GetOpener();
   if (!opener) {
     aOpener.setNull();
     return;

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>, 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<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
@@ -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<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;
 };
 
 /**

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;

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() {

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:

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);