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
authorSeth Fowler <seth@mozilla.com>
Wed, 03 Oct 2012 15:28:57 -0700
changeset 109269 94ab602328ff6a2ab9b2daf75125695038ad57db
parent 109268 41d840c9cf301290affa11fe7bcc67711dfbeb84
child 109270 b2735492746e2f0f08c7aa7183e1a9847338d8ce
push id82
push usershu@rfrn.org
push dateFri, 05 Oct 2012 13:20:22 +0000
reviewersbz
bugs786108
milestone18.0a1
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||browserIsRemote) load 786108-1.html # Bug 795534
+skip-if(browserIsRemote) load 786108-2.html # Bug 795534
 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