Bug 1532661 - Part 2: Use a field list macro header for synced fields, r=farre
authorNika Layzell <nika@thelayzells.com>
Thu, 14 Mar 2019 18:51:05 +0000
changeset 464042 a39da23d450b7e3cda053097d77ca47de40f1bb2
parent 464041 84d076d2163d4ee8c0d7d0e5d41591e9d9132a80
child 464043 58f77fa35eeab8de03c52f316d9f0553685a5f8d
push id35707
push userrmaries@mozilla.com
push dateFri, 15 Mar 2019 03:42:43 +0000
treeherdermozilla-central@5ce27c44f79e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfarre
bugs1532661
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 1532661 - Part 2: Use a field list macro header for synced fields, r=farre Depends on D22190 Differential Revision: https://phabricator.services.mozilla.com/D22191
docshell/base/BrowsingContext.cpp
docshell/base/BrowsingContext.h
docshell/base/BrowsingContextFieldList.h
docshell/base/moz.build
dom/fetch/FetchDriver.cpp
dom/ipc/ContentChild.cpp
dom/ipc/ContentParent.cpp
netwerk/protocol/http/nsHttpChannel.cpp
--- a/docshell/base/BrowsingContext.cpp
+++ b/docshell/base/BrowsingContext.cpp
@@ -676,17 +676,27 @@ void BrowsingContext::Transaction::Commi
     for (auto iter = aBrowsingContext->Group()->ContentParentsIter();
          !iter.Done(); iter.Next()) {
       nsRefPtrHashKey<ContentParent>* entry = iter.Get();
       Unused << entry->GetKey()->SendCommitBrowsingContextTransaction(
           aBrowsingContext, *this);
     }
   }
 
-  Apply(aBrowsingContext);
+  Apply(aBrowsingContext, nullptr);
+}
+
+void BrowsingContext::Transaction::Apply(BrowsingContext* aBrowsingContext,
+                                         ContentParent* aSource) {
+#define MOZ_BC_FIELD(name, ...)                         \
+  if (m##name) {                                        \
+    aBrowsingContext->m##name = std::move(*m##name);    \
+    m##name.reset();                                    \
+  }
+#include "mozilla/dom/BrowsingContextFieldList.h"
 }
 
 void BrowsingContext::LocationProxy::SetHref(const nsAString& aHref,
                                              nsIPrincipal& aSubjectPrincipal,
                                              ErrorResult& aError) {
   nsPIDOMWindowOuter* win = GetBrowsingContext()->GetDOMWindow();
   if (!win || !win->GetLocation()) {
     aError.Throw(NS_ERROR_FAILURE);
@@ -746,28 +756,27 @@ bool IPDLParamTraits<dom::BrowsingContex
   }
 
   return aResult != nullptr;
 }
 
 void IPDLParamTraits<dom::BrowsingContext::Transaction>::Write(
     IPC::Message* aMessage, IProtocol* aActor,
     const dom::BrowsingContext::Transaction& aTransaction) {
-  void_t sentinel;
-  const dom::BrowsingContext::Transaction* transaction = &aTransaction;
-  auto tuple = mozilla::Tie(
-      MOZ_FOR_EACH_SYNCED_BC_FIELD(MOZ_SYNCED_BC_FIELD_ARGUMENT, sentinel));
-
-  WriteIPDLParam(aMessage, aActor, tuple);
+#define MOZ_BC_FIELD(name, ...) \
+  WriteIPDLParam(aMessage, aActor, aTransaction.m##name);
+#include "mozilla/dom/BrowsingContextFieldList.h"
 }
 
 bool IPDLParamTraits<dom::BrowsingContext::Transaction>::Read(
     const IPC::Message* aMessage, PickleIterator* aIterator, IProtocol* aActor,
     dom::BrowsingContext::Transaction* aTransaction) {
-  void_t sentinel;
-  dom::BrowsingContext::Transaction* transaction = aTransaction;
-  auto tuple = mozilla::Tie(
-      MOZ_FOR_EACH_SYNCED_BC_FIELD(MOZ_SYNCED_BC_FIELD_ARGUMENT, sentinel));
-  return ReadIPDLParam(aMessage, aIterator, aActor, &tuple);
+#define MOZ_BC_FIELD(name, ...)                                              \
+  if (!ReadIPDLParam(aMessage, aIterator, aActor, &aTransaction->m##name)) { \
+    return false;                                                            \
+  }
+#include "mozilla/dom/BrowsingContextFieldList.h"
+
+  return true;
 }
 
 }  // namespace ipc
 }  // namespace mozilla
