Bug 1935471 - Make resolving image urls thread-safe. r=dshin, a=RyanVM

We can resolve them off the main thread now due to registered custom
properties (which requires the computed <url> serialization).

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

layout/style/ServoStyleConstsInlines.h

@@ -378,22 +378,25 @@ inline const URLExtraData& StyleCssUrl::ExtraData() const {
   return _0->extra_data.get();
 }
 
-inline StyleLoadData& StyleCssUrl::LoadData() const {
+inline const StyleLoadData& StyleCssUrl::LoadData() const {
   if (MOZ_LIKELY(_0->load_data.tag == StyleLoadDataSource::Tag::Owned)) {
-    MOZ_DIAGNOSTIC_ASSERT(NS_IsMainThread() ||
+    return _0->load_data.owned._0;
-                          dom::IsCurrentThreadRunningWorker());
-    return const_cast<StyleLoadData&>(_0->load_data.owned._0);
   }
-  MOZ_DIAGNOSTIC_ASSERT(NS_IsMainThread(),
+  return *Servo_LoadData_GetLazy(&_0->load_data);
-                        "Lazy load datas should come from user-agent sheets, "
+}
-                        "which don't make sense on workers");
+
-  return const_cast<StyleLoadData&>(*Servo_LoadData_GetLazy(&_0->load_data));
+inline StyleLoadData& StyleCssUrl::MutLoadData() const {
+  MOZ_DIAGNOSTIC_ASSERT(NS_IsMainThread() ||
+                        dom::IsCurrentThreadRunningWorker());
+  return const_cast<StyleLoadData&>(LoadData());
 }
 
 inline nsIURI* StyleCssUrl::GetURI() const {
-  auto& loadData = LoadData();
+  auto& loadData = const_cast<StyleLoadData&>(LoadData());
-  if (!(loadData.flags & StyleLoadDataFlags::TRIED_TO_RESOLVE_URI)) {
+  // Try to read the flags first. If it's set we can avoid entering the CAS
-    loadData.flags |= StyleLoadDataFlags::TRIED_TO_RESOLVE_URI;
+  // loop.
+  auto flags = __atomic_load_n(&loadData.flags.bits, __ATOMIC_RELAXED);
+  if (!(flags & StyleLoadDataFlags::TRIED_TO_RESOLVE_URI.bits)) {
     nsDependentCSubstring serialization = SpecifiedSerialization();
     // https://drafts.csswg.org/css-values-4/#url-empty:
     //
@@ -401,14 +404,29 @@ inline nsIURI* StyleCssUrl::GetURI() const {
     //     url()), the url must resolve to an invalid resource (similar to what
     //     the url about:invalid does).
     //
+    nsIURI* resolved = nullptr;
     if (!serialization.IsEmpty()) {
-      RefPtr<nsIURI> resolved;
+      nsIURI* old_resolved = nullptr;
-      NS_NewURI(getter_AddRefs(resolved), serialization, nullptr,
+      // NOTE: This addrefs `resolved`, and `resolved` might still be null for
-                ExtraData().BaseURI());
+      // invalid URIs.
-      loadData.resolved_uri = resolved.forget().take();
+      NS_NewURI(&resolved, serialization, nullptr, ExtraData().BaseURI());
+      if (!__atomic_compare_exchange_n(&loadData.resolved_uri, &old_resolved,
+                                       resolved, /* weak = */ false,
+                                       __ATOMIC_RELEASE, __ATOMIC_RELAXED)) {
+        // In the unlikely case two threads raced to write the url, avoid
+        // leaking resolved. The actual value is in `old_resolved`.
+        NS_IF_RELEASE(resolved);
+        resolved = old_resolved;
+      }
     }
+    // The flag is effectively just an optimization so we can use relaxed
+    // ordering.
+    __atomic_fetch_or(&loadData.flags.bits,
+                      StyleLoadDataFlags::TRIED_TO_RESOLVE_URI.bits,
+                      __ATOMIC_RELAXED);
+    return resolved;
   }
-  return loadData.resolved_uri;
+  return __atomic_load_n(&loadData.resolved_uri, __ATOMIC_ACQUIRE);
 }
 
 inline nsDependentCSubstring StyleComputedUrl::SpecifiedSerialization() const {
@@ -417,9 +435,12 @@ inline nsDependentCSubstring StyleComputedUrl::SpecifiedSerialization() const {
 inline const URLExtraData& StyleComputedUrl::ExtraData() const {
   return _0.ExtraData();
 }
-inline StyleLoadData& StyleComputedUrl::LoadData() const {
+inline const StyleLoadData& StyleComputedUrl::LoadData() const {
   return _0.LoadData();
 }
+inline StyleLoadData& StyleComputedUrl::MutLoadData() const {
+  return _0.MutLoadData();
+}
 inline StyleCorsMode StyleComputedUrl::CorsMode() const {
   return _0._0->cors_mode;
 }

layout/style/nsStyleStruct.cpp

@@ -115,14 +115,12 @@ void StyleComputedUrl::ResolveImage(Document& aDocument,
                                     const StyleComputedUrl* aOldImage) {
   MOZ_DIAGNOSTIC_ASSERT(NS_IsMainThread());
 
-  StyleLoadData& data = LoadData();
+  StyleLoadData& data = MutLoadData();
 
   MOZ_ASSERT(!(data.flags & StyleLoadDataFlags::TRIED_TO_RESOLVE_IMAGE));
 
   data.flags |= StyleLoadDataFlags::TRIED_TO_RESOLVE_IMAGE;
 
-  MOZ_ASSERT(NS_IsMainThread());
-
   // TODO(emilio, bug 1440442): This is a hackaround to avoid flickering due the
   // lack of non-http image caching in imagelib (bug 1406134), which causes
   // stuff like bug 1439285. Cleanest fix if that doesn't get fixed is bug

servo/ports/geckolib/cbindgen.toml

@@ -817,7 +817,8 @@ renaming_overrides_prefixing = true
 "CssUrl" = """
   inline nsDependentCSubstring SpecifiedSerialization() const;
   inline const URLExtraData& ExtraData() const;
-  inline StyleLoadData& LoadData() const;
+  inline const StyleLoadData& LoadData() const;
+  inline StyleLoadData& MutLoadData() const;
   inline nsIURI* GetURI() const;
 """
 
@@ -826,7 +827,8 @@ renaming_overrides_prefixing = true
   inline nsDependentCSubstring SpecifiedSerialization() const;
   inline const URLExtraData& ExtraData() const;
   inline nsIURI* GetURI() const;
-  inline StyleLoadData& LoadData() const;
+  inline const StyleLoadData& LoadData() const;
+  inline StyleLoadData& MutLoadData() const;
 
   inline bool IsLocalRef() const;
   inline bool HasRef() const;