Bug 1388855 - Extract source map URL when parsing CSS, r=bz,heycam
authorTom Tromey <tom@tromey.com>
Wed, 09 Aug 2017 13:33:24 -0600
changeset 428396 742f0335e8583e7c277218a5dfb7f41d9d9bd9f7
parent 428395 41a141a23b5fca85d418de493640c50a34f65e0e
child 428397 258bede4a3791fc92dccd2cbd2799eee91b2c48a
push id7761
push userjlund@mozilla.com
push dateFri, 15 Sep 2017 00:19:52 +0000
treeherdermozilla-beta@c38455951db4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz, heycam
bugs1388855
milestone57.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 1388855 - Extract source map URL when parsing CSS, r=bz,heycam This changes the CSS lexer to extract sourceMappingURL directives from comments and preserve them; then changes the parser to expose this value as the style sheet's sourceMapURL. MozReview-Commit-ID: LfMamJ1PsU0
dom/webidl/StyleSheet.webidl
layout/style/ServoBindingList.h
layout/style/ServoStyleSheet.cpp
layout/style/StyleSheet.cpp
layout/style/StyleSheet.h
layout/style/StyleSheetInfo.h
layout/style/nsCSSParser.cpp
layout/style/nsCSSScanner.cpp
layout/style/nsCSSScanner.h
layout/style/test/browser.ini
layout/style/test/browser_sourcemap_comment.js
layout/style/test/mapped2.css
--- a/dom/webidl/StyleSheet.webidl
+++ b/dom/webidl/StyleSheet.webidl
@@ -19,16 +19,24 @@ interface StyleSheet {
   [Pure]
   readonly attribute StyleSheet? parentStyleSheet;
   [Pure]
   readonly attribute DOMString? title;
   [Constant]
   readonly attribute MediaList media;
   [Pure]
   attribute boolean disabled;
+  // The source map URL for this style sheet.  The source map URL can
+  // be found in one of two ways.
+  //
   // 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.  Because this relies on
-  // the HTTP response, it can change if checked before the response
-  // is available -- which is why it is not [Constant].
+  // the value.  If both are seen, SourceMap is preferred.  Because
+  // this relies on the HTTP response, it can change if checked before
+  // the response is available -- which is why it is not [Constant].
+  //
+  // If the style sheet has the special "# sourceMappingURL=" comment,
+  // then this is the URL specified there.
+  //
+  // 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;
 };
