Bug 1797312 - SharedSubResourceCacheLoadingValueBase use on accessors. r=emilio

`SharedSubResourceCacheLoadingValueBase` define the `mIsCancelled` field which
is overloaded by `SheetLoadData`. This patch moves all fields including
`mIsLoading` to `SheetLoadData` and add accessors to access these fields.

Differential Revision: https://phabricator.services.mozilla.com/D160226

layout/style/Loader.cpp

@@ -287,6 +287,7 @@ SheetLoadData::SheetLoadData(css::Loader* aLoader, const nsAString& aTitle,
       mIsNonDocumentSheet(false),
       mIsChildSheet(aSheet->GetParentSheet()),
       mIsBeingParsed(false),
+      mIsLoading(false),
       mIsCancelled(false),
       mMustNotify(false),
       mWasAlternate(aIsAlternate == IsAlternate::Yes),
@@ -326,6 +327,7 @@ SheetLoadData::SheetLoadData(css::Loader* aLoader, nsIURI* aURI,
       mIsNonDocumentSheet(aParentData && aParentData->mIsNonDocumentSheet),
       mIsChildSheet(aSheet->GetParentSheet()),
       mIsBeingParsed(false),
+      mIsLoading(false),
       mIsCancelled(false),
       mMustNotify(false),
       mWasAlternate(false),
@@ -364,6 +366,7 @@ SheetLoadData::SheetLoadData(
       mIsNonDocumentSheet(true),
       mIsChildSheet(false),
       mIsBeingParsed(false),
+      mIsLoading(false),
       mIsCancelled(false),
       mMustNotify(false),
       mWasAlternate(false),
@@ -659,7 +662,7 @@ static nsresult VerifySheetIntegrity(const SRIMetadata& aMetadata,
 static bool AllLoadsCanceled(const SheetLoadData& aData) {
   const SheetLoadData* data = &aData;
   do {
-    if (!data->mIsCancelled) {
+    if (!data->IsCancelled()) {
       return false;
     }
   } while ((data = data->mNext));
@@ -2200,7 +2203,7 @@ void Loader::Stop() {
 
   auto arr = std::move(mPostedEvents);
   for (auto& data : arr) {
-    data->mIsCancelled = true;
+    data->Cancel();
   }
 }
 

layout/style/SharedStyleSheetCache.cpp

@@ -48,7 +48,7 @@ void SharedStyleSheetCache::LoadCompleted(SharedStyleSheetCache* aCache,
     cancelledStatus = NS_BINDING_ABORTED;
     css::SheetLoadData* data = &aData;
     do {
-      if (data->mIsCancelled) {
+      if (data->IsCancelled()) {
         // We only need to mark loads for this loader as cancelled, so as to not
         // fire error events in unrelated documents.
         css::Loader::MarkLoadTreeFailed(*data, data->mLoader);
@@ -64,7 +64,7 @@ void SharedStyleSheetCache::LoadCompleted(SharedStyleSheetCache* aCache,
 
   // Now it's safe to go ahead and notify observers
   for (RefPtr<css::SheetLoadData>& data : datasToNotify) {
-    auto status = data->mIsCancelled ? cancelledStatus : aStatus;
+    auto status = data->IsCancelled() ? cancelledStatus : aStatus;
     data->mLoader->NotifyObservers(*data, status);
   }
 }

layout/style/SharedSubResourceCache.h

@@ -50,8 +50,12 @@ struct SharedSubResourceCacheLoadingValueBase {
   // Whether we're in the "loading" hash table.
   RefPtr<Derived> mNext;
 
-  bool mIsLoading = false;
-  bool mIsCancelled = false;
+  virtual bool IsLoading() const = 0;
+  virtual bool IsCancelled() const = 0;
+
+  virtual void StartLoading() = 0;
+  virtual void SetLoadCompleted() = 0;
+  virtual void Cancel() = 0;
 
   ~SharedSubResourceCacheLoadingValueBase() {
     // Do this iteratively to avoid blowing up the stack.
@@ -307,7 +311,8 @@ void SharedSubResourceCache<Traits, Derived>::CancelLoadsForLoader(
                           "incorrectly dropped on the floor");
     for (; data; data = data->mNext) {
       if (&data->Loader() == &aLoader) {
-        data->mIsCancelled = true;
+        data->Cancel();
+        MOZ_ASSERT(data->IsCancelled());
       }
     }
   }
@@ -458,10 +463,11 @@ size_t SharedSubResourceCache<Traits, Derived>::SizeOfIncludingThis(
 template <typename Traits, typename Derived>
 void SharedSubResourceCache<Traits, Derived>::LoadStarted(
     const Key& aKey, LoadingValue& aValue) {
-  MOZ_DIAGNOSTIC_ASSERT(!aValue.mIsLoading, "Already loading? How?");
+  MOZ_DIAGNOSTIC_ASSERT(!aValue.IsLoading(), "Already loading? How?");
   MOZ_DIAGNOSTIC_ASSERT(KeyFromLoadingValue(aValue).KeyEquals(aKey));
   MOZ_DIAGNOSTIC_ASSERT(!mLoading.Contains(aKey), "Load not coalesced?");
-  aValue.mIsLoading = true;
+  aValue.StartLoading();
+  MOZ_ASSERT(aValue.IsLoading(), "Check that StartLoading is effectful.");
   mLoading.InsertOrUpdate(aKey, &aValue);
 }
 
@@ -475,7 +481,7 @@ bool SharedSubResourceCache<Traits, Derived>::CompleteSubResource::Expired()
 template <typename Traits, typename Derived>
 void SharedSubResourceCache<Traits, Derived>::LoadCompleted(
     LoadingValue& aValue) {
-  if (!aValue.mIsLoading) {
+  if (!aValue.IsLoading()) {
     return;
   }
   auto key = KeyFromLoadingValue(aValue);
@@ -483,7 +489,8 @@ void SharedSubResourceCache<Traits, Derived>::LoadCompleted(
   MOZ_DIAGNOSTIC_ASSERT(value);
   MOZ_DIAGNOSTIC_ASSERT(value.value() == &aValue);
   Unused << value;
-  aValue.mIsLoading = false;
+  aValue.SetLoadCompleted();
+  MOZ_ASSERT(!aValue.IsLoading(), "Check that SetLoadCompleted is effectful.");
 }
 
 }  // namespace mozilla

layout/style/SheetLoadData.h

@@ -148,6 +148,11 @@ class SheetLoadData final
   // mIsBeingParsed is true if this stylesheet is currently being parsed.
   bool mIsBeingParsed : 1;
 
+  // mIsLoading is set to true when a sheet load is initiated. This field is
+  // also used by the SharedSubResourceCache to avoid having multiple loads for
+  // the same resource.
+  bool mIsLoading : 1;
+
   // mIsCancelled is set to true when a sheet load is stopped by
   // Stop() or StopLoadingSheet() (which was removed in Bug 556446).
   // SheetLoadData::OnStreamComplete() checks this to avoid parsing
@@ -255,6 +260,13 @@ class SheetLoadData final
 
   bool BlocksLoadEvent() const { return !RootLoadData().IsLinkRelPreload(); }
 
+  bool IsLoading() const override { return mIsLoading; }
+  bool IsCancelled() const override { return mIsCancelled; }
+
+  void StartLoading() override { mIsLoading = true; }
+  void SetLoadCompleted() override { mIsLoading = false; }
+  void Cancel() override { mIsCancelled = true; }
+
  private:
   const SheetLoadData& RootLoadData() const {
     auto* top = this;