Bug 1539069 - Part 2: Use field epochs to avoid racy field interactions, r=farre
authorNika Layzell <nika@thelayzells.com>
Wed, 27 Mar 2019 09:19:29 +0000
changeset 466375 23b3ab9c9e1fee3424a45b13d52819c5677fa2f4
parent 466374 bc6af49dc687721993e8400a8c46820d83366276
child 466376 a6367a5c6020a947763bbbf9293491d045e671ba
push id35768
push useropoprus@mozilla.com
push dateThu, 28 Mar 2019 09:55:54 +0000
treeherdermozilla-central@c045dd97faf2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfarre
bugs1539069
milestone68.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 1539069 - Part 2: Use field epochs to avoid racy field interactions, r=farre Differential Revision: https://phabricator.services.mozilla.com/D24976
docshell/base/BrowsingContext.cpp
docshell/base/BrowsingContext.h
docshell/base/CanonicalBrowsingContext.cpp
docshell/base/CanonicalBrowsingContext.h
dom/ipc/ContentChild.cpp
dom/ipc/ContentChild.h
dom/ipc/ContentParent.cpp
dom/ipc/ContentParent.h
dom/ipc/PContent.ipdl
--- a/docshell/base/BrowsingContext.cpp
+++ b/docshell/base/BrowsingContext.cpp
@@ -681,33 +681,61 @@ void BrowsingContext::PostMessageMoz(JSC
                                      nsIPrincipal& aSubjectPrincipal,
                                      ErrorResult& aError) {
   PostMessageMoz(aCx, aMessage, aOptions.mTargetOrigin, aOptions.mTransfer,
                  aSubjectPrincipal, aError);
 }
 
 void BrowsingContext::Transaction::Commit(BrowsingContext* aBrowsingContext) {
   if (XRE_IsContentProcess()) {
+    // Increment the field epoch for fields affected by this transaction. We
+    // only need to do this in content.
+#define MOZ_BC_FIELD_RACY(name, ...)          \
+  if (m##name) {                              \
+    aBrowsingContext->mFieldEpochs.m##name++; \
+  }
+#define MOZ_BC_FIELD(...) /* nothing */
+#include "mozilla/dom/BrowsingContextFieldList.h"
+
     ContentChild* cc = ContentChild::GetSingleton();
-    cc->SendCommitBrowsingContextTransaction(aBrowsingContext, *this);
+    cc->SendCommitBrowsingContextTransaction(aBrowsingContext, *this,
+                                             aBrowsingContext->mFieldEpochs);
   } else {
     MOZ_DIAGNOSTIC_ASSERT(XRE_IsParentProcess());
     for (auto iter = aBrowsingContext->Group()->ContentParentsIter();
          !iter.Done(); iter.Next()) {
-      nsRefPtrHashKey<ContentParent>* entry = iter.Get();
-      Unused << entry->GetKey()->SendCommitBrowsingContextTransaction(
-          aBrowsingContext, *this);
+      RefPtr<ContentParent> child = iter.Get()->GetKey();
+      const FieldEpochs& childEpochs =
+          aBrowsingContext->Canonical()->GetFieldEpochsForChild(child);
+      Unused << child->SendCommitBrowsingContextTransaction(aBrowsingContext,
+                                                            *this, childEpochs);
     }
   }
 
   Apply(aBrowsingContext, nullptr);
 }
 
 void BrowsingContext::Transaction::Apply(BrowsingContext* aBrowsingContext,
-                                         ContentParent* aSource) {
+                                         ContentParent* aSource,
+                                         const FieldEpochs* aEpochs) {
+  // Filter out racy fields which have been updated in this process since this
+  // transaction was committed in the parent. This should only ever occur in the
+  // content process.
+  if (aEpochs) {
+    MOZ_ASSERT(XRE_IsContentProcess());
+#define MOZ_BC_FIELD_RACY(name, ...)                                 \
+  if (m##name) {                                                     \
+    if (aEpochs->m##name < aBrowsingContext->mFieldEpochs.m##name) { \
+      m##name.reset();                                               \
+    }                                                                \
+  }
+#define MOZ_BC_FIELD(...) /* nothing */
+#include "mozilla/dom/BrowsingContextFieldList.h"
+  }
+
 #define MOZ_BC_FIELD(name, ...)                         \
   if (m##name) {                                        \
     aBrowsingContext->WillSet##name(*m##name, aSource); \
     aBrowsingContext->m##name = std::move(*m##name);    \
     aBrowsingContext->DidSet##name(aSource);            \
     m##name.reset();                                    \
   }
 #include "mozilla/dom/BrowsingContextFieldList.h"
@@ -827,16 +855,37 @@ bool IPDLParamTraits<dom::BrowsingContex
   if (!ReadIPDLParam(aMessage, aIterator, aActor, &aTransaction->m##name)) { \
     return false;                                                            \
   }
 #include "mozilla/dom/BrowsingContextFieldList.h"
 
   return true;
 }
 
+void IPDLParamTraits<dom::BrowsingContext::FieldEpochs>::Write(
+    IPC::Message* aMessage, IProtocol* aActor,
+    const dom::BrowsingContext::FieldEpochs& aEpochs) {
+#define MOZ_BC_FIELD_RACY(name, ...) \
+  WriteIPDLParam(aMessage, aActor, aEpochs.m##name);
+#define MOZ_BC_FIELD(...) /* nothing */
+#include "mozilla/dom/BrowsingContextFieldList.h"
+}
+
+bool IPDLParamTraits<dom::BrowsingContext::FieldEpochs>::Read(
+    const IPC::Message* aMessage, PickleIterator* aIterator, IProtocol* aActor,
+    dom::BrowsingContext::FieldEpochs* aEpochs) {
+#define MOZ_BC_FIELD_RACY(name, ...)                                    \
+  if (!ReadIPDLParam(aMessage, aIterator, aActor, &aEpochs->m##name)) { \
+    return false;                                                       \
+  }
+#define MOZ_BC_FIELD(...) /* nothing */
+#include "mozilla/dom/BrowsingContextFieldList.h"
+  return true;
+}
+
 void IPDLParamTraits<dom::BrowsingContext::IPCInitializer>::Write(
     IPC::Message* aMessage, IProtocol* aActor,
     const dom::BrowsingContext::IPCInitializer& aInit) {
   // Write actor ID parameters.
   WriteIPDLParam(aMessage, aActor, aInit.mId);
   WriteIPDLParam(aMessage, aActor, aInit.mParentId);
 
   // Write other synchronized fields.
--- a/docshell/base/BrowsingContext.h
+++ b/docshell/base/BrowsingContext.h
@@ -245,16 +245,27 @@ class BrowsingContext : public nsWrapper
                       const WindowPostMessageOptions& aOptions,
                       nsIPrincipal& aSubjectPrincipal, ErrorResult& aError);
 
   JSObject* WrapObject(JSContext* aCx);
 
   void StartDelayedAutoplayMediaComponents();
 
   /**
+   * Each synced racy field in a BrowsingContext needs to have a epoch value
+   * which is used to resolve race conflicts by ensuring that only the last
+   * message received in the parent process wins.
+   */
+  struct FieldEpochs {
+#define MOZ_BC_FIELD(...) /* nothing */
+#define MOZ_BC_FIELD_RACY(name, ...) uint64_t m##name = 0;
+#include "mozilla/dom/BrowsingContextFieldList.h"
+  };
+
+  /**
    * 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.
@@ -262,17 +273,18 @@ class BrowsingContext : public nsWrapper
     // 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);
+    void Apply(BrowsingContext* aOwner, ContentParent* aSource,
+               const FieldEpochs* aEpochs = nullptr);
 
     bool HasNonRacyField() const {
 #define MOZ_BC_FIELD(name, ...) \
   if (m##name.isSome()) {       \
     return true;                \
   }
 #define MOZ_BC_FIELD_RACY(...) /* nothing */
 #include "mozilla/dom/BrowsingContextFieldList.h"
@@ -426,16 +438,18 @@ class BrowsingContext : public nsWrapper
 
   // 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;
 
+  FieldEpochs mFieldEpochs;
+
   // 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
@@ -445,16 +459,17 @@ class BrowsingContext : public nsWrapper
  * lives in this process, and a same-process WindowProxy should be used (see
  * nsGlobalWindowOuter). This should only be called by bindings code, ToJSValue
  * is the right API to get a WindowProxy for a BrowsingContext.
  */
 extern bool GetRemoteOuterWindowProxy(JSContext* aCx, BrowsingContext* aContext,
                                       JS::MutableHandle<JSObject*> aRetVal);
 
 typedef BrowsingContext::Transaction BrowsingContextTransaction;
+typedef BrowsingContext::FieldEpochs BrowsingContextFieldEpochs;
 typedef BrowsingContext::IPCInitializer BrowsingContextInitializer;
 
 }  // namespace dom
 
 // Allow sending BrowsingContext objects over IPC.
 namespace ipc {
 template <>
 struct IPDLParamTraits<dom::BrowsingContext> {
@@ -470,16 +485,26 @@ struct IPDLParamTraits<dom::BrowsingCont
                     const dom::BrowsingContext::Transaction& aTransaction);
 
   static bool Read(const IPC::Message* aMessage, PickleIterator* aIterator,
                    IProtocol* aActor,
                    dom::BrowsingContext::Transaction* aTransaction);
 };
 
 template <>
+struct IPDLParamTraits<dom::BrowsingContext::FieldEpochs> {
+  static void Write(IPC::Message* aMessage, IProtocol* aActor,
+                    const dom::BrowsingContext::FieldEpochs& aEpochs);
+
+  static bool Read(const IPC::Message* aMessage, PickleIterator* aIterator,
+                   IProtocol* aActor,
+                   dom::BrowsingContext::FieldEpochs* aEpochs);
+};
+
+template <>
 struct IPDLParamTraits<dom::BrowsingContext::IPCInitializer> {
   static void Write(IPC::Message* aMessage, IProtocol* aActor,
                     const dom::BrowsingContext::IPCInitializer& aInitializer);
 
   static bool Read(const IPC::Message* aMessage, PickleIterator* aIterator,
                    IProtocol* aActor,
                    dom::BrowsingContext::IPCInitializer* aInitializer);
 };
--- a/docshell/base/CanonicalBrowsingContext.cpp
+++ b/docshell/base/CanonicalBrowsingContext.cpp
@@ -143,10 +143,25 @@ void CanonicalBrowsingContext::NotifySta
   // Notfiy all content browsing contexts which are related with the canonical
   // browsing content tree to start delayed autoplay media.
   for (auto iter = Group()->ContentParentsIter(); !iter.Done(); iter.Next()) {
     auto entry = iter.Get();
     Unused << entry->GetKey()->SendStartDelayedAutoplayMediaComponents(this);
   }
 }
 
+void CanonicalBrowsingContext::SetFieldEpochsForChild(
+    ContentParent* aChild, const BrowsingContext::FieldEpochs& aEpochs) {
+  mChildFieldEpochs.Put(aChild->ChildID(), aEpochs);
+}
+
+const BrowsingContext::FieldEpochs&
+CanonicalBrowsingContext::GetFieldEpochsForChild(ContentParent* aChild) {
+  static const BrowsingContext::FieldEpochs sDefaultFieldEpochs;
+
+  if (auto entry = mChildFieldEpochs.Lookup(aChild->ChildID())) {
+    return entry.Data();
+  }
+  return sDefaultFieldEpochs;
+}
+
 }  // namespace dom
 }  // namespace mozilla
