Backed out 3 changesets (bug 1181907) for marionette bustage a=backout
authorWes Kocher <wkocher@mozilla.com>
Wed, 09 Sep 2015 15:15:08 -0700
changeset 289179 ada03589e5eae255a91c33611b0f5a11e7e81df3
parent 289178 c3fec00fc2bd05638483035bb303ec9602176b56
child 289180 1850357479bb1421807e830238ca8fc97bb20b39
push id5067
push userraliiev@mozilla.com
push dateMon, 21 Sep 2015 14:04:52 +0000
treeherdermozilla-beta@14221ffe5b2f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbackout
bugs1181907
milestone42.0a2
backs out77513059a6e4425a19d1c803fcb8225522c375d6
97526d372199ca11d57c45959b853cc448a25dbd
4929041d9806546e5d20f498f02248d784b7309e
Backed out 3 changesets (bug 1181907) for marionette bustage a=backout Backed out changeset 77513059a6e4 (bug 1181907) Backed out changeset 97526d372199 (bug 1181907) Backed out changeset 4929041d9806 (bug 1181907)
layout/style/CSSVariableImageTable.h
layout/style/nsCSSDataBlock.cpp
layout/style/nsCSSProperty.h
layout/style/nsCSSValue.h
layout/style/nsStyleContext.cpp
xpcom/glue/nsHashKeys.h
deleted file mode 100644
--- a/layout/style/CSSVariableImageTable.h
+++ /dev/null
@@ -1,176 +0,0 @@
-/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
-/* 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 global table that tracks images referenced by CSS variables. */
-
-#ifndef mozilla_CSSVariableImageTable_h
-#define mozilla_CSSVariableImageTable_h
-
-#include "nsClassHashtable.h"
-#include "nsCSSProperty.h"
-#include "nsCSSValue.h"
-#include "nsStyleContext.h"
-#include "nsTArray.h"
-
-/**
- * CSSVariableImageTable maintains a global mapping
- *   (nsStyleContext, nsCSSProperty) -> nsTArray<ImageValue>
- * which allows us to track the relationship between CSS property values
- * involving variables and any images they may reference.
- *
- * When properties like background-image contain a normal url(), the
- * Declaration's data block will hold a reference to the ImageValue.  When a
- * token stream is used, the Declaration only holds on to an
- * nsCSSValueTokenStream object, and the ImageValue would only exist for the
- * duration of nsRuleNode::WalkRuleTree, in the AutoCSSValueArray.  So instead
- * when we re-parse a token stream and get an ImageValue, we record it in the
- * CSSVariableImageTable to keep the ImageValue alive. Such ImageValues are
- * eventually freed the next time the token stream is re-parsed, or when the
- * associated style context is destroyed.
- *
- * To add ImageValues to the CSSVariableImageTable, callers should pass a lambda
- * to CSSVariableImageTable::ReplaceAll() that calls
- * CSSVariableImageTable::Add() for each ImageValue that needs to be added to
- * the table. When callers are sure that the ImageValues for a given
- * nsStyleContext won't be needed anymore, they can call
- * CSSVariableImageTable::RemoveAll() to release them.
- */
-
-namespace mozilla {
-namespace CSSVariableImageTable {
-
-namespace detail {
-
-typedef nsTArray<nsRefPtr<css::ImageValue>> ImageValueArray;
-typedef nsClassHashtable<nsGenericHashKey<nsCSSProperty>, ImageValueArray>
-        PerPropertyImageHashtable;
-typedef nsClassHashtable<nsPtrHashKey<nsStyleContext>, PerPropertyImageHashtable>
-        CSSVariableImageHashtable;
-
-inline CSSVariableImageHashtable& GetTable()
-{
-  static CSSVariableImageHashtable imageTable;
-  return imageTable;
-}
-
-#ifdef DEBUG
-inline bool& IsReplacing()
-{
-  static bool isReplacing = false;
-  return isReplacing;
-}
-#endif
-
-} // namespace detail
-
-/**
- * ReplaceAll() allows callers to replace the ImageValues associated with a
- * (nsStyleContext, nsCSSProperty) pair. The memory used by the previous list of
- * ImageValues is automatically released.
- *
- * @param aContext The style context the ImageValues are associated with.
- * @param aProp    The CSS property the ImageValues are associated with.
- * @param aFunc    A lambda that calls CSSVariableImageTable::Add() to add new
- *                 ImageValues which will replace the old ones.
- */
-template <typename Lambda>
-inline void ReplaceAll(nsStyleContext* aContext,
-                       nsCSSProperty aProp,
-                       Lambda aFunc)
-{
-  MOZ_ASSERT(aContext);
-
-  auto& imageTable = detail::GetTable();
-
-  // Clear the existing image array, if any, for this property.
-  {
-    auto* perPropertyImageTable = imageTable.Get(aContext);
-    auto* imageList = perPropertyImageTable ? perPropertyImageTable->Get(aProp)
-                                            : nullptr;
-    if (imageList) {
-      imageList->ClearAndRetainStorage();
-    }
-  }
-
-#ifdef DEBUG
-  MOZ_ASSERT(!detail::IsReplacing());
-  detail::IsReplacing() = true;
-#endif
-
-  aFunc();
-
-#ifdef DEBUG
-  detail::IsReplacing() = false;
-#endif
-
-  // Clean up.
-  auto* perPropertyImageTable = imageTable.Get(aContext);
-  auto* imageList = perPropertyImageTable ? perPropertyImageTable->Get(aProp)
-                                          : nullptr;
-  if (imageList) {
-    if (imageList->IsEmpty()) {
-      // We used to have an image array for this property, but now we don't.
-      // Remove the entry in the per-property image table for this property.
-      // That may then allow us to remove the entire per-property image table.
-      perPropertyImageTable->Remove(aProp);
-      if (perPropertyImageTable->Count() == 0) {
-        imageTable.Remove(aContext);
-      }
-    } else {
-      // We still have a non-empty image array for this property. Compact the
-      // storage it's using if possible.
-      imageList->Compact();
-    }
-  }
-}
-
-/**
- * Adds a new ImageValue @aValue to the CSSVariableImageTable, which will be
- * associated with @aContext and @aProp.
- *
- * It's illegal to call this function outside of a lambda passed to
- * CSSVariableImageTable::ReplaceAll().
- */
-inline void
-Add(nsStyleContext* aContext, nsCSSProperty aProp, css::ImageValue* aValue)
-{
-  MOZ_ASSERT(aValue);
-  MOZ_ASSERT(aContext);
-  MOZ_ASSERT(detail::IsReplacing());
-
-  auto& imageTable = detail::GetTable();
-
-  // Ensure there's a per-property image table for this style context.
-  auto* perPropertyImageTable = imageTable.Get(aContext);
-  if (!perPropertyImageTable) {
-    perPropertyImageTable = new detail::PerPropertyImageHashtable();
-    imageTable.Put(aContext, perPropertyImageTable);
-  }
-
-  // Ensure there's an image array for this property.
-  auto* imageList = perPropertyImageTable->Get(aProp);
-  if (!imageList) {
-    imageList = new detail::ImageValueArray();
-    perPropertyImageTable->Put(aProp, imageList);
-  }
-
-  // Append the provided ImageValue to the list.
-  imageList->AppendElement(aValue);
-}
-
-/**
- * Removes all ImageValues stored in the CSSVariableImageTable for the provided
- * @aContext.
- */
-inline void
-RemoveAll(nsStyleContext* aContext)
-{
-  detail::GetTable().Remove(aContext);
-}
-
-} // namespace CSSVariableImageTable
-} // namespace mozilla
-
-#endif // mozilla_CSSVariableImageTable_h
--- a/layout/style/nsCSSDataBlock.cpp
+++ b/layout/style/nsCSSDataBlock.cpp
@@ -5,28 +5,26 @@
 
 /*
  * compact representation of the property-value pairs within a CSS
  * declaration, and the code for expanding and compacting it
  */
 
 #include "nsCSSDataBlock.h"
 
