CSSStyleSheetResource should cache decoded text instead of raw bytes

third_party/WebKit/Source/core/fetch/CSSStyleSheetResource.cpp

Index: third_party/WebKit/Source/core/fetch/CSSStyleSheetResource.cpp
diff --git a/third_party/WebKit/Source/core/fetch/CSSStyleSheetResource.cpp b/third_party/WebKit/Source/core/fetch/CSSStyleSheetResource.cpp
index f44d16a3fcff570c6df1692629a9cc5dde000860..a18537844a6f7b638c788b2fa77e97d4c7bdf75b 100644
--- a/third_party/WebKit/Source/core/fetch/CSSStyleSheetResource.cpp
+++ b/third_party/WebKit/Source/core/fetch/CSSStyleSheetResource.cpp
@@ -90,16 +90,25 @@ void CSSStyleSheetResource::didAddClient(ResourceClient* c)
         static_cast<StyleSheetResourceClient*>(c)->setCSSStyleSheet(resourceRequest().url(), response().url(), encoding(), this);
 }
 
-const String CSSStyleSheetResource::sheetText(MIMETypeCheck mimeTypeCheck) const
+const String CSSStyleSheetResource::sheetText(MIMETypeCheck mimeTypeCheck)
 {
-    if (!data() || data()->isEmpty() || !canUseSheet(mimeTypeCheck))
+    if (!canUseSheet(mimeTypeCheck))
         return String();
 
+    // Only cache decoded sheet text when loading is complete.
+    if (isLoading())
+        return decodedText();
+
+    // Use cached decoded sheet text when available.
     if (!m_decodedSheetText.isNull())
         return m_decodedSheetText;
 
-    // Don't cache the decoded text, regenerating is cheap and it can use quite a bit of memory
-    return decodedText();
+    if (!data() || data()->isEmpty())
+        return String();
+
+    m_decodedSheetText = decodedText();
+    clearData();
+    return m_decodedSheetText;

[hiroshige, 2016/08/31 07:30:25]

IIUC, the Lines 106--111 are executed only in sheetText() called in
checkNotify().
How about splitting out this block to a new method and making checkNotify() to
call the new method, and keeping sheetText() const?
This will clarify when |m_decodedSheetText| is set, and also make this CL much
smaller.

[kouhei (in TOK), 2016/08/31 10:27:43]

Done.

 }
 
 void CSSStyleSheetResource::appendData(const char* data, size_t length)
@@ -116,14 +125,11 @@ void CSSStyleSheetResource::appendData(const char* data, size_t length)
 void CSSStyleSheetResource::checkNotify()
 {
     // Decode the data to find out the encoding and keep the sheet text around during checkNotify()
-    if (data())
-        m_decodedSheetText = decodedText();
+    sheetText();

[hiroshige, 2016/08/31 07:30:25]

I think we call sheetText() here to ensure |m_data| is always null after this
point.
Please add a comment (perhaps somewhere in CssStyleSheetResource.h) about the
lifetime of |m_data| and |m_decodedSheetText|.

[kouhei (in TOK), 2016/08/31 10:27:43]

Done.

 
     ResourceClientWalker<StyleSheetResourceClient> w(clients());
     while (StyleSheetResourceClient* c = w.next())
         c->setCSSStyleSheet(resourceRequest().url(), response().url(), encoding(), this);
-    // Clear the decoded text as it is unlikely to be needed immediately again and is cheap to regenerate.
-    m_decodedSheetText = String();
 }
 
 void CSSStyleSheetResource::destroyDecodedDataIfPossible()