Bug 1367904 - Part 13: stylo: Flatten ServoComputedValues into ServoStyleContext; r=bholley
authorManish Goregaokar <manishearth@gmail.com>
Mon, 17 Jul 2017 11:42:08 -0700
changeset 418019 c55df972f7c3ee0ec11e4b2841e20b7d0b1a0fd8
parent 418018 4d5e5d40c7ee04bba9fc8b96322a9aa10c220107
child 418020 bbad2cf92d8726f1f739045685a9159bfc75d687
push id7566
push usermtabara@mozilla.com
push dateWed, 02 Aug 2017 08:25:16 +0000
treeherdermozilla-beta@86913f512c3c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbholley
bugs1367904
milestone56.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 1367904 - Part 13: stylo: Flatten ServoComputedValues into ServoStyleContext; r=bholley This patch also removes the duplication of style contexts during the restyle, because otherwise pointer equality of ServoComputedValues stops holding (and we assert on that in a few places) MozReview-Commit-ID: 7Evc1p8ZfM2
layout/base/ServoRestyleManager.cpp
layout/style/ServoArcTypeList.h
layout/style/ServoBindingTypes.h
layout/style/ServoBindings.cpp
layout/style/ServoBindings.h
layout/style/ServoBindings.toml
layout/style/ServoStyleContext.cpp
layout/style/ServoStyleContext.h
layout/style/ServoStyleSet.cpp
layout/style/ServoTypes.h
layout/style/nsStyleContext.h
layout/style/nsStyleContextInlines.h
--- a/layout/base/ServoRestyleManager.cpp
+++ b/layout/base/ServoRestyleManager.cpp
@@ -549,26 +549,18 @@ ServoRestyleManager::ProcessPostTraversa
 
   RefPtr<ServoStyleContext> newContext = nullptr;
   if (wasRestyled && oldStyleContext) {
     MOZ_ASSERT(styleFrame || displayContentsNode);
     RefPtr<ServoStyleContext> currentContext =
       aRestyleState.StyleSet().ResolveServoStyle(aElement);
     MOZ_ASSERT(oldStyleContext->ComputedValues() != currentContext->ComputedValues());
 
-    auto pseudo = aElement->GetPseudoElementType();
-    nsIAtom* pseudoTag = pseudo == CSSPseudoElementType::NotPseudo
-      ? nullptr : nsCSSPseudoElements::GetPseudoAtom(pseudo);
 
-    // XXXManishearth we should just reuse the old one here
-    RefPtr<ServoStyleContext> tempContext =
-      Servo_StyleContext_NewContext(currentContext->ComputedValues(), aParentContext,
-                                    PresContext(), pseudo, pseudoTag).Consume();
-    newContext = aRestyleState.StyleSet().GetContext(tempContext.forget(), aParentContext,
-                                                     pseudoTag, pseudo, aElement);
+    newContext = currentContext;
 
     newContext->ResolveSameStructsAs(PresContext(), oldStyleContext);
 
     // We want to walk all the continuations here, even the ones with different
     // styles.  In practice, the only reason we get continuations with different
     // styles here is ::first-line (::first-letter never affects element
     // styles).  But in that case, newContext is the right context for the
     // _later_ continuations anyway (the ones not affected by ::first-line), not
--- a/layout/style/ServoArcTypeList.h
+++ b/layout/style/ServoArcTypeList.h
@@ -3,17 +3,16 @@
 /* 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/. */
 
 /* a list of all Servo Arc types used in stylo bindings for preprocessing */
 
 SERVO_ARC_TYPE(CssRules, ServoCssRules)
 SERVO_ARC_TYPE(StyleSheetContents, RawServoStyleSheetContents)
-SERVO_ARC_TYPE(ComputedValues, ServoComputedValues)
 SERVO_ARC_TYPE(DeclarationBlock, RawServoDeclarationBlock)
 SERVO_ARC_TYPE(StyleRule, RawServoStyleRule)
 SERVO_ARC_TYPE(ImportRule, RawServoImportRule)
 SERVO_ARC_TYPE(AnimationValue, RawServoAnimationValue)
 SERVO_ARC_TYPE(Keyframe, RawServoKeyframe)
 SERVO_ARC_TYPE(KeyframesRule, RawServoKeyframesRule)
 SERVO_ARC_TYPE(MediaList, RawServoMediaList)
 SERVO_ARC_TYPE(MediaRule, RawServoMediaRule)
--- a/layout/style/ServoBindingTypes.h
+++ b/layout/style/ServoBindingTypes.h
@@ -100,16 +100,19 @@ typedef mozilla::dom::StyleChildrenItera
     type_* mPtr;                             \
     already_AddRefed<type_> Consume();       \
   };
 #include "mozilla/ServoArcTypeList.h"
 #undef SERVO_ARC_TYPE
 
 typedef mozilla::ServoStyleContext const* ServoStyleContextBorrowed;
 typedef mozilla::ServoStyleContext const* ServoStyleContextBorrowedOrNull;
