Bug 1442246: Copy less URLs on stylo. r?jdm draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Thu, 01 Mar 2018 15:37:23 +0100
changeset 761813 95045468924bc4a878be449ca66e867313458e52
parent 761812 c7441d3f4d773d43044b636d698f0b47cac898c3
push id101006
push userbmo:emilio@crisal.io
push dateThu, 01 Mar 2018 14:48:29 +0000
reviewersjdm
bugs1442246
milestone60.0a1
Bug 1442246: Copy less URLs on stylo. r?jdm MozReview-Commit-ID: NmHue1mGDq
layout/style/ServoBindings.cpp
layout/style/ServoBindings.h
layout/style/nsCSSValue.h
servo/components/style/gecko/url.rs
--- a/layout/style/ServoBindings.cpp
+++ b/layout/style/ServoBindings.cpp
@@ -1454,28 +1454,32 @@ const AnonymousCounterStyle*
 Gecko_CounterStyle_GetAnonymous(const CounterStylePtr* aPtr)
 {
   return aPtr->AsAnonymous();
 }
 
 already_AddRefed<css::URLValue>
 ServoBundledURI::IntoCssUrl()
 {
-  if (!mURLString) {
-    return nullptr;
-  }
-
   MOZ_ASSERT(mExtraData->GetReferrer());
   MOZ_ASSERT(mExtraData->GetPrincipal());
 
-  NS_ConvertUTF8toUTF16 url(reinterpret_cast<const char*>(mURLString),
-                            mURLStringLength);
-
   RefPtr<css::URLValue> urlValue =
-    new css::URLValue(url, do_AddRef(mExtraData));
+    new css::URLValue(mURLString, do_AddRef(mExtraData));
+  return urlValue.forget();
+}
+
+already_AddRefed<css::ImageValue>
+ServoBundledURI::IntoCssImage(mozilla::CORSMode aCorsMode)
+{
+  MOZ_ASSERT(mExtraData->GetReferrer());
+  MOZ_ASSERT(mExtraData->GetPrincipal());
+
+  RefPtr<css::ImageValue> urlValue =
+    new css::ImageValue(mURLString, do_AddRef(mExtraData), aCorsMode);
   return urlValue.forget();
 }
 
 void
 Gecko_SetNullImageValue(nsStyleImage* aImage)
 {
   MOZ_ASSERT(aImage);
   aImage->SetNull();
@@ -1495,24 +1499,20 @@ CreateStyleImageRequest(nsStyleImageRequ
                         mozilla::css::ImageValue* aImageValue)
 {
   RefPtr<nsStyleImageRequest> req =
     new nsStyleImageRequest(aModeFlags, aImageValue);
   return req.forget();
 }
 
 mozilla::css::ImageValue*
-Gecko_ImageValue_Create(ServoBundledURI aURI, ServoRawOffsetArc<RustString> aURIString)
+Gecko_ImageValue_Create(ServoBundledURI aURI)
 {
   // Bug 1434963: Change this to accept a CORS mode from the caller.
-  RefPtr<css::ImageValue> value(
-    new css::ImageValue(aURIString,
-                        do_AddRef(aURI.mExtraData),
-                        mozilla::CORSMode::CORS_NONE));
-  return value.forget().take();
+  return aURI.IntoCssImage(mozilla::CORSMode::CORS_NONE).take();
 }
 
 MOZ_DEFINE_MALLOC_SIZE_OF(GeckoImageValueMallocSizeOf)
 
 size_t
 Gecko_ImageValue_SizeOfIncludingThis(mozilla::css::ImageValue* aImageValue)
 {
   MOZ_ASSERT(NS_IsMainThread());
--- a/layout/style/ServoBindings.h
+++ b/layout/style/ServoBindings.h
@@ -124,19 +124,21 @@ namespace mozilla {
 DEFINE_ARRAY_TYPE_FOR(uintptr_t);
 #undef DEFINE_ARRAY_TYPE_FOR
 
 extern "C" {
 
 class ServoBundledURI
 {
 public:
+  // NOTE(emilio): Not calling IntoCssUrl or IntoCssImage will cause to leak the
+  // string, so don't do that :)
   already_AddRefed<mozilla::css::URLValue> IntoCssUrl();
-  const uint8_t* mURLString;
-  uint32_t mURLStringLength;
+  already_AddRefed<mozilla::css::ImageValue> IntoCssImage(mozilla::CORSMode);
+  mozilla::ServoRawOffsetArc<RustString> mURLString;
   mozilla::URLExtraData* mExtraData;
 };
 
 struct FontSizePrefs
 {
   void CopyFrom(const mozilla::LangGroupFontPrefs&);
   nscoord mDefaultVariableSize;
   nscoord mDefaultFixedSize;
@@ -348,18 +350,17 @@ void Gecko_CopyCounterStyle(mozilla::Cou
 nsAtom* Gecko_CounterStyle_GetName(const mozilla::CounterStylePtr* ptr);
 const mozilla::AnonymousCounterStyle*
 Gecko_CounterStyle_GetAnonymous(const mozilla::CounterStylePtr* ptr);
 
 // background-image style.
 void Gecko_SetNullImageValue(nsStyleImage* image);
 void Gecko_SetGradientImageValue(nsStyleImage* image, nsStyleGradient* gradient);
 NS_DECL_THREADSAFE_FFI_REFCOUNTING(mozilla::css::ImageValue, ImageValue);
-mozilla::css::ImageValue* Gecko_ImageValue_Create(ServoBundledURI aURI,
-                                                  mozilla::ServoRawOffsetArc<RustString> aURIString);
+mozilla::css::ImageValue* Gecko_ImageValue_Create(ServoBundledURI aURI);
 size_t Gecko_ImageValue_SizeOfIncludingThis(mozilla::css::ImageValue* aImageValue);
 void Gecko_SetLayerImageImageValue(nsStyleImage* image,
                                    mozilla::css::ImageValue* aImageValue);
 
 void Gecko_SetImageElement(nsStyleImage* image, nsAtom* atom);
 void Gecko_CopyImageValueFrom(nsStyleImage* image, const nsStyleImage* other);
 void Gecko_InitializeImageCropRect(nsStyleImage* image);
 
--- a/layout/style/nsCSSValue.h
+++ b/layout/style/nsCSSValue.h
@@ -227,17 +227,17 @@ struct URLValue final : public URLValueD
 {
   // These two constructors are safe to call only on the main thread.
   URLValue(const nsAString& aString, nsIURI* aBaseURI, nsIURI* aReferrer,
            nsIPrincipal* aOriginPrincipal);
   URLValue(nsIURI* aURI, const nsAString& aString, nsIURI* aBaseURI,
            nsIURI* aReferrer, nsIPrincipal* aOriginPrincipal);
 
   // This constructor is safe to call from any thread.
-  URLValue(const nsAString& aString,
+  URLValue(ServoRawOffsetArc<RustString> aString,
            already_AddRefed<URLExtraData> aExtraData)
     : URLValueData(aString, Move(aExtraData)) {}
 
   URLValue(const URLValue&) = delete;
   URLValue& operator=(const URLValue&) = delete;
 
   size_t SizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf) const;
 };
--- a/servo/components/style/gecko/url.rs
+++ b/servo/components/style/gecko/url.rs
@@ -76,17 +76,17 @@ impl SpecifiedUrl {
     /// Convert from nsStyleImageRequest to SpecifiedUrl.
     pub unsafe fn from_image_request(image_request: &nsStyleImageRequest) -> Result<SpecifiedUrl, ()> {
         if image_request.mImageValue.mRawPtr.is_null() {
             return Err(());
         }
 
         let image_value = image_request.mImageValue.mRawPtr.as_ref().unwrap();
         let ref url_value_data = image_value._base;
-        let mut result = try!(Self::from_url_value_data(url_value_data));
+        let mut result = Self::from_url_value_data(url_value_data)?;
         result.build_image_value();
         Ok(result)
     }
 
     /// Returns true if this URL looks like a fragment.
     /// See https://drafts.csswg.org/css-values/#local-urls
     pub fn is_fragment(&self) -> bool {
         self.as_str().chars().next().map_or(false, |c| c == '#')
@@ -103,35 +103,33 @@ impl SpecifiedUrl {
     /// Little helper for Gecko's ffi.
     pub fn as_slice_components(&self) -> (*const u8, usize) {
         (self.serialization.as_str().as_ptr(), self.serialization.as_str().len())
     }
 
     /// Create a bundled URI suitable for sending to Gecko
     /// to be constructed into a css::URLValue
     pub fn for_ffi(&self) -> ServoBundledURI {
-        let (ptr, len) = self.as_slice_components();
+        let arc_offset = Arc::into_raw_offset(self.serialization.clone());
         ServoBundledURI {
-            mURLString: ptr,
-            mURLStringLength: len as u32,
+            mURLString: unsafe {
+                mem::transmute::<_, RawOffsetArc<RustString>>(arc_offset)
+            },
             mExtraData: self.extra_data.get(),
         }
     }
 
     /// Build and carry an image value on request.
     pub fn build_image_value(&mut self) {
         use gecko_bindings::bindings::Gecko_ImageValue_Create;
 
         debug_assert_eq!(self.image_value, None);
         self.image_value = {
             unsafe {
-                let arc_offset = Arc::into_raw_offset(self.serialization.clone());
-                let ptr = Gecko_ImageValue_Create(
-                    self.for_ffi(),
-                    mem::transmute::<_, RawOffsetArc<RustString>>(arc_offset));
+                let ptr = Gecko_ImageValue_Create(self.for_ffi());
                 // We do not expect Gecko_ImageValue_Create returns null.
                 debug_assert!(!ptr.is_null());
                 Some(RefPtr::from_addrefed(ptr))
             }
         }
     }
 }