Backed out 2 changesets (bug 1480146) for causing talos perf reftest to timeout in link-style-cache-1.html

Backed out changeset ff3ba7a16c21 (bug 1480146)
Backed out changeset f47314ec33ff (bug 1480146)

layout/style/Loader.cpp

@@ -461,32 +461,11 @@ struct Loader::Sheets {
   // The SheetLoadData pointers in mLoadingDatas below are weak references.
   nsDataHashtable<SheetLoadDataHashKey, SheetLoadData*> mLoadingDatas;
 
-  nsRefPtrHashtable<nsStringHashKey, StyleSheet> mInlineSheets;
-
-
-  RefPtr<StyleSheet> LookupInline(const nsAString&);
-
   // A cache hit or miss. It is a miss if the `StyleSheet` is null.
   using CacheResult = Tuple<RefPtr<StyleSheet>, SheetState>;
   CacheResult Lookup(SheetLoadDataHashKey&, bool aSyncLoad);
-
-  size_t SizeOfIncludingThis(MallocSizeOf) const;
 };
 
-RefPtr<StyleSheet> Loader::Sheets::LookupInline(const nsAString& aBuffer) {
-  auto result = mInlineSheets.Lookup(aBuffer);
-  if (!result) {
-    return nullptr;
-  }
-  if (result.Data()->HasForcedUniqueInner()) {
-    // Remove it now that we know that we're never going to use this stylesheet
-    // again.
-    result.Remove();
-    return nullptr;
-  }
-  return result.Data()->Clone(nullptr, nullptr, nullptr, nullptr);
-}
-
 static void AssertComplete(const StyleSheet& aSheet) {
   // This sheet came from the XUL cache or our per-document hashtable; it
   // better be a complete sheet.
@@ -580,39 +559,6 @@ auto Loader::Sheets::Lookup(SheetLoadDataHashKey& aKey, bool aSyncLoad)
   return {};
 }
 
-size_t Loader::Sheets::SizeOfIncludingThis(MallocSizeOf aMallocSizeOf) const {
-  size_t n = aMallocSizeOf(this);
-
-  n += mCompleteSheets.ShallowSizeOfExcludingThis(aMallocSizeOf);
-  for (auto iter = mCompleteSheets.ConstIter(); !iter.Done(); iter.Next()) {
-    // If the sheet has a parent, then its parent will report it so we don't
-    // have to worry about it here. Likewise, if aSheet has an owning node, then
-    // the document that node is in will report it.
-    const StyleSheet* sheet = iter.UserData();
-    if (!sheet->GetOwnerNode() && !sheet->GetParentSheet()) {
-      n += sheet->SizeOfIncludingThis(aMallocSizeOf);
-    }
-  }
-
-  n += mInlineSheets.ShallowSizeOfExcludingThis(aMallocSizeOf);
-  for (auto iter = mInlineSheets.ConstIter(); !iter.Done(); iter.Next()) {
-    n += iter.Key().SizeOfExcludingThisIfUnshared(aMallocSizeOf);
-    // If the sheet has a parent, then its parent will report it so we don't
-    // have to worry about it here.
-    const StyleSheet* sheet = iter.UserData();
-    MOZ_ASSERT(!sheet->GetParentSheet(), "How did an @import rule end up here?");
-    if (!sheet->GetOwnerNode()) {
-      n += sheet->SizeOfIncludingThis(aMallocSizeOf);
-    }
-  }
-
-  // Measurement of the following members may be added later if DMD finds it is
-  // worthwhile:
-  // - mLoadingDatas: transient, and should be small
-  // - mPendingDatas: transient, and should be small
-  return n;
-}
-
 /*************************
  * Loader Implementation *
  *************************/
