Bug 1188721 - Part 21: Remove style struct ownership assertions. r?dbaron draft
authorCameron McCormack <cam@mcc.id.au>
Wed, 23 Mar 2016 17:36:00 +1100
changeset 343771 76dbb3c451ab4318b6ac434568b73b1b6c5a2215
parent 343770 1015dad10c71e7a263dddaed75b2b764a70c60a1
child 516824 fc5dc516550ffb81db9a8e35a240490e288b3e0d
push id13680
push usercmccormack@mozilla.com
push dateWed, 23 Mar 2016 06:36:18 +0000
reviewersdbaron
bugs1188721
milestone48.0a1
Bug 1188721 - Part 21: Remove style struct ownership assertions. r?dbaron MozReview-Commit-ID: JzJM2IqdGwy
layout/build/nsLayoutStatics.cpp
layout/style/crashtests/crashtests.list
layout/style/nsStyleContext.cpp
layout/style/nsStyleContext.h
modules/libpref/init/all.js
--- a/layout/build/nsLayoutStatics.cpp
+++ b/layout/build/nsLayoutStatics.cpp
@@ -304,17 +304,16 @@ nsLayoutStatics::Initialize()
 
   ServiceWorkerRegistrar::Initialize();
 
 #ifdef MOZ_B2G
   RequestSyncWifiService::Init();
 #endif
 
 #ifdef DEBUG
-  nsStyleContext::Initialize();
   mozilla::LayerAnimationInfo::Initialize();
 #endif
 
   MediaDecoder::InitStatics();
 
   PromiseDebugging::Init();
 
   layers::CompositorLRU::Init();
--- a/layout/style/crashtests/crashtests.list
+++ b/layout/style/crashtests/crashtests.list
@@ -111,18 +111,18 @@ load 945048-1.html
 load 972199-1.html
 load 989965-1.html
 load 992333-1.html
 pref(dom.webcomponents.enabled,true) load 1017798-1.html
 load 1028514-1.html
 load 1066089-1.html
 load 1074651-1.html
 pref(dom.webcomponents.enabled,true) load 1089463-1.html
-pref(layout.css.expensive-style-struct-assertions.enabled,true) load 1136010-1.html
-pref(layout.css.expensive-style-struct-assertions.enabled,true) load 1146101-1.html
+load 1136010-1.html
+load 1146101-1.html
 load 1153693-1.html
 load 1161320-1.html
 pref(dom.animations-api.core.enabled,true) load 1161320-2.html
 load 1161366-1.html
 load 1163446-1.html
 load 1164813-1.html
 load 1167782-1.html
 load 1186768-1.xhtml
--- a/layout/style/nsStyleContext.cpp
+++ b/layout/style/nsStyleContext.cpp
@@ -61,19 +61,16 @@ const uint32_t nsStyleContext::sDependen
 #define STYLE_STRUCT(name, checkdata_cb)
 #define STYLE_STRUCT_DEP(dep) NS_STYLE_INHERIT_BIT(dep) |
 #define STYLE_STRUCT_END() 0,
 #include "nsStyleStructList.h"
 #undef STYLE_STRUCT
 #undef STYLE_STRUCT_DEP
 #undef STYLE_STRUCT_END
 };
-
-// Whether to perform expensive assertions in the nsStyleContext destructor.
-static bool sExpensiveStyleStructAssertionsEnabled;
 #endif
 
 nsStyleContext::nsStyleContext(nsStyleContext* aParent,
                                nsIAtom* aPseudoTag,
                                CSSPseudoElementType aPseudoType,
                                nsRuleNode* aRuleNode,
                                bool aSkipParentDisplayBasedStyleFixup)
   : mParent(aParent)
@@ -146,34 +143,16 @@ nsStyleContext::~nsStyleContext()
   nsPresContext *presContext = mRuleNode->PresContext();
   nsStyleSet* styleSet = presContext->PresShell()->StyleSet()->GetAsGecko();
 
   NS_ASSERTION(!styleSet ||
                styleSet->GetRuleTree() == mRuleNode->RuleTree() ||
                styleSet->IsInRuleTreeReconstruct(),
                "destroying style context from old rule tree too late");
 
