Bug 786108 - Cache UTF-16 version of URI to prevent repeated conversions in the CSS scanner, and free the cache after a short time. r=bz
☠☠ backed out by 5fbf17bdec32 ☠ ☠
authorSeth Fowler <seth@mozilla.com>
Fri, 28 Sep 2012 09:56:47 -0700
changeset 115382 ec0dcd401a3ff735815f4209a7ba26d57bcf87a1
parent 115381 8e0593f974bb0b79e2b7d099444df1595d48c70b
child 115383 51644b3ee33365d9464c1ca058d3773f2e7f6889
push id239
push userakeybl@mozilla.com
push dateThu, 03 Jan 2013 21:54:43 +0000
treeherdermozilla-release@3a7b66445659 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs786108
milestone18.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 786108 - Cache UTF-16 version of URI to prevent repeated conversions in the CSS scanner, and free the cache after a short time. r=bz
layout/style/crashtests/786108-1.html
layout/style/crashtests/786108-2.html
layout/style/crashtests/crashtests.list
layout/style/nsCSSScanner.cpp
layout/style/nsCSSScanner.h
new file mode 100644
--- /dev/null
+++ b/layout/style/crashtests/786108-1.html
@@ -0,0 +1,22 @@
+<html>
+  <head></head>
+  <body></body>
+  <script type="text/javascript">
+    // Detect severe performance and memory issues when large amounts of errors
+    // are reported from CSS embedded in a file with a long data URI. Addressed
+    // by 786108; should finish quickly with that patch and run for a very long
+    // time otherwise.
+
+    var img = new Array;
+    img.push('<img src="data:image/svg+xml,');
+    img.push(encodeURIComponent('<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" width="300px" height="300px">'));
+
+    for (var i = 0 ; i < 10000 ; i++)
+      img.push(encodeURIComponent('<circle cx="0" cy="0" r="1" style="xxx-invalid-property: 0;"/>'));
+
+    img.push(encodeURIComponent('</svg>'));
+    img.push('">');
+
+    document.getElementsByTagName('body')[0].innerHTML = img.join('');
+  </script>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/style/crashtests/786108-2.html
@@ -0,0 +1,23 @@
+<html>
+  <head></head>
+  <body></body>
+  <script type="text/javascript">
+    // Detect severe performance and memory issues when large amounts of errors
+    // are reported from CSS embedded in a file with a long data URI. Addressed
+    // by 786108; should finish quickly with that patch and run for a very long
+    // time otherwise. This version is designed for slow / memory constrained
+    // platforms like Android.
+
+    var img = new Array;
+    img.push('<img src="data:image/svg+xml,');
+    img.push(encodeURIComponent('<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" width="300px" height="300px">'));
+
+    for (var i = 0 ; i < 2500 ; i++)
+      img.push(encodeURIComponent('<circle cx="0" cy="0" r="1" style="xxx-invalid-property: 0;"/>'));
+
+    img.push(encodeURIComponent('</svg>'));
+    img.push('">');
+
+    document.getElementsByTagName('body')[0].innerHTML = img.join('');
+  </script>
+</html>
--- a/layout/style/crashtests/crashtests.list
+++ b/layout/style/crashtests/crashtests.list
@@ -72,9 +72,11 @@ load 611922-1.html
 load 665209-1.html
 asserts(2) load 671799-1.html
 asserts(2) load 671799-2.html
 load 690990-1.html
 load 696188-1.html
 load 700116.html
 load 729126-1.html
 load 729126-2.html
+skip-if(Android) load 786108-1.html
+load 786108-2.html
 load 788836.html
--- a/layout/style/nsCSSScanner.cpp
+++ b/layout/style/nsCSSScanner.cpp
@@ -244,16 +244,39 @@ nsCSSToken::AppendToString(nsString& aBu
       aBuffer.Append(mIdent);
       break;
     default:
       NS_ERROR("invalid token type");
       break;
   }
 }
 
+class DeferredCleanupRunnable : public nsRunnable
+{
+public:
+  DeferredCleanupRunnable(nsCSSScanner* aToClean)
+    : mToClean(aToClean)
+  {}
+
+  NS_IMETHOD Run() {
+    if (mToClean) {
+      mToClean->PerformDeferredCleanup();
+    }
+
+    return NS_OK;
+  }
+
+  void Revoke() {
+    mToClean = nullptr;
+  }
+
+private:
+  nsCSSScanner* mToClean;
+};
+
 nsCSSScanner::nsCSSScanner()
   : mReadPointer(nullptr)
   , mSVGMode(false)
 #ifdef CSS_REPORT_PARSE_ERRORS
   , mError(mErrorBuf, ArrayLength(mErrorBuf), 0)
   , mInnerWindowID(0)
   , mWindowIDCached(false)
   , mSheet(nullptr)