@@ -1879,7 +1825,6 @@ Result<Loader::LoadSheetResult, nsresult> Loader::LoadInlineStyle(
 
   // Check IsAlternateSheet now, since it can mutate our document.
   auto isAlternate = IsAlternateSheet(aInfo.mTitle, aInfo.mHasAlternateRel);
-  LOG(("  Sheet is alternate: %d", static_cast<int>(isAlternate)));
 
   // Use the document's base URL so that @import in the inline sheet picks up
   // the right base.
@@ -1888,71 +1833,48 @@ Result<Loader::LoadSheetResult, nsresult> Loader::LoadInlineStyle(
   nsIURI* originalURI = nullptr;
 
   MOZ_ASSERT(aInfo.mIntegrity.IsEmpty());
+  auto sheet = MakeRefPtr<StyleSheet>(eAuthorSheetFeatures, aInfo.mCORSMode,
+                                      SRIMetadata{});
+  sheet->SetURIs(sheetURI, originalURI, baseURI);
+  nsCOMPtr<nsIReferrerInfo> referrerInfo =
+      ReferrerInfo::CreateForInternalCSSResources(aInfo.mContent->OwnerDoc());
+  sheet->SetReferrerInfo(referrerInfo);
 
-  // We only cache sheets if in shadow trees, since regular document sheets are
-  // likely to be unique.
-  const bool isWorthCaching = aInfo.mContent->IsInShadowTree();
-  RefPtr<StyleSheet> sheet;
-  if (isWorthCaching) {
-    if (!mSheets) {
-      mSheets = MakeUnique<Sheets>();
-    }
-    sheet = mSheets->LookupInline(aBuffer);
+  nsIPrincipal* principal = aInfo.mContent->NodePrincipal();
+  if (aInfo.mTriggeringPrincipal) {
+    // The triggering principal may be an expanded principal, which is safe to
+    // use for URL security checks, but not as the loader principal for a
+    // stylesheet. So treat this as principal inheritance, and downgrade if
+    // necessary.
+    principal =
+        BasePrincipal::Cast(aInfo.mTriggeringPrincipal)->PrincipalToInherit();
   }
-  const bool sheetFromCache = !!sheet;
-  if (!sheet) {
-    sheet = MakeRefPtr<StyleSheet>(eAuthorSheetFeatures, aInfo.mCORSMode,
-                                   SRIMetadata{});
-    sheet->SetURIs(sheetURI, originalURI, baseURI);
-    nsCOMPtr<nsIReferrerInfo> referrerInfo =
-        ReferrerInfo::CreateForInternalCSSResources(aInfo.mContent->OwnerDoc());
-    sheet->SetReferrerInfo(referrerInfo);
 
-    nsIPrincipal* principal = aInfo.mContent->NodePrincipal();
-    if (aInfo.mTriggeringPrincipal) {
-      // The triggering principal may be an expanded principal, which is safe to
-      // use for URL security checks, but not as the loader principal for a
-      // stylesheet. So treat this as principal inheritance, and downgrade if
-      // necessary.
-      principal =
-          BasePrincipal::Cast(aInfo.mTriggeringPrincipal)->PrincipalToInherit();
-    }
+  // We never actually load this, so just set its principal directly
+  sheet->SetPrincipal(principal);
 
-    // We never actually load this, so just set its principal directly
-    sheet->SetPrincipal(principal);
-  }
+  LOG(("  Sheet is alternate: %d", static_cast<int>(isAlternate)));
 
   auto matched = PrepareSheet(*sheet, aInfo.mTitle, aInfo.mMedia, nullptr,
                               isAlternate, aInfo.mIsExplicitlyEnabled);
 
   InsertSheetInTree(*sheet, aInfo.mContent);
 
-  Completed completed;
-  if (sheetFromCache) {
-    MOZ_ASSERT(sheet->IsComplete());
-    completed = Completed::Yes;
-  } else {
-    auto data = MakeRefPtr<SheetLoadData>(
-        this, aInfo.mTitle, nullptr, sheet, false, owningElement, isAlternate,
-        matched, aObserver, nullptr, aInfo.mReferrerInfo, aInfo.mContent);
-    data->mLineNumber = aLineNumber;
-    // Parse completion releases the load data.
-    //
-    // Note that we need to parse synchronously, since the web expects that the
-    // effects of inline stylesheets are visible immediately (aside from
-    // @imports).
-    NS_ConvertUTF16toUTF8 utf8(aBuffer);
-    completed = ParseSheet(utf8, *data, AllowAsyncParse::No);
-    if (completed == Completed::Yes) {
-      // TODO(emilio): Try to cache sheets with @import rules, maybe?
-      if (isWorthCaching) {
-        mSheets->mInlineSheets.Put(aBuffer, sheet);
-      }
-    } else {
-      data->mMustNotify = true;
-    }
-  }
+  auto data = MakeRefPtr<SheetLoadData>(
+      this, aInfo.mTitle, nullptr, sheet, false, owningElement, isAlternate,
+      matched, aObserver, nullptr, aInfo.mReferrerInfo, aInfo.mContent);
+  data->mLineNumber = aLineNumber;
+  // Parse completion releases the load data.
+  //
+  // Note that we need to parse synchronously, since the web expects that the
+  // effects of inline stylesheets are visible immediately (aside from
+  // @imports).
+  NS_ConvertUTF16toUTF8 utf8(aBuffer);
+  Completed completed = ParseSheet(utf8, *data, AllowAsyncParse::No);
 
+  if (completed == Completed::No) {
+    data->mMustNotify = true;
+  }
   return LoadSheetResult{completed, isAlternate, matched};
 }
 
@@ -2475,16 +2397,28 @@ NS_IMPL_CYCLE_COLLECTION_UNLINK_END
 NS_IMPL_CYCLE_COLLECTION_ROOT_NATIVE(Loader, AddRef)
 NS_IMPL_CYCLE_COLLECTION_UNROOT_NATIVE(Loader, Release)
 
-size_t Loader::SizeOfIncludingThis(MallocSizeOf aMallocSizeOf) const {
+size_t Loader::SizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf) const {
   size_t n = aMallocSizeOf(this);
 
   if (mSheets) {
-    n += mSheets->SizeOfIncludingThis(aMallocSizeOf);
+    n += mSheets->mCompleteSheets.ShallowSizeOfExcludingThis(aMallocSizeOf);
+    for (auto iter = mSheets->mCompleteSheets.ConstIter(); !iter.Done();
+         iter.Next()) {
+      // If aSheet has a parent, then its parent will report it so we don't
+      // have to worry about it here. Likewise, if aSheet has an owning node,
+      // then the document that node is in will report it.
+      const StyleSheet* sheet = iter.UserData();
+      n += (sheet->GetOwnerNode() || sheet->GetParentSheet())
+               ? 0
+               : sheet->SizeOfIncludingThis(aMallocSizeOf);
+    }
   }
   n += mObservers.ShallowSizeOfExcludingThis(aMallocSizeOf);
 
   // Measurement of the following members may be added later if DMD finds it is
   // worthwhile:
+  // - mLoadingDatas: transient, and should be small
+  // - mPendingDatas: transient, and should be small
   // - mPostedEvents: transient, and should be small
   //
   // The following members aren't measured:

testing/talos/talos/tests/perf-reftest-singletons/inline-style-cache-1.html

@@ -1,24 +0,0 @@
-<!doctype html>
-<script src="util.js"></script>
-<body>
-<script>
-const styles = `:host { background: red; width: 100px; height: 100px; display: block; }`.repeat(10000);
-customElements.define("custom-element", class CustomElement extends HTMLElement {
-  connectedCallback() {
-    this.attachShadow({ mode: "open" });
-    const style = document.createElement("style");
-    style.textContent = styles;
-    this.shadowRoot.appendChild(style);
-  }
-});
-
-perf_start();
-
-for (let i = 0; i < 1000; ++i)
-  document.body.appendChild(document.createElement("custom-element"));
-
-onload = function() {
-  flush_layout(document.body);
-  perf_finish();
-};
-</script>

testing/talos/talos/tests/perf-reftest-singletons/link-style-cache-1.html

@@ -1,26 +0,0 @@
-<!doctype html>
-<script src="util.js"></script>
-<body>
-<script>
-const styles = `:host { background: red; width: 100px; height: 100px; display: block; }`.repeat(10000);
-const blob = URL.createObjectURL(new Blob([styles], { type: 'text/css' }));
-customElements.define("custom-element", class CustomElement extends HTMLElement {
-  connectedCallback() {
-    this.attachShadow({ mode: "open" });
-    const link = document.createElement("link");
-    link.rel = "stylesheet";
-    link.href = blob;
-    this.shadowRoot.appendChild(link);
-  }
-});
-
-perf_start();
-
-for (let i = 0; i < 1000; ++i)
-  document.body.appendChild(document.createElement("custom-element"));
-
-onload = function() {
-  flush_layout(document.body);
-  perf_finish();
-};
-</script>

testing/talos/talos/tests/perf-reftest-singletons/perf_reftest_singletons.manifest

@@ -23,5 +23,3 @@
 % http://localhost/tests/perf-reftest-singletons/id-getter-7.html
 % http://localhost/tests/perf-reftest-singletons/abspos-reflow-1.html
 % http://localhost/tests/perf-reftest-singletons/scrollbar-styles-1.html
-% http://localhost/tests/perf-reftest-singletons/inline-style-cache-1.html
-% http://localhost/tests/perf-reftest-singletons/link-style-cache-1.html