-#ifdef DEBUG
-  if (sExpensiveStyleStructAssertionsEnabled) {
-    // Assert that the style structs we are about to destroy are not referenced
-    // anywhere else in the style context tree.  These checks are expensive,
-    // which is why they are not enabled by default.
-    nsStyleContext* root = this;
-    while (root->mParent) {
-      root = root->mParent;
-    }
-    root->AssertStructsNotUsedElsewhere(this,
-                                        std::numeric_limits<int32_t>::max());
-  } else {
-    // In DEBUG builds when the pref is not enabled, we perform a more limited
-    // check just of the children of this style context.
-    AssertStructsNotUsedElsewhere(this, 2);
-  }
-#endif
-
   mRuleNode->Release();
 
   if (styleSet) {
     styleSet->NotifyStyleContextDestroyed(this);
   }
 
   if (mParent) {
     mParent->RemoveChild(this);
@@ -185,99 +164,16 @@ nsStyleContext::~nsStyleContext()
   if (mCachedResetData) {
     mCachedResetData->Destroy(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
-{
-  if (aLevels == 0) {
-    return;
-  }
-
-  nsStyleStruct* data;
-
-  if (mBits & NS_STYLE_IS_GOING_AWAY) {
-    return;
-  }
-
-  if (this != aDestroyingContext) {
-    nsInheritedStyleData& destroyingInheritedData =
-      aDestroyingContext->mCachedInheritedData;
-#define STYLE_STRUCT_INHERITED(name_, checkdata_cb)                            \
-    data = destroyingInheritedData.GetStyleData(eStyleStruct_##name_);         \
-    if (data &&                                                                \
-        !(aDestroyingContext->mBits & NS_STYLE_INHERIT_BIT(name_)) &&          \
-         (mCachedInheritedData.GetStyleData(eStyleStruct_##name_) == data)) {  \
-      printf_stderr("style struct %p found on style context %p\n", data, this);\
-      nsString url;                                                            \
-      PresContext()->Document()->GetURL(url);                                  \
-      printf_stderr("  in %s\n", NS_ConvertUTF16toUTF8(url).get());            \
-      MOZ_ASSERT(false, "destroying " #name_ " style struct still present "    \
-                        "in style context tree");                              \
-    }
-#define STYLE_STRUCT_RESET(name_, checkdata_cb)
-
-#include "nsStyleStructList.h"
-
-#undef STYLE_STRUCT_INHERITED
-#undef STYLE_STRUCT_RESET
-
-    if (mCachedResetData) {
-      nsResetStyleData* destroyingResetData =
-        aDestroyingContext->mCachedResetData;
-      if (destroyingResetData) {
-#define STYLE_STRUCT_INHERITED(name_, checkdata_cb_)
-#define STYLE_STRUCT_RESET(name_, checkdata_cb)                                \
-        data = destroyingResetData->GetStyleData(eStyleStruct_##name_);        \
-        if (data &&                                                            \
-            !(aDestroyingContext->mBits & NS_STYLE_INHERIT_BIT(name_)) &&      \
-            (mCachedResetData->GetStyleData(eStyleStruct_##name_) == data)) {  \
-          printf_stderr("style struct %p found on style context %p\n", data,   \
-                        this);                                                 \
-          nsString url;                                                        \
-          PresContext()->Document()->GetURL(url);                              \
-          printf_stderr("  in %s\n", NS_ConvertUTF16toUTF8(url).get());        \
-          MOZ_ASSERT(false, "destroying " #name_ " style struct still present "\
-                            "in style context tree");                          \
-        }
-
-#include "nsStyleStructList.h"
-
-#undef STYLE_STRUCT_INHERITED
-#undef STYLE_STRUCT_RESET
-      }
-    }
-  }
-
-  if (mChild) {
-    const nsStyleContext* child = mChild;
-    do {
-      child->AssertStructsNotUsedElsewhere(aDestroyingContext, aLevels - 1);
-      child = child->mNextSibling;
-    } while (child != mChild);
-  }
-
-  if (mEmptyChild) {
-    const nsStyleContext* child = mEmptyChild;
-    do {
-      child->AssertStructsNotUsedElsewhere(aDestroyingContext, aLevels - 1);
-      child = child->mNextSibling;
-    } while (child != mEmptyChild);
-  }
-}
-#endif
-
 void nsStyleContext::AddChild(nsStyleContext* aChild)
 {
   NS_ASSERTION(aChild->mPrevSibling == aChild &&
                aChild->mNextSibling == aChild,
                "child already in a child list");
 
   nsStyleContext **listPtr = aChild->mRuleNode->IsRoot() ? &mEmptyChild : &mChild;
   // Explicitly dereference listPtr so that compiler doesn't have to know that mNextSibling
@@ -1470,18 +1366,8 @@ nsStyleContext::LogStyleContextTree(bool
     nsStyleContext* child = mEmptyChild;
     do {
       child->LogStyleContextTree(false, aStructs);
       child = child->mNextSibling;
     } while (mEmptyChild != child);
   }
 }
 #endif
-
-#ifdef DEBUG
-/* static */ void
-nsStyleContext::Initialize()
-{
-  Preferences::AddBoolVarCache(
-      &sExpensiveStyleStructAssertionsEnabled,
-      "layout.css.expensive-style-struct-assertions.enabled");
-}
-#endif
--- a/layout/style/nsStyleContext.h
+++ b/layout/style/nsStyleContext.h
@@ -78,23 +78,16 @@ public:
 
   // These two methods are for use by ArenaRefPtr.
   static mozilla::ArenaObjectID ArenaObjectID()
   {
     return mozilla::eArenaObjectID_nsStyleContext;
   }
   nsIPresShell* Arena();
 
-#ifdef DEBUG
-  /**
-   * Initializes a cached pref, which is only used in DEBUG code.
-   */
-  static void Initialize();
-#endif
-
   nsrefcnt AddRef() {
     if (mRefCnt == UINT32_MAX) {
       NS_WARNING("refcount overflow, leaking object");
       return mRefCnt;
     }
     ++mRefCnt;
     NS_LOG_ADDREF(this, mRefCnt, "nsStyleContext", sizeof(nsStyleContext));
     return mRefCnt;
@@ -489,21 +482,16 @@ private:
       /* Have the rulenode deal */                                      \
       AUTO_CHECK_DEPENDENCY(eStyleStruct_##name_);                      \
       return mRuleNode->GetStyle##name_<aComputeData>(this);            \
     }
   #include "nsStyleStructList.h"
   #undef STYLE_STRUCT_RESET
   #undef STYLE_STRUCT_INHERITED
 
-#ifdef DEBUG
-  void AssertStructsNotUsedElsewhere(nsStyleContext* aDestroyingContext,
-                                     int32_t aLevels) const;
-#endif
-
 #ifdef RESTYLE_LOGGING
   void LogStyleContextTree(bool aFirst, uint32_t aStructs);
 
   // This only gets called under call trees where we've already checked
   // that PresContext()->RestyleManager()->ShouldLogRestyle() returned true.
   // It exists here just to satisfy LOG_RESTYLE's expectations.
   bool ShouldLogRestyle() { return true; }
 #endif
--- a/modules/libpref/init/all.js
+++ b/modules/libpref/init/all.js
@@ -4672,21 +4672,16 @@ pref("dom.w3c_touch_events.enabled", 2);
 pref("dom.w3c_pointer_events.enabled", false);
 
 // W3C draft ImageCapture API
 pref("dom.imagecapture.enabled", false);
 
 // W3C touch-action css property (related to touch and pointer events)
 pref("layout.css.touch_action.enabled", false);
 
-// Enables some assertions in nsStyleContext that are too expensive
-// for general use, but might be useful to enable for specific tests.
-// This only has an effect in DEBUG-builds.
-pref("layout.css.expensive-style-struct-assertions.enabled", false);
-
 // enable JS dump() function.
 pref("browser.dom.window.dump.enabled", false);
 
 #if defined(MOZ_WIDGET_GONK) || defined(MOZ_WIDGET_ANDROID)
 // Network Information API
 pref("dom.netinfo.enabled", true);
 #else
 pref("dom.netinfo.enabled", false);