--- a/docshell/base/CanonicalBrowsingContext.h
+++ b/docshell/base/CanonicalBrowsingContext.h
@@ -66,16 +66,20 @@ class CanonicalBrowsingContext final : p
   // autoplay media.
   void NotifyStartDelayedAutoplayMedia();
 
   // Validate that the given process is allowed to perform the given
   // transaction. aSource is |nullptr| if set in the parent process.
   bool ValidateTransaction(const Transaction& aTransaction,
                            ContentParent* aSource);
 
+  void SetFieldEpochsForChild(ContentParent* aChild,
+                              const FieldEpochs& aEpochs);
+  const FieldEpochs& GetFieldEpochsForChild(ContentParent* aChild);
+
  protected:
   void Traverse(nsCycleCollectionTraversalCallback& cb);
   void Unlink();
 
   using Type = BrowsingContext::Type;
   CanonicalBrowsingContext(BrowsingContext* aParent,
                            BrowsingContextGroup* aGroup,
                            uint64_t aBrowsingContextId, uint64_t aProcessId,
@@ -86,14 +90,18 @@ class CanonicalBrowsingContext final : p
 
   // XXX(farre): Store a ContentParent pointer here rather than mProcessId?
   // Indicates which process owns the docshell.
   uint64_t mProcessId;
 
   // All live window globals within this browsing context.
   nsTHashtable<nsRefPtrHashKey<WindowGlobalParent>> mWindowGlobals;
   RefPtr<WindowGlobalParent> mCurrentWindowGlobal;
+
+  // Generation information for each content process which has interacted with
+  // this CanonicalBrowsingContext, by ChildID.
+  nsDataHashtable<nsUint64HashKey, FieldEpochs> mChildFieldEpochs;
 };
 
 }  // namespace dom
 }  // namespace mozilla
 
 #endif  // !defined(mozilla_dom_CanonicalBrowsingContext_h)