--- a/layout/style/ServoBindingList.h
+++ b/layout/style/ServoBindingList.h
@@ -56,16 +56,18 @@ SERVO_BINDING_FUNC(Servo_StyleSheet_HasR
 SERVO_BINDING_FUNC(Servo_StyleSheet_GetRules, ServoCssRulesStrong,
                    RawServoStyleSheetContentsBorrowed sheet)
 SERVO_BINDING_FUNC(Servo_StyleSheet_Clone, RawServoStyleSheetContentsStrong,
                    RawServoStyleSheetContentsBorrowed sheet,
                    const mozilla::ServoStyleSheet* reference_sheet);
 SERVO_BINDING_FUNC(Servo_StyleSheet_SizeOfIncludingThis, size_t,
                    mozilla::MallocSizeOf malloc_size_of,
                    RawServoStyleSheetContentsBorrowed sheet)
+SERVO_BINDING_FUNC(Servo_StyleSheet_GetSourceMapURL, void,
+                   RawServoStyleSheetContentsBorrowed sheet, nsAString* result)
 // We'd like to return `OriginFlags` here, but bindgen bitfield enums don't
 // work as return values with the Linux 32-bit ABI at the moment because
 // they wrap the value in a struct.
 SERVO_BINDING_FUNC(Servo_StyleSheet_GetOrigin, uint8_t,
                    RawServoStyleSheetContentsBorrowed sheet)
 SERVO_BINDING_FUNC(Servo_StyleSet_Init, RawServoStyleSet*, RawGeckoPresContextOwned pres_context)
 SERVO_BINDING_FUNC(Servo_StyleSet_RebuildCachedData, void,
                    RawServoStyleSetBorrowed set)
--- a/layout/style/ServoStyleSheet.cpp
+++ b/layout/style/ServoStyleSheet.cpp
@@ -213,16 +213,20 @@ ServoStyleSheet::ParseSheet(css::Loader*
                                                       aInput.Length(),
                                                       mParsingMode,
                                                       extraData,
                                                       aLineNumber,
                                                       aCompatMode,
                                                       aReusableSheets)
                          .Consume();
 
+  nsString sourceMapURL;
+  Servo_StyleSheet_GetSourceMapURL(Inner()->mContents, &sourceMapURL);
+  SetSourceMapURLFromComment(sourceMapURL);
+
   Inner()->mURLData = extraData.forget();
   return NS_OK;
 }
 
 nsresult
 ServoStyleSheet::ReparseSheet(const nsAString& aInput)
 {
   if (!mInner->mComplete) {
--- a/layout/style/StyleSheet.cpp
+++ b/layout/style/StyleSheet.cpp
@@ -255,16 +255,17 @@ StyleSheetInfo::StyleSheetInfo(StyleShee
   , mPrincipal(aCopy.mPrincipal)
   , mCORSMode(aCopy.mCORSMode)
   , mReferrerPolicy(aCopy.mReferrerPolicy)
   , mIntegrity(aCopy.mIntegrity)
   , mComplete(aCopy.mComplete)
   , mFirstChild()  // We don't rebuild the child because we're making a copy
                    // without children.
   , mSourceMapURL(aCopy.mSourceMapURL)
+  , mSourceMapURLFromComment(aCopy.mSourceMapURLFromComment)
 #ifdef DEBUG
   , mPrincipalSet(aCopy.mPrincipalSet)
 #endif
 {
   AddSheet(aPrimarySheet);
 }
 
 StyleSheetInfo::~StyleSheetInfo()
@@ -505,25 +506,35 @@ StyleSheet::GetCssRules(nsIPrincipal& aS
     return nullptr;
   }
   FORWARD_INTERNAL(GetCssRulesInternal, ())
 }
 
 void
 StyleSheet::GetSourceMapURL(nsAString& aSourceMapURL)
 {
-  aSourceMapURL = mInner->mSourceMapURL;
+  if (mInner->mSourceMapURL.IsEmpty()) {
+    aSourceMapURL = mInner->mSourceMapURLFromComment;
+  } else {
+    aSourceMapURL = mInner->mSourceMapURL;
+  }
 }
 
 void
 StyleSheet::SetSourceMapURL(const nsAString& aSourceMapURL)
 {
   mInner->mSourceMapURL = aSourceMapURL;
 }
 
+void
+StyleSheet::SetSourceMapURLFromComment(const nsAString& aSourceMapURLFromComment)
+{
+  mInner->mSourceMapURLFromComment = aSourceMapURLFromComment;
+}
+
 css::Rule*
 StyleSheet::GetDOMOwnerRule() const
 {
   return mOwnerRule;
 }
 
 uint32_t
 StyleSheet::InsertRule(const nsAString& aRule, uint32_t aIndex,
--- a/layout/style/StyleSheet.h
+++ b/layout/style/StyleSheet.h
@@ -210,16 +210,17 @@ public:
   // GetOwnerNode is defined above.
   inline StyleSheet* GetParentStyleSheet() const;
   // The XPCOM GetTitle is fine for WebIDL.
   dom::MediaList* Media();
   bool Disabled() const { return mDisabled; }
   // The XPCOM SetDisabled is fine for WebIDL.
   void GetSourceMapURL(nsAString& aTitle);
   void SetSourceMapURL(const nsAString& aSourceMapURL);
+  void SetSourceMapURLFromComment(const nsAString& aSourceMapURLFromComment);
 
   // WebIDL CSSStyleSheet API
   // Can't be inline because we can't include ImportRule here.  And can't be
   // called GetOwnerRule because that would be ambiguous with the ImportRule
   // version.
   css::Rule* GetDOMOwnerRule() const;
   dom::CSSRuleList* GetCssRules(nsIPrincipal& aSubjectPrincipal,
                                 ErrorResult& aRv);
--- a/layout/style/StyleSheetInfo.h
+++ b/layout/style/StyleSheetInfo.h
@@ -60,16 +60,21 @@ struct StyleSheetInfo
   // throughout its parent chain and things are good.
   RefPtr<StyleSheet>     mFirstChild;
   AutoTArray<StyleSheet*, 8> mSheets;
 
   // 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;
 
 #ifdef DEBUG
   bool                   mPrincipalSet;
 #endif
 };
 
 } // namespace mozilla
 
--- a/layout/style/nsCSSParser.cpp
+++ b/layout/style/nsCSSParser.cpp
@@ -1641,16 +1641,18 @@ CSSParserImpl::ParseSheet(const nsAStrin
       ParseAtRule(AppendRuleToSheet, this, false);
       continue;
     }
     UngetToken();
     if (ParseRuleSet(AppendRuleToSheet, this)) {
       mSection = eCSSSection_General;
     }
   }
