From 891d20f850cbfe541bf8cc29c2a6b2ad1479ce72 Mon Sep 17 00:00:00 2001 From: Mihai Alexandru Michis Date: Sat, 17 Aug 2019 04:48:35 +0300 Subject: [PATCH] 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 | 156 +++++------------- .../inline-style-cache-1.html | 24 --- .../link-style-cache-1.html | 26 --- .../perf_reftest_singletons.manifest | 2 - 4 files changed, 45 insertions(+), 163 deletions(-) delete mode 100644 testing/talos/talos/tests/perf-reftest-singletons/inline-style-cache-1.html delete mode 100644 testing/talos/talos/tests/perf-reftest-singletons/link-style-cache-1.html diff --git a/layout/style/Loader.cpp b/layout/style/Loader.cpp index 8832db3911b6..fc0bd3c42462 100644 --- a/layout/style/Loader.cpp +++ b/layout/style/Loader.cpp @@ -461,32 +461,11 @@ struct Loader::Sheets { // The SheetLoadData pointers in mLoadingDatas below are weak references. nsDataHashtable mLoadingDatas; - nsRefPtrHashtable mInlineSheets; - - - RefPtr LookupInline(const nsAString&); - // A cache hit or miss. It is a miss if the `StyleSheet` is null. using CacheResult = Tuple, SheetState>; CacheResult Lookup(SheetLoadDataHashKey&, bool aSyncLoad); - - size_t SizeOfIncludingThis(MallocSizeOf) const; }; -RefPtr 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::LoadInlineStyle( // Check IsAlternateSheet now, since it can mutate our document. auto isAlternate = IsAlternateSheet(aInfo.mTitle, aInfo.mHasAlternateRel); - LOG((" Sheet is alternate: %d", static_cast(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::LoadInlineStyle( nsIURI* originalURI = nullptr; MOZ_ASSERT(aInfo.mIntegrity.IsEmpty()); + auto sheet = MakeRefPtr(eAuthorSheetFeatures, aInfo.mCORSMode, + SRIMetadata{}); + sheet->SetURIs(sheetURI, originalURI, baseURI); + nsCOMPtr 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 sheet; - if (isWorthCaching) { - if (!mSheets) { - mSheets = MakeUnique(); - } - 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(eAuthorSheetFeatures, aInfo.mCORSMode, - SRIMetadata{}); - sheet->SetURIs(sheetURI, originalURI, baseURI); - nsCOMPtr 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(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( - 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( + 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: diff --git a/testing/talos/talos/tests/perf-reftest-singletons/inline-style-cache-1.html b/testing/talos/talos/tests/perf-reftest-singletons/inline-style-cache-1.html deleted file mode 100644 index f7a41f50911c..000000000000 --- a/testing/talos/talos/tests/perf-reftest-singletons/inline-style-cache-1.html +++ /dev/null @@ -1,24 +0,0 @@ - - - - diff --git a/testing/talos/talos/tests/perf-reftest-singletons/link-style-cache-1.html b/testing/talos/talos/tests/perf-reftest-singletons/link-style-cache-1.html deleted file mode 100644 index c8b1911b5353..000000000000 --- a/testing/talos/talos/tests/perf-reftest-singletons/link-style-cache-1.html +++ /dev/null @@ -1,26 +0,0 @@ - - - - diff --git a/testing/talos/talos/tests/perf-reftest-singletons/perf_reftest_singletons.manifest b/testing/talos/talos/tests/perf-reftest-singletons/perf_reftest_singletons.manifest index 1123ce180c2b..37f0d17dc8d4 100644 --- a/testing/talos/talos/tests/perf-reftest-singletons/perf_reftest_singletons.manifest +++ b/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