--- a/docshell/base/BrowsingContext.h
+++ b/docshell/base/BrowsingContext.h
@@ -37,91 +37,39 @@ class LogModule;
 namespace ipc {
 class IProtocol;
 
 template <typename T>
 struct IPDLParamTraits;
 }  // namespace ipc
 
 namespace dom {
+class BrowsingContent;
 class BrowsingContextGroup;
 class CanonicalBrowsingContext;
 class ContentParent;
 template <typename>
 struct Nullable;
 template <typename T>
 class Sequence;
 struct WindowPostMessageOptions;
 class WindowProxyHolder;
 
-// MOZ_FOR_EACH_SYNCED_FIELD declares BrowsingContext fields that need
-// to be synced to the synced versions of BrowsingContext in parent
-// and child processes. To add a new field for syncing add a line:
-//
-// declare(name of new field, storage type, parameter type)
-//
-// before __VA_ARGS__. This will declare a private member with the
-// 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)                                \
-  declare(CrossOriginPolicy, nsILoadInfo::CrossOriginPolicy, nsILoadInfo::CrossOriginPolicy) \
-  declare(Opener, RefPtr<BrowsingContext>, BrowsingContext*) \
-  declare(IsActivatedByUserGesture, bool, bool)              \
-  __VA_ARGS__
-// clang-format on
+class BrowsingContextBase {
+ protected:
+  BrowsingContextBase() {
+    // default-construct each field.
+#define MOZ_BC_FIELD(name, type) m##name = type();
+#include "mozilla/dom/BrowsingContextFieldList.h"
+  }
+  ~BrowsingContextBase() = default;
 
-#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) \
-  type const& Get##name() const { return MOZ_SYNCED_BC_FIELD_NAME(name); }
-#define MOZ_SYNCED_BC_FIELD_SETTER(name, type, atype) \
-  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);
-#define MOZ_SYNCED_BC_FIELD_APPLIER(name, type, atype) \
-  if (MOZ_SYNCED_BC_FIELD_NAME(name)) {                \
-    aOwner->MOZ_SYNCED_BC_FIELD_NAME(name) =           \
-        std::move(*MOZ_SYNCED_BC_FIELD_NAME(name));    \
-    MOZ_SYNCED_BC_FIELD_NAME(name).reset();            \
-  }
-
-#define MOZ_SYNCED_BC_FIELDS                                              \
- public:                                                                  \
-  MOZ_FOR_EACH_SYNCED_BC_FIELD(MOZ_SYNCED_BC_FIELD_GETTER)                \
-  MOZ_FOR_EACH_SYNCED_BC_FIELD(MOZ_SYNCED_BC_FIELD_SETTER)                \
-  class Transaction {                                                     \
-   public:                                                                \
-    void Commit(BrowsingContext* aOwner);                                 \
-    void Apply(BrowsingContext* aOwner) {                                 \
-      MOZ_FOR_EACH_SYNCED_BC_FIELD(MOZ_SYNCED_BC_FIELD_APPLIER)           \
-      return; /* without this return clang-format messes up formatting */ \
-    }                                                                     \
-                                                                          \
-    MOZ_FOR_EACH_SYNCED_BC_FIELD(MOZ_SYNCED_BC_FIELD_MAYBE_MEMBER)        \
-   private:                                                               \
-    friend struct mozilla::ipc::IPDLParamTraits<Transaction>;             \
-  };                                                                      \
-                                                                          \
- private:                                                                 \
-  MOZ_FOR_EACH_SYNCED_BC_FIELD(MOZ_SYNCED_BC_FIELD_MEMBER)
+#define BC_FIELD(name, type) type m##name;
+#include "mozilla/dom/BCFieldList.h"
+};
 
 // BrowsingContext, in this context, is the cross process replicated
 // environment in which information about documents is stored. In
 // particular the tree structure of nested browsing contexts is
 // represented by the tree of BrowsingContexts.
 //
 // The tree of BrowsingContexts is created in step with its
 // corresponding nsDocShell, and when nsDocShells are connected
@@ -131,20 +79,18 @@ class WindowProxyHolder;
 // BrowsingContext tree for a tab, in both the parent and the child
 // process.
 //
 // Trees of BrowsingContexts should only ever contain nodes of the
 // same BrowsingContext::Type. This is enforced by asserts in the
 // BrowsingContext::Create* methods.
 class BrowsingContext : public nsWrapperCache,
                         public SupportsWeakPtr<BrowsingContext>,