+typedef ServoComputedValues const* ServoComputedValuesBorrowed;
+typedef ServoComputedValues const* ServoComputedValuesBorrowedOrNull;
+
 struct MOZ_MUST_USE_TYPE ServoStyleContextStrong
 {
   mozilla::ServoStyleContext* mPtr;
   already_AddRefed<mozilla::ServoStyleContext> Consume();
 };
 
 #define DECL_OWNED_REF_TYPE_FOR(type_)    \
   typedef type_* type_##Owned;            \
--- a/layout/style/ServoBindings.cpp
+++ b/layout/style/ServoBindings.cpp
@@ -206,25 +206,30 @@ Gecko_DestroyAnonymousContentList(nsTArr
 {
   MOZ_ASSERT(aAnonContent);
   delete aAnonContent;
 }
 
 void
 Gecko_ServoStyleContext_Init(ServoStyleContext* aContext,
                              const ServoStyleContext* aParentContext,
-                             RawGeckoPresContextBorrowed aPresContext, ServoComputedValuesStrong aValues,
+                             RawGeckoPresContextBorrowed aPresContext, const ServoComputedValues* aValues,
                              mozilla::CSSPseudoElementType aPseudoType, nsIAtom* aPseudoTag)
 {
   // because it is within an Arc it is unsafe for the Rust side to ever
   // carry around a mutable non opaque reference to the context, so we
   // cast it here.
   ServoStyleContext* parent = const_cast<ServoStyleContext*>(aParentContext);
   nsPresContext* pres = const_cast<nsPresContext*>(aPresContext);
-  new (KnownNotNull, aContext) ServoStyleContext(parent, pres, aPseudoTag, aPseudoType, aValues.Consume());
+  new (KnownNotNull, aContext) ServoStyleContext(parent, pres, aPseudoTag,
+                                                 aPseudoType, ServoComputedValuesForgotten(aValues));
+}
+
+ServoComputedValues::ServoComputedValues(const ServoComputedValuesForgotten aValue) {
+  PodAssign(this, aValue.mPtr);
 }
 
 void
 Gecko_ServoStyleContext_Destroy(ServoStyleContext* aContext)
 {
   aContext->~ServoStyleContext();
 }
 
--- a/layout/style/ServoBindings.h
+++ b/layout/style/ServoBindings.h
@@ -138,17 +138,17 @@ bool Gecko_IsSignificantChild(RawGeckoNo
 RawGeckoNodeBorrowedOrNull Gecko_GetLastChild(RawGeckoNodeBorrowed node);
 RawGeckoNodeBorrowedOrNull Gecko_GetFlattenedTreeParentNode(RawGeckoNodeBorrowed node);
 RawGeckoElementBorrowedOrNull Gecko_GetBeforeOrAfterPseudo(RawGeckoElementBorrowed element, bool is_before);
 nsTArray<nsIContent*>* Gecko_GetAnonymousContentForElement(RawGeckoElementBorrowed element);
 void Gecko_DestroyAnonymousContentList(nsTArray<nsIContent*>* anon_content);
 
 void Gecko_ServoStyleContext_Init(mozilla::ServoStyleContext* context,
                                   ServoStyleContextBorrowedOrNull parent_context,
-                                  RawGeckoPresContextBorrowed pres_context, ServoComputedValuesStrong values,
+                                  RawGeckoPresContextBorrowed pres_context, ServoComputedValuesBorrowed values,
                                   mozilla::CSSPseudoElementType pseudo_type, nsIAtom* pseudo_tag);
 void Gecko_ServoStyleContext_Destroy(mozilla::ServoStyleContext* context);
 
 // By default, Servo walks the DOM by traversing the siblings of the DOM-view
 // first child. This generally works, but misses anonymous children, which we
 // want to traverse during styling. To support these cases, we create an
 // optional stack-allocated iterator in aIterator for nodes that need it.
 void Gecko_ConstructStyleChildrenIterator(RawGeckoElementBorrowed aElement,
--- a/layout/style/ServoBindings.toml
+++ b/layout/style/ServoBindings.toml
@@ -243,16 +243,18 @@ whitelist-types = [
     "nsTArray",
     "nsTArrayHeader",
     "Position",
     "PropertyValuePair",
     "Runnable",
     "ServoAttrSnapshot",
     "ServoBundledURI",
     "ServoComputedValues",
+    "ServoComputedValuesBorrowed",
+    "ServoComputedValuesBorrowedOrNull",
     "ServoElementSnapshot",
     "ServoStyleContextStrong",
     "ServoStyleContextBorrowed",
     "ServoStyleContextBorrowedOrNull",
     "SheetParsingMode",
     "StaticRefPtr",
     "StyleAnimation",
     "StyleBasicShape",
@@ -340,16 +342,18 @@ hide-types = [
 raw-lines = [
     "pub use nsstring::{nsACString, nsAString, nsString, nsStringRepr};",
     "use gecko_bindings::structs::nsStyleTransformMatrix;",
     "use gecko_bindings::structs::nsTArray;",
     "type nsACString_internal = nsACString;",
     "type nsAString_internal = nsAString;",
     "pub type ServoStyleContextBorrowed<'a> = &'a ServoStyleContext;",
     "pub type ServoStyleContextBorrowedOrNull<'a> = Option<&'a ::properties::ComputedValues>;",
+    "pub type ServoComputedValuesBorrowed<'a> = &'a ServoComputedValues;",
+    "pub type ServoComputedValuesBorrowedOrNull<'a> = Option<&'a ServoComputedValues>;",
 ]
 whitelist-functions = ["Servo_.*", "Gecko_.*"]
 structs-types = [
     "mozilla::css::GridTemplateAreasValue",
     "mozilla::css::ErrorReporter",
     "mozilla::css::ImageValue",
     "mozilla::css::URLValue",
     "mozilla::css::URLValueData",
--- a/layout/style/ServoStyleContext.cpp
+++ b/layout/style/ServoStyleContext.cpp
@@ -12,20 +12,20 @@
 #include "mozilla/ServoBindings.h"
 
 using namespace mozilla;
 
 ServoStyleContext::ServoStyleContext(nsStyleContext* aParent,
                                nsPresContext* aPresContext,
                                nsIAtom* aPseudoTag,
                                CSSPseudoElementType aPseudoType,
-                               already_AddRefed<ServoComputedValues> aComputedValues)
+                              ServoComputedValuesForgotten aComputedValues)
   : nsStyleContext(aParent, aPseudoTag, aPseudoType),
-  mSource(Move(aComputedValues))
+    mSource(aComputedValues)
 {
   mPresContext = aPresContext;
-  AddStyleBit(Servo_ComputedValues_GetStyleBits(mSource));
+  AddStyleBit(Servo_ComputedValues_GetStyleBits(&mSource));
 
   FinishConstruction();
 
   // No need to call ApplyStyleFixups here, since fixups are handled by Servo when
   // producing the ServoComputedValues.
 }
--- a/layout/style/ServoStyleContext.h
+++ b/layout/style/ServoStyleContext.h
@@ -12,24 +12,24 @@
 namespace mozilla {
 
 class ServoStyleContext final : public nsStyleContext {
 public:
   ServoStyleContext(nsStyleContext* aParent,
                     nsPresContext* aPresContext,
                     nsIAtom* aPseudoTag,
                     CSSPseudoElementType aPseudoType,
-                    already_AddRefed<ServoComputedValues> aComputedValues);
+                    ServoComputedValuesForgotten aComputedValues);
 
   nsPresContext* PresContext() const {
     return mPresContext;
   }
 
-  ServoComputedValues* ComputedValues() const {
-    return mSource;
+  const ServoComputedValues* ComputedValues() const {
+    return &mSource;
   }
 
   void AddRef() {
     Servo_StyleContext_AddRef(this);
   }
 
   void Release() {
     Servo_StyleContext_Release(this);
@@ -53,14 +53,14 @@ public:
   #include "nsStyleStructList.h"
   #undef STYLE_STRUCT
 
     mBits |= newBits;
   }
 
 private:
   nsPresContext* mPresContext;
-  RefPtr<ServoComputedValues> mSource;
+  ServoComputedValues mSource;
 };
 
 }
 
 #endif // mozilla_ServoStyleContext_h
--- a/layout/style/ServoStyleSet.cpp
+++ b/layout/style/ServoStyleSet.cpp
@@ -217,23 +217,22 @@ ServoStyleSet::GetContext(already_AddRef
   // we can precompute these and pass them as flags, similar to nsStyleSet.cpp.
   if (aElementForAnimation) {
     isLink = nsCSSRuleProcessor::IsLink(aElementForAnimation);
     isVisitedLink = nsCSSRuleProcessor::GetContentState(aElementForAnimation)
                                        .HasState(NS_EVENT_STATE_VISITED);
   }
 
   RefPtr<ServoStyleContext> result = Move(aComputedValues);
-  RefPtr<ServoComputedValues> computedValues = result->ComputedValues();
 
   MOZ_ASSERT(result->GetPseudoType() == aPseudoType);
   MOZ_ASSERT(result->GetPseudo() == aPseudoTag);
 
   RefPtr<ServoStyleContext> resultIfVisited =
-    Servo_ComputedValues_GetVisitedStyle(computedValues).Consume();
+    Servo_ComputedValues_GetVisitedStyle(result->ComputedValues()).Consume();
 
   // If `resultIfVisited` is non-null, then there was a relevant link and
   // visited styles were computed.  This corresponds to the cases where Gecko's
   // style system produces `aVisitedRuleNode`.
   // Set up `parentIfVisited` depending on whether our parent context has a
   // a visited style.  If it doesn't but we do have visited styles, use the
   // regular parent context for visited.
   nsStyleContext *parentIfVisited =
--- a/layout/style/ServoTypes.h
+++ b/layout/style/ServoTypes.h
@@ -167,25 +167,29 @@ struct ServoComputedValueFlags {
 #define STYLE_STRUCT_LIST_IGNORE_VARIABLES
 #include "nsStyleStructList.h"
 #undef STYLE_STRUCT
 #undef STYLE_STRUCT_LIST_IGNORE_VARIABLES
 
 } // namespace mozilla
 
 
+struct ServoComputedValues;
+struct ServoComputedValuesForgotten {
+  // Make sure you manually mem::forget the backing ServoComputedValues
+  // after calling this
+  explicit ServoComputedValuesForgotten(const ServoComputedValues* aValue) : mPtr(aValue) {}
+  const ServoComputedValues* mPtr;
+};
+
 /**
  * We want C++ to be abe to read the style struct fields of ComputedValues
  * so we define this type on the C++ side and use the bindgenned version
  * on the Rust side.
  *
- * C++ just sees pointers and opaque types here, so bindgen will attempt to generate a Copy
- * impl. This will fail because the bindgenned version contains owned types. Opt out.
- *
- * <div rustbindgen nocopy></div>
  */
 struct ServoComputedValues {
 #define STYLE_STRUCT(name_, checkdata_cb_) mozilla::ServoRawOffsetArc<mozilla::Gecko##name_> name_;
   #define STYLE_STRUCT_LIST_IGNORE_VARIABLES
 #include "nsStyleStructList.h"
 #undef STYLE_STRUCT
   #undef STYLE_STRUCT_LIST_IGNORE_VARIABLES
   mozilla::ServoCustomPropertiesMap custom_properties;
@@ -195,12 +199,28 @@ struct ServoComputedValues {
   /// node.  Can be None for default values and text nodes.  This is
   /// essentially an optimization to avoid referencing the root rule node.
   mozilla::ServoRuleNode rules;
   /// The element's computed values if visited, only computed if there's a
   /// relevant link for this element. A element's "relevant link" is the
   /// element being matched if it is a link or the nearest ancestor link.
   mozilla::ServoVisitedStyle visited_style;
   mozilla::ServoComputedValueFlags flags;
-  ~ServoComputedValues() {} // do nothing, but prevent Copy from being impl'd by bindgen
+
+  // C++ just sees this struct as a bucket of bits, and will
+  // do the wrong thing if we let it use the default copy ctor/assignment
+  // operator. Remove them so that there is no footgun.
+  //
+  // We remove the move ctor/assignment operator as well, because
+  // moves in C++ don't prevent destructors from being called,
+  // which will lead to double frees.
+  ServoComputedValues& operator=(const ServoComputedValues&) = delete;
+  ServoComputedValues(const ServoComputedValues&) = delete;
+  ServoComputedValues&& operator=(const ServoComputedValues&&) = delete;
+  ServoComputedValues(const ServoComputedValues&&) = delete;
+
+  // Constructs via memcpy. Will not invalidate old struct
+  explicit ServoComputedValues(const ServoComputedValuesForgotten aValue);
 };
 
+
+
 #endif // mozilla_ServoTypes_h
--- a/layout/style/nsStyleContext.h
+++ b/layout/style/nsStyleContext.h
@@ -217,17 +217,17 @@ public:
    * happen because it was inherited from the parent style context, or
    * because it was stored conditionally on the rule node.
    */
   bool HasCachedDependentStyleData(nsStyleStructID aSID) {
     return mBits & nsCachedStyleData::GetBitForSID(aSID);
   }
 
   inline nsRuleNode* RuleNode();
-  inline ServoComputedValues* ComputedValues();
+  inline const ServoComputedValues* ComputedValues();
 
   void AddStyleBit(const uint64_t& aBit) { mBits |= aBit; }
 
   /**
    * Define typesafe getter functions for each style struct by
    * preprocessing the list of style structs.  These functions are the
    * preferred way to get style data.  The macro creates functions like:
    *   const nsStyleBorder* StyleBorder();
--- a/layout/style/nsStyleContextInlines.h
+++ b/layout/style/nsStyleContextInlines.h
@@ -25,17 +25,17 @@ MOZ_DEFINE_STYLO_METHODS(nsStyleContext,
 
 nsRuleNode*
 nsStyleContext::RuleNode()
 {
     MOZ_RELEASE_ASSERT(IsGecko());
     return AsGecko()->RuleNode();
 }
 
-ServoComputedValues*
+const ServoComputedValues*
 nsStyleContext::ComputedValues()
 {
     MOZ_RELEASE_ASSERT(IsServo());
     return AsServo()->ComputedValues();
 }
 
 void
 nsStyleContext::AddRef()