@@ -266,23 +289,34 @@ nsCSSScanner::nsCSSScanner()
   // No need to init the other members, since they represent state
   // which can get cleared.  We'll init them every time Init() is
   // called.
 }
 
 nsCSSScanner::~nsCSSScanner()
 {
   MOZ_COUNT_DTOR(nsCSSScanner);
-  Close();
+  Reset();
   if (mLocalPushback != mPushback) {
     delete [] mPushback;
   }
 }
 
 #ifdef CSS_REPORT_PARSE_ERRORS
+void
+nsCSSScanner::PerformDeferredCleanup()
+{
+  // Clean up all short term caches.
+  mCachedURI = nullptr;
+  mCachedFileName.Truncate();
+
+  // Release our DeferredCleanupRunnable.
+  mDeferredCleaner.Forget();
+}
+
 #define CSS_ERRORS_PREF "layout.css.report_errors"
 
 static int
 CSSErrorsPrefChanged(const char *aPref, void *aClosure)
 {
   gReportErrors = Preferences::GetBool(CSS_ERRORS_PREF, true);
   return 0;
 }
@@ -326,27 +360,23 @@ nsCSSScanner::Init(const nsAString& aBuf
                    nsCSSStyleSheet* aSheet, mozilla::css::Loader* aLoader)
 {
   NS_PRECONDITION(!mReadPointer, "Should not have an existing input buffer!");
 
   mReadPointer = aBuffer.BeginReading();
   mCount = aBuffer.Length();
 
 #ifdef CSS_REPORT_PARSE_ERRORS
-  // If aURI is the same as mURI, no need to reget mFileName -- it
-  // shouldn't have changed.
-  if (aURI != mURI) {
-    mURI = aURI;
-    if (aURI) {
-      aURI->GetSpec(mFileName);
-    } else {
-      mFileName.Adopt(NS_strdup("from DOM"));
-    }
+  // If aURI is different from mCachedURI, invalidate the filename cache.
+  if (aURI != mCachedURI) {
+    mCachedURI = aURI;
+    mCachedFileName.Truncate();
   }
 #endif // CSS_REPORT_PARSE_ERRORS
+
   mLineNumber = aLineNumber;
 
   // Reset variables that we use to keep track of our progress through the input
   mOffset = 0;
   mPushbackCount = 0;
   mRecording = false;
 
 #ifdef CSS_REPORT_PARSE_ERRORS
@@ -401,18 +431,29 @@ nsCSSScanner::OutputError()
       mWindowIDCached = true;
     }
 
     nsresult rv;
     nsCOMPtr<nsIScriptError> errorObject =
       do_CreateInstance(gScriptErrorFactory, &rv);
 
     if (NS_SUCCEEDED(rv)) {
+      // Update the cached filename if needed.
+      if (mCachedFileName.IsEmpty()) {
+        if (mCachedURI) {
+          nsAutoCString cFileName;
+          mCachedURI->GetSpec(cFileName);
+          CopyUTF8toUTF16(cFileName, mCachedFileName);
+        } else {
+          mCachedFileName.AssignLiteral("from DOM");
+        }
+      }
+
       rv = errorObject->InitWithWindowID(mError,
-                                         NS_ConvertUTF8toUTF16(mFileName),
+                                         mCachedFileName,
                                          EmptyString(),
                                          mErrorLineNumber,
                                          mErrorColNumber,
                                          nsIScriptError::warningFlag,
                                          "CSS Parser",
                                          mInnerWindowID);
       if (NS_SUCCEEDED(rv)) {
         gConsoleService->LogMessage(errorObject);
@@ -550,28 +591,46 @@ nsCSSScanner::ReportUnexpectedTokenParam
 
 #define REPORT_UNEXPECTED_EOF(lf_)
 
 #endif // CSS_REPORT_PARSE_ERRORS
 
 void
 nsCSSScanner::Close()
 {
+  Reset();
+
+  // Schedule deferred cleanup for cached data. We want to strike a balance
+  // between performance and memory usage, so we only allow short-term caching.
+#ifdef CSS_REPORT_PARSE_ERRORS
+  if (!mDeferredCleaner.IsPending()) {
+    mDeferredCleaner = new DeferredCleanupRunnable(this);
+    if (NS_FAILED(NS_DispatchToCurrentThread(mDeferredCleaner.get()))) {
+      // Peform the "deferred" cleanup immediately if the dispatch fails.
+      // This will also have the effect of clearing mDeferredCleaner.
+      nsCSSScanner::PerformDeferredCleanup();
+    }
+  }
+#endif
+}
+
+void
+nsCSSScanner::Reset()
+{
   mReadPointer = nullptr;
 
   // Clean things up so we don't hold on to memory if our parser gets recycled.
 #ifdef CSS_REPORT_PARSE_ERRORS
-  mFileName.Truncate();
-  mURI = nullptr;
   mError.Truncate();
   mInnerWindowID = 0;
   mWindowIDCached = false;
   mSheet = nullptr;
   mLoader = nullptr;
 #endif
+
   if (mPushback != mLocalPushback) {
     delete [] mPushback;
     mPushback = mLocalPushback;
     mPushbackSize = ArrayLength(mLocalPushback);
   }
 }
 
 // Returns -1 on error or eof
--- a/layout/style/nsCSSScanner.h
+++ b/layout/style/nsCSSScanner.h
@@ -13,16 +13,17 @@
 #include "mozilla/css/Loader.h"
 #include "nsCSSStyleSheet.h"
 
 // XXX turn this off for minimo builds
 #define CSS_REPORT_PARSE_ERRORS
 
 // for #ifdef CSS_REPORT_PARSE_ERRORS
 #include "nsXPIDLString.h"
+#include "nsThreadUtils.h"
 class nsIURI;
 
 // Token types
 enum nsCSSTokenType {
   // A css identifier (e.g. foo)
   eCSSToken_Ident,          // mIdent
 
   // A css at keyword (e.g. @foo)
@@ -86,16 +87,18 @@ struct nsCSSToken {
 
   bool IsSymbol(PRUnichar aSymbol) {
     return bool((eCSSToken_Symbol == mType) && (mSymbol == aSymbol));
   }
 
   void AppendToString(nsString& aBuffer);
 };
 
+class DeferredCleanupRunnable; 
+
 // CSS Scanner API. Used to tokenize an input stream using the CSS
 // forward compatible tokenization rules. This implementation is
 // private to this package and is only used internally by the css
 // parser.
 class nsCSSScanner {
   public:
   nsCSSScanner();
   ~nsCSSScanner();
@@ -115,24 +118,29 @@ class nsCSSScanner {
   void SetSVGMode(bool aSVGMode) {
     mSVGMode = aSVGMode;
   }
   bool IsSVGMode() const {
     return mSVGMode;
   }
 
 #ifdef CSS_REPORT_PARSE_ERRORS
+  // Clean up any reclaimable cached resources.
+  void PerformDeferredCleanup();
+
   void AddToError(const nsSubstring& aErrorText);
   void OutputError();
   void ClearError();
 
   // aMessage must take no parameters
   void ReportUnexpected(const char* aMessage);
   
 private:
+  void Reset();
+
   void ReportUnexpectedParams(const char* aMessage,
                               const PRUnichar** aParams,
                               uint32_t aParamsLength);
 
 public:
   template<uint32_t N>                           
   void ReportUnexpectedParams(const char* aMessage,
                               const PRUnichar* (&aParams)[N])
@@ -206,18 +214,19 @@ protected:
 
   uint32_t mLineNumber;
   // True if we are in SVG mode; false in "normal" CSS
   bool mSVGMode;
   bool mRecording;
   uint32_t mRecordStartOffset;
 
 #ifdef CSS_REPORT_PARSE_ERRORS
-  nsXPIDLCString mFileName;
-  nsCOMPtr<nsIURI> mURI;  // Cached so we know to not refetch mFileName
+  nsRevocableEventPtr<DeferredCleanupRunnable> mDeferredCleaner;
+  nsCOMPtr<nsIURI> mCachedURI;  // Used to invalidate the cached filename.
+  nsString mCachedFileName;
   uint32_t mErrorLineNumber, mColNumber, mErrorColNumber;
   nsFixedString mError;
   PRUnichar mErrorBuf[200];
   uint64_t mInnerWindowID;
   bool mWindowIDCached;
   nsCSSStyleSheet* mSheet;
   mozilla::css::Loader* mLoader;
 #endif