-                        public LinkedListElement<RefPtr<BrowsingContext>> {
-  // Do not declare members above MOZ_SYNCED_BC_FIELDS
-  MOZ_SYNCED_BC_FIELDS
-
+                        public LinkedListElement<RefPtr<BrowsingContext>>,
+                        public BrowsingContextBase {
  public:
   enum class Type { Chrome, Content };
 
   static void Init();
   static LogModule* GetLog();
   static void CleanupContexts(uint64_t aProcessId);
 
   // Look up a BrowsingContext in the current process by ID.
@@ -283,19 +229,53 @@ class BrowsingContext : public nsWrapper
                       const Sequence<JSObject*>& aTransfer,
                       nsIPrincipal& aSubjectPrincipal, ErrorResult& aError);
   void PostMessageMoz(JSContext* aCx, JS::Handle<JS::Value> aMessage,
                       const WindowPostMessageOptions& aOptions,
                       nsIPrincipal& aSubjectPrincipal, ErrorResult& aError);
 
   JSObject* WrapObject(JSContext* aCx);
 
-  nsILoadInfo::CrossOriginPolicy CrossOriginPolicy() {
-    return mCrossOriginPolicy;
-  }
+  /**
+   * Transaction object. This object is used to specify and then commit
+   * modifications to synchronized fields in BrowsingContexts.
+   */
+  class Transaction {
+   public:
+    // Apply the changes from this transaction to the specified BrowsingContext
+    // in all processes. This method will call the correct `WillSet` and
+    // `DidSet` methods, as well as move the value.
+    //
+    // NOTE: This method mutates `this`, resetting all members to `Nothing()`
+    void Commit(BrowsingContext* aOwner);
+
+    // You probably don't want to directly call this method - instead call
+    // `Commit`, which will perform the necessary synchronization.
+    //
+    // |aSource| is the ContentParent which is performing the mutation in the
+    // parent process.
+    void Apply(BrowsingContext* aOwner, ContentParent* aSource);
+
+#define MOZ_BC_FIELD(name, type) mozilla::Maybe<type> m##name;
+#include "mozilla/dom/BrowsingContextFieldList.h"
+
+   private:
+    friend struct mozilla::ipc::IPDLParamTraits<Transaction>;
+  };
+
+#define MOZ_BC_FIELD(name, type)                        \
+  template <typename... Args>                           \
+  void Set##name(Args&&... aValue) {                    \
+    Transaction txn;                                    \
+    txn.m##name.emplace(std::forward<Args>(aValue)...); \
+    txn.Commit(this);                                   \
+  }                                                     \
+                                                        \
+  type const& Get##name() const { return m##name; }
+#include "mozilla/dom/BrowsingContextFieldList.h"
 
  protected:
   virtual ~BrowsingContext();
   BrowsingContext(BrowsingContext* aParent, BrowsingContext* aOpener,
                   const nsAString& aName, uint64_t aBrowsingContextId,
                   Type aType);
 
  private:
