Avoid multiple copies of inline script & style strings
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 5 Jun 2013 21:20:40 +0000 (21:20 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 5 Jun 2013 21:20:40 +0000 (21:20 +0000)
https://bugs.webkit.org/show_bug.cgi?id=117202

Patch by Arunprasad Rajkumar <arurajku@cisco.com> on 2013-06-05
Reviewed by Darin Adler.

Merge from https://chromiumcodereview.appspot.com/16005007.

No new tests needed.

The HTML parser breaks up large text nodes into small chunks to avoid some
O(n^2) editing algorithms. This fix skips that workaround for <script> and
<style> elements, which aren't likely to need editing. As a result, <script>
ends up with a single text node, containing a contiguous String, which is the
source code of that inline script block.

Prior this fix, we could end up with two copies of large inline scripts: one
monolithic string retained by JSC and a number of shards retained by the DOM.
After this fix, both the DOM and JSC use the same monolithic string, removing a
copy.

* dom/Text.cpp:
(WebCore::Text::createWithLengthLimit):
* html/parser/HTMLConstructionSite.cpp:
(WebCore::shouldUseLengthLimit):
(WebCore::HTMLConstructionSite::insertTextNode):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@151236 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/dom/Text.cpp
Source/WebCore/html/parser/HTMLConstructionSite.cpp

index d8c440a..ab6b53a 100644 (file)
@@ -1,3 +1,31 @@
+2013-06-05  Arunprasad Rajkumar  <arurajku@cisco.com>
+
+        Avoid multiple copies of inline script & style strings
+        https://bugs.webkit.org/show_bug.cgi?id=117202
+
+        Reviewed by Darin Adler.
+
+        Merge from https://chromiumcodereview.appspot.com/16005007.
+
+        No new tests needed.
+
+        The HTML parser breaks up large text nodes into small chunks to avoid some
+        O(n^2) editing algorithms. This fix skips that workaround for <script> and
+        <style> elements, which aren't likely to need editing. As a result, <script>
+        ends up with a single text node, containing a contiguous String, which is the
+        source code of that inline script block.
+
+        Prior this fix, we could end up with two copies of large inline scripts: one
+        monolithic string retained by JSC and a number of shards retained by the DOM.
+        After this fix, both the DOM and JSC use the same monolithic string, removing a
+        copy.
+
+        * dom/Text.cpp:
+        (WebCore::Text::createWithLengthLimit):
+        * html/parser/HTMLConstructionSite.cpp:
+        (WebCore::shouldUseLengthLimit):
+        (WebCore::HTMLConstructionSite::insertTextNode):
+
 2013-06-05  Kondapally Kalyan  <kalyan.kondapally@intel.com>
 
         [EFL] Build fix with EGL and GLES2 backend.
index cf36e52..736c932 100644 (file)
@@ -326,15 +326,15 @@ PassRefPtr<Text> Text::virtualCreate(const String& data)
     return create(document(), data);
 }
 
-PassRefPtr<Text> Text::createWithLengthLimit(Document* document, const String& data, unsigned start, unsigned maxChars)
+PassRefPtr<Text> Text::createWithLengthLimit(Document* document, const String& data, unsigned start, unsigned lengthLimit)
 {
     unsigned dataLength = data.length();
 
-    if (!start && dataLength <= maxChars)
+    if (!start && dataLength <= lengthLimit)
         return create(document, data);
 
     RefPtr<Text> result = Text::create(document, String());
-    result->parserAppendData(data, start, maxChars);
+    result->parserAppendData(data, start, lengthLimit);
 
     return result;
 }
index 4fd43fa..4f3a774 100644 (file)
@@ -75,6 +75,13 @@ static bool hasImpliedEndTag(const HTMLStackItem* item)
         || item->hasTagName(rtTag);
 }
 
+static bool shouldUseLengthLimit(const ContainerNode* node)
+{
+    return !node->hasTagName(scriptTag)
+        && !node->hasTagName(styleTag)
+        && !node->hasTagName(SVGNames::scriptTag);
+}
+
 static inline bool isAllWhitespace(const String& string)
 {
     return string.isAllSpecialCharacters<isHTMLSpace>();
@@ -490,6 +497,7 @@ void HTMLConstructionSite::insertTextNode(const String& characters, WhitespaceMo
         || (whitespaceMode == WhitespaceUnknown && isAllWhitespace(characters));
 
     unsigned currentPosition = 0;
+    unsigned lengthLimit = shouldUseLengthLimit(task.parent.get()) ? Text::defaultLengthLimit : std::numeric_limits<unsigned>::max();
 
     // FIXME: Splitting text nodes into smaller chunks contradicts HTML5 spec, but is currently necessary
     // for performance, see <https://bugs.webkit.org/show_bug.cgi?id=55898>.
@@ -499,11 +507,11 @@ void HTMLConstructionSite::insertTextNode(const String& characters, WhitespaceMo
         // FIXME: We're only supposed to append to this text node if it
         // was the last text node inserted by the parser.
         CharacterData* textNode = static_cast<CharacterData*>(previousChild);
-        currentPosition = textNode->parserAppendData(characters, 0, Text::defaultLengthLimit);
+        currentPosition = textNode->parserAppendData(characters, 0, lengthLimit);
     }
 
     while (currentPosition < characters.length()) {
-        RefPtr<Text> textNode = Text::createWithLengthLimit(task.parent->document(), shouldUseAtomicString ? AtomicString(characters).string() : characters, currentPosition);
+        RefPtr<Text> textNode = Text::createWithLengthLimit(task.parent->document(), shouldUseAtomicString ? AtomicString(characters).string() : characters, currentPosition, lengthLimit);
         // If we have a whole string of unbreakable characters the above could lead to an infinite loop. Exceeding the length limit is the lesser evil.
         if (!textNode->length()) {
             String substring = characters.substring(currentPosition);