--- a/dom/ipc/ContentChild.cpp
+++ b/dom/ipc/ContentChild.cpp
@@ -3836,19 +3836,20 @@ mozilla::ipc::IPCResult ContentChild::Re
       aData.callerDocumentURI(), aData.isFromPrivateWindow());
   event->UnpackFrom(aMessage);
 
   window->Dispatch(TaskCategory::Other, event.forget());
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult ContentChild::RecvCommitBrowsingContextTransaction(
-    BrowsingContext* aContext, BrowsingContext::Transaction&& aTransaction) {
+    BrowsingContext* aContext, BrowsingContext::Transaction&& aTransaction,
+    BrowsingContext::FieldEpochs&& aEpochs) {
   if (aContext) {
-    aTransaction.Apply(aContext, nullptr);
+    aTransaction.Apply(aContext, nullptr, &aEpochs);
   }
   return IPC_OK();
 }
 
 }  // namespace dom
 
 #if defined(__OpenBSD__) && defined(MOZ_SANDBOX)
 #  include <unistd.h>
--- a/dom/ipc/ContentChild.h
+++ b/dom/ipc/ContentChild.h
@@ -732,17 +732,18 @@ class ContentChild final : public PConte
                                           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 RecvCommitBrowsingContextTransaction(
-      BrowsingContext* aContext, BrowsingContext::Transaction&& aTransaction);
+      BrowsingContext* aContext, BrowsingContext::Transaction&& aTransaction,
+      BrowsingContext::FieldEpochs&& aEpochs);
 
 #ifdef NIGHTLY_BUILD
   virtual PContentChild::Result OnMessageReceived(const Message& aMsg) override;
 #else
   using PContentChild::OnMessageReceived;
 #endif
 
   virtual PContentChild::Result OnMessageReceived(const Message& aMsg,
--- a/dom/ipc/ContentParent.cpp
+++ b/dom/ipc/ContentParent.cpp
@@ -5889,17 +5889,18 @@ void ContentParent::OnBrowsingContextGro
 
 void ContentParent::OnBrowsingContextGroupUnsubscribe(
     BrowsingContextGroup* aGroup) {
   MOZ_DIAGNOSTIC_ASSERT(aGroup);
   mGroups.RemoveEntry(aGroup);
 }
 
 mozilla::ipc::IPCResult ContentParent::RecvCommitBrowsingContextTransaction(
-    BrowsingContext* aContext, BrowsingContext::Transaction&& aTransaction) {
+    BrowsingContext* aContext, BrowsingContext::Transaction&& aTransaction,
+    BrowsingContext::FieldEpochs&& aEpochs) {
   if (!aContext) {
     MOZ_LOG(BrowsingContext::GetLog(), LogLevel::Warning,
             ("ParentIPC: Trying to run transaction on missing context."));
     return IPC_OK();
   }
 
   // Check if the transaction is valid.
   if (!aContext->Canonical()->ValidateTransaction(aTransaction, this)) {
@@ -5908,22 +5909,24 @@ mozilla::ipc::IPCResult ContentParent::R
     return IPC_FAIL_NO_REASON(this);
   }
 
   for (auto iter = aContext->Group()->ContentParentsIter(); !iter.Done();
        iter.Next()) {
     auto* entry = iter.Get();
     ContentParent* parent = entry->GetKey();
     if (parent != this) {
-      Unused << parent->SendCommitBrowsingContextTransaction(aContext,
-                                                             aTransaction);
+      Unused << parent->SendCommitBrowsingContextTransaction(
+          aContext, aTransaction,
+          aContext->Canonical()->GetFieldEpochsForChild(parent));
     }
   }
 
   aTransaction.Apply(aContext, this);
+  aContext->Canonical()->SetFieldEpochsForChild(this, aEpochs);
 
   return IPC_OK();
 }
 }  // namespace dom
 }  // namespace mozilla
 
 NS_IMPL_ISUPPORTS(ParentIdleListener, nsIObserver)
 