+
+  mSheet->SetSourceMapURLFromComment(scanner.GetSourceMapURL());
   ReleaseScanner();
 
   mParsingMode = css::eAuthorSheetFeatures;
   mIsChrome = false;
   mReusableSheets = nullptr;
 
   return NS_OK;
 }
--- a/layout/style/nsCSSScanner.cpp
+++ b/layout/style/nsCSSScanner.cpp
@@ -543,42 +543,88 @@ nsCSSScanner::SkipWhitespace()
 }
 
 /**
  * Skip over one CSS comment starting at the current read position.
  */
 void
 nsCSSScanner::SkipComment()
 {
+  static const char sourceMappingURLDirective[] = "# sourceMappingURL=";
+
   MOZ_ASSERT(Peek() == '/' && Peek(1) == '*', "should not have been called");
   Advance(2);
+  // Look in each comment for a source map directive; using a simple
+  // state machine.  The states are:
+  // * sourceMapIndex >= 0 means that we're still looking for the
+  //   directive and expect the next character to be at that index of
+  //   sourceMappingURLDirective.
+  //   As a special case, when sourceMapIndex == 0, '@' is also recognized.
+  // * sourceMapIndex < 0 means that we don't need to look for the
+  //   directive any more -- whether it was found or not.
+  // * copying == true means that the directive was found and we're
+  //   copying characters into mSourceMapURL.  This stops at the first
+  //   whitespace, or at the end of the comment.
+  int sourceMapIndex = 0;
+  bool copying = false;
   for (;;) {
     int32_t ch = Peek();
     if (ch < 0) {
       if (mReporter)
         mReporter->ReportUnexpectedEOF("PECommentEOF");
       SetEOFCharacters(eEOFCharacters_Asterisk | eEOFCharacters_Slash);
       return;
     }
+    if (sourceMapIndex >= 0) {
+      if ((sourceMapIndex == 0 && ch == '@') || ch == sourceMappingURLDirective[sourceMapIndex]) {
+        ++sourceMapIndex;
+        if (sourceMappingURLDirective[sourceMapIndex] == '\0') {
+          sourceMapIndex = -1;
+          mSourceMapURL.Truncate();
+          copying = true;
+          Advance();
+          // Make sure we don't copy out the '=' by falling through.
+          continue;
+        }
+      } else {
+        // Did not see the directive.
+        sourceMapIndex = -1;
+      }
+    }
+
     if (ch == '*') {
       Advance();
       ch = Peek();
       if (ch < 0) {
+        // In this case, even if we saw a source map directive, leave
+        // the "*" out of it.
         if (mReporter)
           mReporter->ReportUnexpectedEOF("PECommentEOF");
         SetEOFCharacters(eEOFCharacters_Slash);
         return;
       }
       if (ch == '/') {
         Advance();
         return;
       }
+      if (copying) {
+        mSourceMapURL.Append('*');
+      }
     } else if (IsVertSpace(ch)) {
       AdvanceLine();
+      // Done with the directive, so stop copying.
+      copying = false;
+    } else if (IsWhitespace(ch)) {
+      Advance();
+      // Done with the directive, so stop copying.
+      copying = false;
     } else {
+      if (copying) {
+        mSourceMapURL.Append(ch);
+      }
       Advance();
     }
   }
 }
 
 /**
  * If there is a valid escape sequence starting at the current read
  * position, consume it, decode it, append the result to |aOutput|,
--- a/layout/style/nsCSSScanner.h
+++ b/layout/style/nsCSSScanner.h
@@ -227,16 +227,19 @@ class nsCSSScanner {
   { return mTokenOffset - mTokenLineOffset; }
 
   uint32_t GetTokenOffset() const
   { return mTokenOffset; }
 
   uint32_t GetTokenEndOffset() const
   { return mOffset; }
 
+  const nsAString& GetSourceMapURL() const
+  { return mSourceMapURL; }
+
   // Get the text of the line containing the first character of
   // the most recently processed token.
   nsDependentSubstring GetCurrentLine() const;
 
   // Get the next token.  Return false on EOF.  aTokenResult is filled
   // in with the data for the token.  aSkip controls whether
   // whitespace and/or comment tokens are ever returned.
   bool Next(nsCSSToken& aTokenResult, nsCSSScannerExclude aSkip);
@@ -356,16 +359,18 @@ protected:
   uint32_t mRecordStartOffset;
   EOFCharacters mEOFCharacters;
 
   mozilla::css::ErrorReporter *mReporter;
 
   bool mRecording;
   bool mSeenBadToken;
   bool mSeenVariableReference;
+
+  nsString mSourceMapURL;
 };
 
 // Token for the grid-template-areas micro-syntax
 // http://dev.w3.org/csswg/css-grid/#propdef-grid-template-areas
 struct MOZ_STACK_CLASS nsCSSGridTemplateAreaToken {
   nsAutoString mName;  // Empty for a null cell, non-empty for a named cell
   bool isTrash;  // True for a trash token, mName is ignored in this case.
 };
--- a/layout/style/test/browser.ini
+++ b/layout/style/test/browser.ini
@@ -8,8 +8,9 @@ support-files =
   mapped2.css
   mapped2.css^headers^
   sourcemap_css.html
 
 [browser_bug453896.js]
 [browser_newtab_share_rule_processors.js]
 skip-if = stylo # Gecko-specific test
 [browser_sourcemap.js]
+[browser_sourcemap_comment.js]
new file mode 100644
--- /dev/null
+++ b/layout/style/test/browser_sourcemap_comment.js
@@ -0,0 +1,40 @@
+add_task(async function() {
+  // Test text and expected results.
+  let test_cases = [
+    ["/*# sourceMappingURL=here*/", "here"],
+    ["/*# sourceMappingURL=here  */", "here"],
+    ["/*@ sourceMappingURL=here*/", "here"],
+    ["/*@ sourceMappingURL=there*/ /*# sourceMappingURL=here*/", "here"],
+    ["/*# sourceMappingURL=here there  */", "here"],
+
+    ["/*# sourceMappingURL=  here  */", ""],
+    ["/*# sourceMappingURL=*/", ""],
+    ["/*# sourceMappingUR=here  */", ""],
+    ["/*! sourceMappingURL=here  */", ""],
+    ["/*# sourceMappingURL = here  */", ""],
+    ["/*   # sourceMappingURL=here   */", ""],
+  ];
+
+  let page = "<!DOCTYPE HTML>\n<html>\n<head>\n";
+  for (let i = 0; i < test_cases.length; ++i) {
+    page += `<style type="text/css"> #x${i} { color: red; }${test_cases[i][0]}</style>\n`;
+  }
+  page += "</head><body>some text</body></html>";
+
+  let uri = "data:text/html;base64," + btoa(page);
+  info(`URI is ${uri}`);
+
+  await BrowserTestUtils.withNewTab({
+    gBrowser,
+    url: uri
+  }, async function(browser) {
+    await ContentTask.spawn(browser, test_cases, function* (tests) {
+      for (let i = 0; i < content.document.styleSheets.length; ++i) {
+        let sheet = content.document.styleSheets[i];
+
+        info(`Checking sheet #${i}`);
+        is(sheet.sourceMapURL, tests[i][1], `correct source map for sheet ${i}`);
+      }
+    });
+  });
+});
--- a/layout/style/test/mapped2.css
+++ b/layout/style/test/mapped2.css
@@ -1,3 +1,4 @@
 span {
   color: #f06;
 }
+//# sourceMappingURL: overridden-by-headers