new file mode 100644
--- /dev/null
+++ b/docshell/base/BrowsingContextFieldList.h
@@ -0,0 +1,23 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=8 sts=2 et sw=2 tw=80: */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+MOZ_BC_FIELD(Name, nsString)
+MOZ_BC_FIELD(Closed, bool)
+MOZ_BC_FIELD(CrossOriginPolicy, nsILoadInfo::CrossOriginPolicy)
+
+// The current opener for this BrowsingContext. This field can be skipped by
+// defining `MOZ_BC_FIELD_SKIP_OPENER`, which is used when serializing
+// synchronized field state when initializing browsing contexts.
+#ifndef MOZ_BC_FIELD_SKIP_OPENER
+MOZ_BC_FIELD(Opener, RefPtr<BrowsingContext>)
+#endif
+
+// Toplevel browsing contexts only. This field controls whether the browsing
+// context is currently considered to be activated by a gesture.
+MOZ_BC_FIELD(IsActivatedByUserGesture, bool)
+
+#undef MOZ_BC_FIELD
+#undef MOZ_BC_FIELD_SKIP_OPENER
--- a/docshell/base/moz.build
+++ b/docshell/base/moz.build
@@ -71,16 +71,17 @@ EXPORTS += [
 
 EXPORTS.mozilla += [
     'IHistory.h',
     'LoadContext.h',
 ]
 
 EXPORTS.mozilla.dom += [
     'BrowsingContext.h',
+    'BrowsingContextFieldList.h',
     'BrowsingContextGroup.h',
     'CanonicalBrowsingContext.h',
     'ChildProcessChannelListener.h',
 ]
 
 UNIFIED_SOURCES += [
     'BrowsingContext.cpp',
     'BrowsingContextGroup.cpp',
--- a/dom/fetch/FetchDriver.cpp
+++ b/dom/fetch/FetchDriver.cpp
@@ -424,17 +424,17 @@ nsresult FetchDriver::HttpFetch(
   rv = NS_NewURI(getter_AddRefs(uri), url, nullptr, nullptr, ios);
   NS_ENSURE_SUCCESS(rv, rv);
 
   if (StaticPrefs::browser_tabs_remote_useCrossOriginPolicy()) {
     // Cross-Origin policy - bug 1525036
     nsILoadInfo::CrossOriginPolicy corsCredentials =
         nsILoadInfo::CROSS_ORIGIN_POLICY_NULL;
     if (mDocument && mDocument->GetBrowsingContext()) {
-      corsCredentials = mDocument->GetBrowsingContext()->CrossOriginPolicy();
+      corsCredentials = mDocument->GetBrowsingContext()->GetCrossOriginPolicy();
     }  // TODO Bug 1532287: else use mClientInfo
 
     if (mRequest->Mode() == RequestMode::No_cors &&
         corsCredentials != nsILoadInfo::CROSS_ORIGIN_POLICY_NULL) {
       mRequest->SetMode(RequestMode::Cors);
       mRequest->SetCredentialsMode(RequestCredentials::Same_origin);
       if (corsCredentials == nsILoadInfo::CROSS_ORIGIN_POLICY_USE_CREDENTIALS) {
         mRequest->SetCredentialsMode(RequestCredentials::Include);
--- a/dom/ipc/ContentChild.cpp
+++ b/dom/ipc/ContentChild.cpp
@@ -3789,17 +3789,17 @@ mozilla::ipc::IPCResult ContentChild::Re
 
   window->Dispatch(TaskCategory::Other, event.forget());
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult ContentChild::RecvCommitBrowsingContextTransaction(
     BrowsingContext* aContext, BrowsingContext::Transaction&& aTransaction) {
   if (aContext) {
-    aTransaction.Apply(aContext);
+    aTransaction.Apply(aContext, nullptr);
   }
   return IPC_OK();
 }
 
 }  // namespace dom
 
 #if defined(__OpenBSD__) && defined(MOZ_CONTENT_SANDBOX)
 #  include <unistd.h>
--- a/dom/ipc/ContentParent.cpp
+++ b/dom/ipc/ContentParent.cpp
@@ -5900,17 +5900,17 @@ mozilla::ipc::IPCResult ContentParent::R
     auto* entry = iter.Get();
     ContentParent* parent = entry->GetKey();
     if (parent != this) {
       Unused << parent->SendCommitBrowsingContextTransaction(aContext,
                                                              aTransaction);
     }
   }
 
-  aTransaction.Apply(aContext);
+  aTransaction.Apply(aContext, this);
 
   return IPC_OK();
 }
 }  // namespace dom
 }  // namespace mozilla
 
 NS_IMPL_ISUPPORTS(ParentIdleListener, nsIObserver)
 
--- a/netwerk/protocol/http/nsHttpChannel.cpp
+++ b/netwerk/protocol/http/nsHttpChannel.cpp
@@ -7362,17 +7362,17 @@ nsresult nsHttpChannel::ProcessCrossOrig
   } else {
     mLoadInfo->GetFrameBrowsingContext(getter_AddRefs(ctx));
   }
 
   if (!ctx) {
     return NS_OK;
   }
 
-  nsILoadInfo::CrossOriginPolicy documentPolicy = ctx->CrossOriginPolicy();
+  nsILoadInfo::CrossOriginPolicy documentPolicy = ctx->GetCrossOriginPolicy();
   nsILoadInfo::CrossOriginPolicy resultPolicy =
       nsILoadInfo::CROSS_ORIGIN_POLICY_NULL;
   rv = GetResponseCrossOriginPolicy(&resultPolicy);
   if (NS_FAILED(rv)) {
     return NS_OK;
   }
 
   ctx->SetCrossOriginPolicy(resultPolicy);