--- a/dom/ipc/ContentParent.h
+++ b/dom/ipc/ContentParent.h
@@ -1023,17 +1023,18 @@ class ContentParent final : public PCont
       const uint32_t& aColNumber, const uint32_t& aFlags,
       const nsCString& aCategory, const bool& aIsFromPrivateWindow,
       const ClonedMessageData* aStack = nullptr);
 
  public:
   mozilla::ipc::IPCResult RecvPrivateDocShellsExist(const bool& aExist);
 
   mozilla::ipc::IPCResult RecvCommitBrowsingContextTransaction(
-      BrowsingContext* aContext, BrowsingContext::Transaction&& aTransaction);
+      BrowsingContext* aContext, BrowsingContext::Transaction&& aTransaction,
+      BrowsingContext::FieldEpochs&& aEpochs);
 
   mozilla::ipc::IPCResult RecvFirstIdle();
 
   mozilla::ipc::IPCResult RecvDeviceReset();
 
   mozilla::ipc::IPCResult RecvKeywordToURI(const nsCString& aKeyword,
                                            nsString* aProviderName,
                                            RefPtr<nsIInputStream>* aPostData,
--- a/dom/ipc/PContent.ipdl
+++ b/dom/ipc/PContent.ipdl
@@ -94,16 +94,17 @@ using mozilla::Telemetry::KeyedScalarAct
 using mozilla::Telemetry::DynamicScalarDefinition from "mozilla/TelemetryComms.h";
 using mozilla::Telemetry::ChildEventData from "mozilla/TelemetryComms.h";
 using mozilla::Telemetry::DiscardedData from "mozilla/TelemetryComms.h";
 using mozilla::CrossProcessMutexHandle from "mozilla/ipc/CrossProcessMutex.h";
 using refcounted class nsIInputStream from "mozilla/ipc/IPCStreamUtils.h";
 using refcounted class mozilla::dom::BrowsingContext from "mozilla/dom/BrowsingContext.h";
 using mozilla::dom::BrowsingContextId from "mozilla/dom/ipc/IdType.h";
 using mozilla::dom::BrowsingContextTransaction from "mozilla/dom/BrowsingContext.h";
+using mozilla::dom::BrowsingContextFieldEpochs from "mozilla/dom/BrowsingContext.h";
 using mozilla::dom::BrowsingContextInitializer from "mozilla/dom/BrowsingContext.h";
 
 union ChromeRegistryItem
 {
     ChromePackage;
     OverrideMapping;
     SubstitutionMapping;
 };
@@ -1231,17 +1232,18 @@ parent:
                                                   nsCString aGrantedOrigin,
                                                   int aAllowMode)
           returns (bool unused);
 
     async StoreUserInteractionAsPermission(Principal aPrincipal);
 
 both:
     async CommitBrowsingContextTransaction(BrowsingContext aContext,
-                                           BrowsingContextTransaction aTransaction);
+                                           BrowsingContextTransaction aTransaction,
+                                           BrowsingContextFieldEpochs aEpochs);
 
     async AsyncMessage(nsString aMessage, CpowEntry[] aCpows,
                        Principal aPrincipal, ClonedMessageData aData);
 
     /**
      * Notify `push-subscription-modified` observers in the parent and child.
      */
     async NotifyPushSubscriptionModifiedObservers(nsCString scope,