-#include "CSSVariableImageTable.h"
 #include "mozilla/css/Declaration.h"
 #include "mozilla/css/ImageLoader.h"
 #include "mozilla/MemoryReporting.h"
 #include "mozilla/WritingModes.h"
 #include "nsIDocument.h"
 #include "nsRuleData.h"
 #include "nsStyleContext.h"
 #include "nsStyleSet.h"
 
 using namespace mozilla;
-using namespace mozilla::css;
 
 /**
  * Does a fast move of aSource to aDest.  The previous value in
  * aDest is cleanly destroyed, and aSource is cleared.  Returns
  * true if, before the copy, the value at aSource compared unequal
  * to the value at aDest; false otherwise.
  */
 static bool
@@ -48,69 +46,64 @@ ShouldIgnoreColors(nsRuleData *aRuleData
 }
 
 /**
  * Tries to call |nsCSSValue::StartImageLoad()| on an image source.
  * Image sources are specified by |url()| or |-moz-image-rect()| function.
  */
 static void
 TryToStartImageLoadOnValue(const nsCSSValue& aValue, nsIDocument* aDocument,
-                           nsStyleContext* aContext, nsCSSProperty aProperty,
-                           bool aForTokenStream)
+                           nsCSSValueTokenStream* aTokenStream)
 {
   MOZ_ASSERT(aDocument);
 
   if (aValue.GetUnit() == eCSSUnit_URL) {
     aValue.StartImageLoad(aDocument);
-    if (aForTokenStream && aContext) {
-      CSSVariableImageTable::Add(aContext, aProperty,
-                                 aValue.GetImageStructValue());
+    if (aTokenStream) {
+      aTokenStream->mImageValues.PutEntry(aValue.GetImageStructValue());
     }
   }
   else if (aValue.GetUnit() == eCSSUnit_Image) {
     // If we already have a request, see if this document needs to clone it.
     imgIRequest* request = aValue.GetImageValue(nullptr);
 
     if (request) {
-      ImageValue* imageValue = aValue.GetImageStructValue();
+      mozilla::css::ImageValue* imageValue = aValue.GetImageStructValue();
       aDocument->StyleImageLoader()->MaybeRegisterCSSImage(imageValue);
-      if (aForTokenStream && aContext) {
-        CSSVariableImageTable::Add(aContext, aProperty, imageValue);
+      if (aTokenStream) {
+        aTokenStream->mImageValues.PutEntry(imageValue);
       }
     }
   }
   else if (aValue.EqualsFunction(eCSSKeyword__moz_image_rect)) {
     nsCSSValue::Array* arguments = aValue.GetArrayValue();
     MOZ_ASSERT(arguments->Count() == 6, "unexpected num of arguments");
 
     const nsCSSValue& image = arguments->Item(1);
-    TryToStartImageLoadOnValue(image, aDocument, aContext, aProperty,
-                               aForTokenStream);
+    TryToStartImageLoadOnValue(image, aDocument, aTokenStream);
   }
 }
 
 static void
 TryToStartImageLoad(const nsCSSValue& aValue, nsIDocument* aDocument,
-                    nsStyleContext* aContext, nsCSSProperty aProperty,
-                    bool aForTokenStream)
+                    nsCSSProperty aProperty,
+                    nsCSSValueTokenStream* aTokenStream)
 {
   if (aValue.GetUnit() == eCSSUnit_List) {
     for (const nsCSSValueList* l = aValue.GetListValue(); l; l = l->mNext) {
-      TryToStartImageLoad(l->mValue, aDocument, aContext, aProperty,
-                          aForTokenStream);
+      TryToStartImageLoad(l->mValue, aDocument, aProperty, aTokenStream);
     }
   } else if (nsCSSProps::PropHasFlags(aProperty,
                                       CSS_PROPERTY_IMAGE_IS_IN_ARRAY_0)) {
     if (aValue.GetUnit() == eCSSUnit_Array) {
       TryToStartImageLoadOnValue(aValue.GetArrayValue()->Item(0), aDocument,
-                                 aContext, aProperty, aForTokenStream);
+                                 aTokenStream);
     }
   } else {
-    TryToStartImageLoadOnValue(aValue, aDocument, aContext, aProperty,
-                               aForTokenStream);
+    TryToStartImageLoadOnValue(aValue, aDocument, aTokenStream);
   }
 }
 
 static inline bool
 ShouldStartImageLoads(nsRuleData *aRuleData, nsCSSProperty aProperty)
 {
   // Don't initiate image loads for if-visited styles.  This is
   // important because:
@@ -131,27 +124,32 @@ MapSinglePropertyInto(nsCSSProperty aPro
 {
     MOZ_ASSERT(aValue->GetUnit() != eCSSUnit_Null, "oops");
 
     // Although aTarget is the nsCSSValue we are going to write into,
     // we also look at its value before writing into it.  This is done
     // when aTarget is a token stream value, which is the case when we
     // have just re-parsed a property that had a variable reference (in
     // nsCSSParser::ParsePropertyWithVariableReferences).  TryToStartImageLoad
-    // then records any resulting ImageValue objects in the
-    // CSSVariableImageTable, to give them the appropriate lifetime.
+    // then records any resulting ImageValue objects on the
+    // nsCSSValueTokenStream object we found on aTarget.  See the comment
+    // above nsCSSValueTokenStream::mImageValues for why.
     MOZ_ASSERT(aTarget->GetUnit() == eCSSUnit_TokenStream ||
                aTarget->GetUnit() == eCSSUnit_Null,
                "aTarget must only be a token stream (when re-parsing "
                "properties with variable references) or null");
 
+    nsCSSValueTokenStream* tokenStream =
+        aTarget->GetUnit() == eCSSUnit_TokenStream ?
+            aTarget->GetTokenStreamValue() :
+            nullptr;
+
     if (ShouldStartImageLoads(aRuleData, aProp)) {
         nsIDocument* doc = aRuleData->mPresContext->Document();
-        TryToStartImageLoad(*aValue, doc, aRuleData->mStyleContext, aProp,
-                            aTarget->GetUnit() == eCSSUnit_TokenStream);
+        TryToStartImageLoad(*aValue, doc, aProp, tokenStream);
     }
     *aTarget = *aValue;
     if (nsCSSProps::PropHasFlags(aProp,
             CSS_PROPERTY_IGNORED_WHEN_COLORS_DISABLED) &&
         ShouldIgnoreColors(aRuleData))
     {
         if (aProp == eCSSProperty_background_color) {
             // Force non-'transparent' background
@@ -699,19 +697,17 @@ nsCSSExpandedDataBlock::MapRuleInfoInto(
     uint8_t wm = WritingMode(aRuleData->mStyleContext).GetBits();
     aRuleData->mConditions.SetWritingModeDependency(wm);
   }
 
   nsCSSValue* dest = aRuleData->ValueFor(physicalProp);
   MOZ_ASSERT(dest->GetUnit() == eCSSUnit_TokenStream &&
              dest->GetTokenStreamValue()->mPropertyID == aPropID);
 
-  CSSVariableImageTable::ReplaceAll(aRuleData->mStyleContext, aPropID, [=] {
-    MapSinglePropertyInto(physicalProp, src, dest, aRuleData);
-  });
+  MapSinglePropertyInto(physicalProp, src, dest, aRuleData);
 }
 
 #ifdef DEBUG
 void
 nsCSSExpandedDataBlock::DoAssertInitialState()
 {
     mPropertiesSet.AssertIsEmpty("not initial state");
     mPropertiesImportant.AssertIsEmpty("not initial state");
--- a/layout/style/nsCSSProperty.h
+++ b/layout/style/nsCSSProperty.h
@@ -3,18 +3,16 @@
  * 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/. */
 
 /* enum types for CSS properties and their values */
  
 #ifndef nsCSSProperty_h___
 #define nsCSSProperty_h___
 
-#include <nsHashKeys.h>
-
 /*
    Declare the enum list using the magic of preprocessing
    enum values are "eCSSProperty_foo" (where foo is the property)
 
    To change the list of properties, see nsCSSPropList.h
 
  */
 enum nsCSSProperty {
@@ -61,27 +59,16 @@ enum nsCSSProperty {
   // Extra dummy values for nsCSSParser internal use.
   eCSSPropertyExtra_x_none_value,
   eCSSPropertyExtra_x_auto_value,
 
   // Extra value to represent custom properties (--*).
   eCSSPropertyExtra_variable
 };
 
-namespace mozilla {
-
-template<>
-inline PLDHashNumber
-Hash<nsCSSProperty>(const nsCSSProperty& aValue)
-{
-  return uint32_t(aValue);
-}
-
-} // namespace mozilla
-
 // The "descriptors" that can appear in a @font-face rule.
 // They have the syntax of properties but different value rules.
 enum nsCSSFontDesc {
   eCSSFontDesc_UNKNOWN = -1,
 #define CSS_FONT_DESC(name_, method_) eCSSFontDesc_##method_,
 #include "nsCSSFontDescList.h"
 #undef CSS_FONT_DESC
   eCSSFontDesc_COUNT
--- a/layout/style/nsCSSValue.h
+++ b/layout/style/nsCSSValue.h
@@ -1537,16 +1537,28 @@ public:
   nsCOMPtr<nsIURI> mBaseURI;
   nsCOMPtr<nsIURI> mSheetURI;
   nsCOMPtr<nsIPrincipal> mSheetPrincipal;
   // XXX Should store sheet here (see Bug 952338)
   // mozilla::CSSStyleSheet* mSheet;
   uint32_t mLineNumber;
   uint32_t mLineOffset;
 
+  // This table is used to hold a reference on to any ImageValue that results
+  // from re-parsing this token stream at computed value time.  When properties
+  // like background-image contain a normal url(), the Declaration's data block
+  // will hold a reference to the ImageValue.  When a token stream is used,
+  // the Declaration only holds on to this nsCSSValueTokenStream object, and
+  // the ImageValue would only exist for the duration of
+  // nsRuleNode::WalkRuleTree, in the AutoCSSValueArray.  So instead when
+  // we re-parse a token stream and get an ImageValue, we record it in this
+  // table so that the Declaration can be the object that keeps holding
+  // a reference to it.
+  nsTHashtable<nsRefPtrHashKey<mozilla::css::ImageValue> > mImageValues;
+
 private:
   nsCSSValueTokenStream(const nsCSSValueTokenStream& aOther) = delete;
   nsCSSValueTokenStream& operator=(const nsCSSValueTokenStream& aOther) = delete;
 };
 
 class nsCSSValueFloatColor final {
 public:
   nsCSSValueFloatColor(float aComponent1, float aComponent2, float aComponent3,
--- a/layout/style/nsStyleContext.cpp
+++ b/layout/style/nsStyleContext.cpp
@@ -1,16 +1,15 @@
 /* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* 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/. */
  
 /* the interface (to internal code) for retrieving computed style data */
 
-#include "CSSVariableImageTable.h"
 #include "mozilla/DebugOnly.h"
 
 #include "nsCSSAnonBoxes.h"
 #include "nsStyleConsts.h"
 #include "nsString.h"
 #include "nsPresContext.h"
 #include "nsIStyleRule.h"
 
@@ -161,19 +160,16 @@ nsStyleContext::~nsStyleContext()
     mParent->Release();
   }
 
   // Free up our data structs.
   mCachedInheritedData.DestroyStructs(mBits, presContext);
   if (mCachedResetData) {
     mCachedResetData->Destroy(mBits, presContext);
   }
-
-  // Free any ImageValues we were holding on to for CSS variable values.
-  CSSVariableImageTable::RemoveAll(this);
 }
 
 #ifdef DEBUG
 void
 nsStyleContext::AssertStructsNotUsedElsewhere(
                                        nsStyleContext* aDestroyingContext,
                                        int32_t aLevels) const
 {
--- a/xpcom/glue/nsHashKeys.h
+++ b/xpcom/glue/nsHashKeys.h
@@ -646,27 +646,16 @@ public:
   }
 
   enum { ALLOW_MEMMOVE = true };
 
 private:
   nsCOMPtr<nsIHashable> mKey;
 };
 
-namespace mozilla {
-
-template <typename T>
-PLDHashNumber
-Hash(const T& aValue)
-{
-  return aValue.Hash();
-}
-
-} // namespace mozilla
-
 /**
  * Hashtable key class to use with objects for which Hash() and operator==()
  * are defined.
  */
 template<typename T>
 class nsGenericHashKey : public PLDHashEntryHdr
 {
 public:
@@ -675,16 +664,16 @@ public:
 
   explicit nsGenericHashKey(KeyTypePointer aKey) : mKey(*aKey) {}
   nsGenericHashKey(const nsGenericHashKey<T>& aOther) : mKey(aOther.mKey) {}
 
   KeyType GetKey() const { return mKey; }
   bool KeyEquals(KeyTypePointer aKey) const { return *aKey == mKey; }
 
   static KeyTypePointer KeyToPointer(KeyType aKey) { return &aKey; }
-  static PLDHashNumber HashKey(KeyTypePointer aKey) { return ::mozilla::Hash(*aKey); }
+  static PLDHashNumber HashKey(KeyTypePointer aKey) { return aKey->Hash(); }
   enum { ALLOW_MEMMOVE = true };
 
 private:
   T mKey;
 };
 
 #endif // nsTHashKeys_h__