Bug 1649728 - Use WeakPtr in SharedStyleSheetCache::mLoadingDatas. r=firefox-style-system-reviewers,jwatt
authorEmilio Cobos Álvarez <emilio@crisal.io>
Thu, 16 Jul 2020 16:31:52 +0000
changeset 540756 10064e664a57f5a297dd25ae8c6ba07c03398b2a
parent 540755 dce645f310a560b8a3cc5fb411be943f2a6ab814
child 540757 5d5fb35f83ed07f16b4ed242e7dd7d091af16748
push id37609
push userrmaries@mozilla.com
push dateFri, 17 Jul 2020 03:27:56 +0000
treeherdermozilla-central@f6127ce5c744 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfirefox-style-system-reviewers, jwatt
bugs1649728, 1651661
milestone80.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 1649728 - Use WeakPtr in SharedStyleSheetCache::mLoadingDatas. r=firefox-style-system-reviewers,jwatt This is safer in case Necko fails to notify us. The only repro we have is fixed by bug 1651661, but this should hopefully be uncontroversial as well and prevents crashing in release builds. Differential Revision: https://phabricator.services.mozilla.com/D83642
layout/style/SharedStyleSheetCache.cpp
layout/style/SharedStyleSheetCache.h
layout/style/SheetLoadData.h
--- a/layout/style/SharedStyleSheetCache.cpp
+++ b/layout/style/SharedStyleSheetCache.cpp
@@ -499,22 +499,24 @@ void SharedStyleSheetCache::CancelDeferr
 }
 
 void SharedStyleSheetCache::CancelLoadsForLoader(css::Loader& aLoader) {
   CancelDeferredLoadsForLoader(aLoader);
 
   // We can't stop in-progress loads because some other loader may care about
   // them.
   for (auto iter = mLoadingDatas.Iter(); !iter.Done(); iter.Next()) {
-    SheetLoadData* data = iter.Data();
-    do {
+    MOZ_DIAGNOSTIC_ASSERT(iter.Data(),
+                          "We weren't properly notified and the load was "
+                          "incorrectly dropped on the floor");
+    for (SheetLoadData* data = iter.Data(); data; data = data->mNext) {
       if (data->mLoader == &aLoader) {
         data->mIsCancelled = true;
       }
-    } while ((data = data->mNext));
+    }
   }
 }
 
 void SharedStyleSheetCache::RegisterLoader(css::Loader& aLoader) {
   MOZ_ASSERT(aLoader.GetDocument());
   mLoaderPrincipalRefCnt.LookupForAdd(aLoader.GetDocument()->NodePrincipal())
       .OrInsert([] { return 0; }) += 1;
 }
--- a/layout/style/SharedStyleSheetCache.h
+++ b/layout/style/SharedStyleSheetCache.h
@@ -119,21 +119,22 @@ class SharedStyleSheetCache final : publ
 
     bool Expired() const;
   };
 
   void WillStartPendingLoad(css::SheetLoadData&);
 
   nsDataHashtable<SheetLoadDataHashKey, CompleteSheet> mCompleteSheets;
   nsRefPtrHashtable<SheetLoadDataHashKey, css::SheetLoadData> mPendingDatas;
-  // The SheetLoadData pointers in mLoadingDatas below are weak references.
+  // The SheetLoadData pointers in mLoadingDatas below are weak references that
+  // get cleaned up when StreamLoader::OnStopRequest gets called.
   //
   // Note that we hold on to all sheet loads, even if in the end they happen not
   // to be cacheable.
-  nsDataHashtable<SheetLoadDataHashKey, css::SheetLoadData*> mLoadingDatas;
+  nsDataHashtable<SheetLoadDataHashKey, WeakPtr<css::SheetLoadData>> mLoadingDatas;
 
   // An origin-to-number-of-registered-documents count, in order to manage cache
   // eviction as described in RegisterLoader / UnregisterLoader.
   nsDataHashtable<PrincipalHashKey, uint32_t> mLoaderPrincipalRefCnt;
 
   static SharedStyleSheetCache* sInstance;
 };
 
--- a/layout/style/SheetLoadData.h
+++ b/layout/style/SheetLoadData.h
@@ -34,16 +34,18 @@ namespace css {
  *********************************************/
 
 static_assert(eAuthorSheetFeatures == 0 && eUserSheetFeatures == 1 &&
                   eAgentSheetFeatures == 2,
               "sheet parsing mode constants won't fit "
               "in SheetLoadData::mParsingMode");
 
 class SheetLoadData final : public PreloaderBase,
+                            // FIXME(bug 1653011): This is a bit unfortunate.
+                            public SupportsWeakPtr<SheetLoadData>,
                             public nsIRunnable,
                             public nsIThreadObserver {
   using MediaMatched = dom::LinkStyle::MediaMatched;
   using IsAlternate = dom::LinkStyle::IsAlternate;
   using IsPreload = Loader::IsPreload;
   using UseSystemPrincipal = Loader::UseSystemPrincipal;
 
  protected: