Bug 1850414 - Simplify style sheet source-map URL code. r=layout-reviewers,firefox-style-system-reviewers,webidl,smaug,boris

The old code was basically doing string copies that are totally
redundant, in a not-very performant way too.

This was from the time where stylo had to live with the old style
engine, and there's no need to keep the copy around anymore.

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

dom/webidl/StyleSheet.webidl

@@ -39,13 +39,13 @@ interface StyleSheet {
   // If the source map URL is not found by either of these methods,
   // then this is an empty string.
   [ChromeOnly, Pure]
-  readonly attribute DOMString sourceMapURL;
+  readonly attribute UTF8String sourceMapURL;
   // The source URL for this style sheet.  If the style sheet has the
   // special "# sourceURL=" comment, then this is the URL specified
   // there.  If no such comment is found, then this is the empty
   // string.
   [ChromeOnly, Pure]
-  readonly attribute DOMString sourceURL;
+  readonly attribute UTF8String sourceURL;
   [ChromeOnly, Pure]
   readonly attribute Document? associatedDocument;
   [ChromeOnly, Pure, BinaryName="isConstructed"]

layout/style/Loader.cpp

@@ -730,9 +730,9 @@ nsresult SheetLoadData::VerifySheetReadyToParse(nsresult aStatus,
       return NS_OK;
     }
 
-    nsAutoCString sourceMapURL;
+    nsCString sourceMapURL;
     if (nsContentUtils::GetSourceMapURL(httpChannel, sourceMapURL)) {
-      mSheet->SetSourceMapURL(NS_ConvertUTF8toUTF16(sourceMapURL));
+      mSheet->SetSourceMapURL(std::move(sourceMapURL));
     }
   }
 

layout/style/StyleSheet.cpp

@@ -397,8 +397,6 @@ StyleSheetInfo::StyleSheetInfo(StyleSheetInfo& aCopy, StyleSheet* aPrimarySheet)
       // We don't rebuild the child because we're making a copy without
       // children.
       mSourceMapURL(aCopy.mSourceMapURL),
-      mSourceMapURLFromComment(aCopy.mSourceMapURLFromComment),
-      mSourceURL(aCopy.mSourceURL),
       mContents(Servo_StyleSheet_Clone(aCopy.mContents.get(), aPrimarySheet)
                     .Consume()),
       mURLData(aCopy.mURLData)
@@ -577,29 +575,20 @@ dom::CSSRuleList* StyleSheet::GetCssRules(nsIPrincipal& aSubjectPrincipal,
   return GetCssRulesInternal();
 }
 
-void StyleSheet::GetSourceMapURL(nsAString& aSourceMapURL) {
-  if (mInner->mSourceMapURL.IsEmpty()) {
-    aSourceMapURL = mInner->mSourceMapURLFromComment;
-  } else {
+void StyleSheet::GetSourceMapURL(nsACString& aSourceMapURL) {
+  if (!mInner->mSourceMapURL.IsEmpty()) {
     aSourceMapURL = mInner->mSourceMapURL;
+    return;
   }
+  Servo_StyleSheet_GetSourceMapURL(mInner->mContents, &aSourceMapURL);
 }
 
-void StyleSheet::SetSourceMapURL(const nsAString& aSourceMapURL) {
-  mInner->mSourceMapURL = aSourceMapURL;
+void StyleSheet::SetSourceMapURL(nsCString&& aSourceMapURL) {
+  mInner->mSourceMapURL = std::move(aSourceMapURL);
 }
 
-void StyleSheet::SetSourceMapURLFromComment(
-    const nsAString& aSourceMapURLFromComment) {
-  mInner->mSourceMapURLFromComment = aSourceMapURLFromComment;
-}
-
-void StyleSheet::GetSourceURL(nsAString& aSourceURL) {
-  aSourceURL = mInner->mSourceURL;
-}
-
-void StyleSheet::SetSourceURL(const nsAString& aSourceURL) {
-  mInner->mSourceURL = aSourceURL;
+void StyleSheet::GetSourceURL(nsACString& aSourceURL) {
+  Servo_StyleSheet_GetSourceURL(mInner->mContents, &aSourceURL);
 }
 
 css::Rule* StyleSheet::GetDOMOwnerRule() const { return GetOwnerRule(); }
@@ -788,7 +777,6 @@ void StyleSheet::ReplaceSync(const nsACString& aText, ErrorResult& aRv) {
   // 5. Set sheet's rules to the new rules.
   Inner().mContents = std::move(rawContent);
   FixUpRuleListAfterContentsChangeIfNeeded();
-  FinishParse();
   RuleChanged(nullptr, StyleRuleChangeKind::Generic);
 }
 
@@ -1252,7 +1240,6 @@ void StyleSheet::FinishAsyncParse(
   MOZ_ASSERT(!mParsePromise.IsEmpty());
   Inner().mContents = aSheetContents;
   Inner().mUseCounters = std::move(aUseCounters);
-  FinishParse();
   mParsePromise.Resolve(true, __func__);
 }
 
@@ -1288,18 +1275,6 @@ void StyleSheet::ParseSheetSync(
                           allowImportRules, StyleSanitizationKind::None,
                           /* sanitized_output = */ nullptr)
                           .Consume();
-
-  FinishParse();
-}
-
-void StyleSheet::FinishParse() {
-  nsString sourceMapURL;
-  Servo_StyleSheet_GetSourceMapURL(Inner().mContents, &sourceMapURL);
-  SetSourceMapURLFromComment(sourceMapURL);
-
-  nsString sourceURL;
-  Servo_StyleSheet_GetSourceURL(Inner().mContents, &sourceURL);
-  SetSourceURL(sourceURL);
 }
 
 void StyleSheet::ReparseSheet(const nsACString& aInput, ErrorResult& aRv) {
@@ -1491,9 +1466,6 @@ void StyleSheet::SetSharedContents(const StyleLockedCssRules* aSharedRules) {
 
   Inner().mContents =
       Servo_StyleSheet_FromSharedData(URLData(), aSharedRules).Consume();
-
-  // Don't call FinishParse(), since that tries to set source map URLs,
-  // which we don't have.
 }
 
 const StyleLockedCssRules* StyleSheet::ToShared(

layout/style/StyleSheet.h

@@ -343,11 +343,10 @@ class StyleSheet final : public nsICSSLoaderObserver, public nsWrapperCache {
   dom::MediaList* Media();
   bool Disabled() const { return bool(mState & State::Disabled); }
   void SetDisabled(bool aDisabled);
-  void GetSourceMapURL(nsAString& aTitle);
-  void SetSourceMapURL(const nsAString& aSourceMapURL);
-  void SetSourceMapURLFromComment(const nsAString& aSourceMapURLFromComment);
-  void GetSourceURL(nsAString& aSourceURL);
-  void SetSourceURL(const nsAString& aSourceURL);
+
+  void GetSourceMapURL(nsACString&);
+  void SetSourceMapURL(nsCString&&);
+  void GetSourceURL(nsACString& aSourceURL);
 
   // WebIDL CSSStyleSheet API
   // Can't be inline because we can't include ImportRule here.  And can't be
@@ -512,9 +511,6 @@ class StyleSheet final : public nsICSSLoaderObserver, public nsWrapperCache {
   nsresult InsertRuleIntoGroupInternal(const nsACString& aRule,
                                        css::GroupRule* aGroup, uint32_t aIndex);
 
-  // Common tail routine for the synchronous and asynchronous parsing paths.
-  void FinishParse();
-
   // Take the recently cloned sheets from the `@import` rules, and reparent them
   // correctly to `aPrimarySheet`.
   void FixUpAfterInnerClone();

layout/style/StyleSheetInfo.h

@@ -67,15 +67,7 @@ struct StyleSheetInfo final {
   // If a SourceMap or X-SourceMap response header is seen, this is
   // the value.  If both are seen, SourceMap is preferred.  If neither
   // is seen, this will be an empty string.
-  nsString mSourceMapURL;
-  // This stores any source map URL that might have been seen in a
-  // comment in the style sheet.  This is separate from mSourceMapURL
-  // so that the value does not overwrite any value that might have
-  // come from a response header.
-  nsString mSourceMapURLFromComment;
-  // This stores any source URL that might have been seen in a comment
-  // in the style sheet.
-  nsString mSourceURL;
+  nsCString mSourceMapURL;
 
   RefPtr<const StyleStylesheetContents> mContents;
 

servo/ports/geckolib/glue.rs

@@ -1940,22 +1940,22 @@ pub extern "C" fn Servo_StyleSheet_GetOrigin(sheet: &StylesheetContents) -> Orig
 #[no_mangle]
 pub extern "C" fn Servo_StyleSheet_GetSourceMapURL(
     contents: &StylesheetContents,
-    result: &mut nsAString,
+    result: &mut nsACString,
 ) {
     let url_opt = contents.source_map_url.read();
     if let Some(ref url) = *url_opt {
-        write!(result, "{}", url).unwrap();
+        result.assign(url);
     }
 }
 
 #[no_mangle]
 pub extern "C" fn Servo_StyleSheet_GetSourceURL(
     contents: &StylesheetContents,
-    result: &mut nsAString,
+    result: &mut nsACString,
 ) {
     let url_opt = contents.source_url.read();
     if let Some(ref url) = *url_opt {
-        write!(result, "{}", url).unwrap();
+        result.assign(url);
     }
 }