Title: [283803] trunk
Revision
283803
Author
[email protected]
Date
2021-10-08 08:44:53 -0700 (Fri, 08 Oct 2021)

Log Message

REGRESSION (r277818): XHR with requestType document broken for larger HTML files
https://bugs.webkit.org/show_bug.cgi?id=231138
<rdar://problem/83823282>

Reviewed by Darin Adler.

Source/WebCore:

The parser may end up yielding during XHR or DOMParser parsing even though those should be
synchronous. This appears to be a long standing bug that was made more visible by r277818
because it makes the parser yield more eagerly. This only affects cases where the document
being parsed contains <script> tags as those are considered potential yield points.

Tests: http/tests/xmlhttprequest/xhr-and-parse-large-document.html
       http/tests/xmlhttprequest/xhr-large-document.html

* dom/Document.cpp:
(WebCore::Document::setContent):

Explicitly parse synchronously. The comment was wrong, nothing forces parser flushing on close().

* dom/DocumentParser.h:
(WebCore::DocumentParser::appendSynchronously):
* html/parser/HTMLDocumentParser.cpp:
(WebCore::HTMLDocumentParser::append):
(WebCore::HTMLDocumentParser::appendSynchronously):
* html/parser/HTMLDocumentParser.h:
* html/parser/HTMLParserScheduler.cpp:
(WebCore::HTMLParserScheduler::shouldYieldBeforeExecutingScript):

Also avoid script yielding if script execution is not allowed.

LayoutTests:

* http/tests/xmlhttprequest/resources/large-html-with-script-tags.py: Added.
* http/tests/xmlhttprequest/xhr-and-parse-large-document-expected.txt: Added.
* http/tests/xmlhttprequest/xhr-and-parse-large-document.html: Added.
* http/tests/xmlhttprequest/xhr-large-document-expected.txt: Added.
* http/tests/xmlhttprequest/xhr-large-document.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (283802 => 283803)


--- trunk/LayoutTests/ChangeLog	2021-10-08 15:33:21 UTC (rev 283802)
+++ trunk/LayoutTests/ChangeLog	2021-10-08 15:44:53 UTC (rev 283803)
@@ -1,3 +1,17 @@
+2021-10-08  Antti Koivisto  <[email protected]>
+
+        REGRESSION (r277818): XHR with requestType document broken for larger HTML files
+        https://bugs.webkit.org/show_bug.cgi?id=231138
+        <rdar://problem/83823282>
+
+        Reviewed by Darin Adler.
+
+        * http/tests/xmlhttprequest/resources/large-html-with-script-tags.py: Added.
+        * http/tests/xmlhttprequest/xhr-and-parse-large-document-expected.txt: Added.
+        * http/tests/xmlhttprequest/xhr-and-parse-large-document.html: Added.
+        * http/tests/xmlhttprequest/xhr-large-document-expected.txt: Added.
+        * http/tests/xmlhttprequest/xhr-large-document.html: Added.
+
 2021-10-08  Sergio Villar Senin  <[email protected]>
 
         [GTK] web-platform-tests/css/css-flexbox/text-as-flexitem-size-001.html is failing

Added: trunk/LayoutTests/http/tests/xmlhttprequest/resources/large-html-with-script-tags.py (0 => 283803)


--- trunk/LayoutTests/http/tests/xmlhttprequest/resources/large-html-with-script-tags.py	                        (rev 0)
+++ trunk/LayoutTests/http/tests/xmlhttprequest/resources/large-html-with-script-tags.py	2021-10-08 15:44:53 UTC (rev 283803)
@@ -0,0 +1,15 @@
+#!/usr/bin/env python3
+import os
+import sys
+
+sys.stdout.write('Content-Type: text/html\r\n\r\n')
+
+for i in range(64 * 1024):
+    sys.stdout.write('<div>text <span style="color:green">{}</span></div>'.format(i))
+
+sys.stdout.write('<script>var i = 0;</script>'.format(i))
+
+for i in range(64 * 1024):
+    sys.stdout.write('<div>text <span style="color:blue">{}</span></div>'.format(i))
+
+sys.stdout.write('<script>i = 1;</script>'.format(i))
Property changes on: trunk/LayoutTests/http/tests/xmlhttprequest/resources/large-html-with-script-tags.py
___________________________________________________________________

Added: svn:executable

+* \ No newline at end of property

Added: trunk/LayoutTests/http/tests/xmlhttprequest/xhr-and-parse-large-document-expected.txt (0 => 283803)


--- trunk/LayoutTests/http/tests/xmlhttprequest/xhr-and-parse-large-document-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/http/tests/xmlhttprequest/xhr-and-parse-large-document-expected.txt	2021-10-08 15:44:53 UTC (rev 283803)
@@ -0,0 +1,10 @@
+Tests that XMLHttpRequest.document is valid for large HTML documents
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS parsedDocument.body.lastChild.nodeName is "SCRIPT"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: trunk/LayoutTests/http/tests/xmlhttprequest/xhr-and-parse-large-document.html (0 => 283803)


--- trunk/LayoutTests/http/tests/xmlhttprequest/xhr-and-parse-large-document.html	                        (rev 0)
+++ trunk/LayoutTests/http/tests/xmlhttprequest/xhr-and-parse-large-document.html	2021-10-08 15:44:53 UTC (rev 283803)
@@ -0,0 +1,33 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src=""
+<script>
+description("Tests that XMLHttpRequest.document is valid for large HTML documents");
+jsTestIsAsync = true;
+
+let parsedDocument;
+
+let xhr = new XMLHttpRequest();
+xhr.open('GET', 'resources/large-html-with-script-tags.py', true);
+
+xhr.addEventListener('readystatechange', () => {
+  if (xhr.readyState === 4) {
+    const parser = new DOMParser();
+    parsedDocument = parser.parseFromString(xhr.responseText, "text/html")
+
+    shouldBeEqualToString("parsedDocument.body.lastChild.nodeName", "SCRIPT");
+
+    finishJSTest();
+  }
+});
+
+xhr.addEventListener('error', (e) => {
+    testFailed("XHR failed");
+    finishJSTest();
+});
+
+xhr.send();
+</script>
+</body>
+</html>

Added: trunk/LayoutTests/http/tests/xmlhttprequest/xhr-large-document-expected.txt (0 => 283803)


--- trunk/LayoutTests/http/tests/xmlhttprequest/xhr-large-document-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/http/tests/xmlhttprequest/xhr-large-document-expected.txt	2021-10-08 15:44:53 UTC (rev 283803)
@@ -0,0 +1,10 @@
+Tests that XMLHttpRequest.document is valid for large HTML documents
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS xhr.response is not null
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: trunk/LayoutTests/http/tests/xmlhttprequest/xhr-large-document.html (0 => 283803)


--- trunk/LayoutTests/http/tests/xmlhttprequest/xhr-large-document.html	                        (rev 0)
+++ trunk/LayoutTests/http/tests/xmlhttprequest/xhr-large-document.html	2021-10-08 15:44:53 UTC (rev 283803)
@@ -0,0 +1,28 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src=""
+<script>
+description("Tests that XMLHttpRequest.document is valid for large HTML documents");
+jsTestIsAsync = true;
+
+let xhr = new XMLHttpRequest();
+xhr.open('GET', 'resources/large-html-with-script-tags.py', true);
+xhr.responseType = 'document';
+
+xhr.addEventListener('readystatechange', () => {
+  if (xhr.readyState === 4) {
+    shouldNotBe("xhr.response", "null");
+    finishJSTest();
+  }
+});
+
+xhr.addEventListener('error', (e) => {
+    testFailed("XHR failed");
+    finishJSTest();
+});
+
+xhr.send();
+</script>
+</body>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (283802 => 283803)


--- trunk/Source/WebCore/ChangeLog	2021-10-08 15:33:21 UTC (rev 283802)
+++ trunk/Source/WebCore/ChangeLog	2021-10-08 15:44:53 UTC (rev 283803)
@@ -1,3 +1,35 @@
+2021-10-08  Antti Koivisto  <[email protected]>
+
+        REGRESSION (r277818): XHR with requestType document broken for larger HTML files
+        https://bugs.webkit.org/show_bug.cgi?id=231138
+        <rdar://problem/83823282>
+
+        Reviewed by Darin Adler.
+
+        The parser may end up yielding during XHR or DOMParser parsing even though those should be
+        synchronous. This appears to be a long standing bug that was made more visible by r277818
+        because it makes the parser yield more eagerly. This only affects cases where the document
+        being parsed contains <script> tags as those are considered potential yield points.
+
+        Tests: http/tests/xmlhttprequest/xhr-and-parse-large-document.html
+               http/tests/xmlhttprequest/xhr-large-document.html
+
+        * dom/Document.cpp:
+        (WebCore::Document::setContent):
+
+        Explicitly parse synchronously. The comment was wrong, nothing forces parser flushing on close().
+
+        * dom/DocumentParser.h:
+        (WebCore::DocumentParser::appendSynchronously):
+        * html/parser/HTMLDocumentParser.cpp:
+        (WebCore::HTMLDocumentParser::append):
+        (WebCore::HTMLDocumentParser::appendSynchronously):
+        * html/parser/HTMLDocumentParser.h:
+        * html/parser/HTMLParserScheduler.cpp:
+        (WebCore::HTMLParserScheduler::shouldYieldBeforeExecutingScript):
+
+        Also avoid script yielding if script execution is not allowed.
+
 2021-10-08  Claudio Saavedra  <[email protected]>
 
         [LibWPE] Do not destroy EGL backend if not created

Modified: trunk/Source/WebCore/dom/Document.cpp (283802 => 283803)


--- trunk/Source/WebCore/dom/Document.cpp	2021-10-08 15:33:21 UTC (rev 283802)
+++ trunk/Source/WebCore/dom/Document.cpp	2021-10-08 15:44:53 UTC (rev 283803)
@@ -1568,11 +1568,7 @@
 void Document::setContent(const String& content)
 {
     open();
-    // FIXME: This should probably use insert(), but that's (intentionally)
-    // not implemented for the XML parser as it's normally synonymous with
-    // document.write(). append() will end up yielding, but close() will
-    // pump the tokenizer syncrhonously and finish the parse.
-    m_parser->append(content.impl());
+    m_parser->appendSynchronously(content.impl());
     close();
 }
 

Modified: trunk/Source/WebCore/dom/DocumentParser.h (283802 => 283803)


--- trunk/Source/WebCore/dom/DocumentParser.h	2021-10-08 15:33:21 UTC (rev 283802)
+++ trunk/Source/WebCore/dom/DocumentParser.h	2021-10-08 15:44:53 UTC (rev 283803)
@@ -50,10 +50,8 @@
     virtual void appendBytes(DocumentWriter&, const uint8_t* bytes, size_t length) = 0;
     virtual void flush(DocumentWriter&) = 0;
 
-    // FIXME: append() should be private, but DocumentWriter::replaceDocument uses it for now.
-    // FIXME: This really should take a std::unique_ptr to signify that it expects to take
-    // ownership of the buffer. The parser expects the RefPtr to hold the only ref of the StringImpl.
     virtual void append(RefPtr<StringImpl>&&) = 0;
+    virtual void appendSynchronously(RefPtr<StringImpl>&& inputSource) { append(WTFMove(inputSource)); }
 
     virtual void finish() = 0;
 

Modified: trunk/Source/WebCore/html/parser/HTMLDocumentParser.cpp (283802 => 283803)


--- trunk/Source/WebCore/html/parser/HTMLDocumentParser.cpp	2021-10-08 15:33:21 UTC (rev 283802)
+++ trunk/Source/WebCore/html/parser/HTMLDocumentParser.cpp	2021-10-08 15:44:53 UTC (rev 283803)
@@ -394,6 +394,16 @@
 
 void HTMLDocumentParser::append(RefPtr<StringImpl>&& inputSource)
 {
+    append(WTFMove(inputSource), AllowYield);
+}
+
+void HTMLDocumentParser::appendSynchronously(RefPtr<StringImpl>&& inputSource)
+{
+    append(WTFMove(inputSource), ForceSynchronous);
+}
+
+void HTMLDocumentParser::append(RefPtr<StringImpl>&& inputSource, SynchronousMode synchronousMode)
+{
     if (isStopped())
         return;
 
@@ -424,7 +434,7 @@
         return;
     }
 
-    pumpTokenizerIfPossible(AllowYield);
+    pumpTokenizerIfPossible(synchronousMode);
 
     endIfDelayed();
 }

Modified: trunk/Source/WebCore/html/parser/HTMLDocumentParser.h (283802 => 283803)


--- trunk/Source/WebCore/html/parser/HTMLDocumentParser.h	2021-10-08 15:33:21 UTC (rev 283802)
+++ trunk/Source/WebCore/html/parser/HTMLDocumentParser.h	2021-10-08 15:44:53 UTC (rev 283803)
@@ -66,6 +66,7 @@
 
     void insert(SegmentedString&&) final;
     void append(RefPtr<StringImpl>&&) override;
+    void appendSynchronously(RefPtr<StringImpl>&&) override;
     void finish() override;
 
     HTMLTreeBuilder& treeBuilder();
@@ -102,6 +103,8 @@
     Document* contextForParsingSession();
 
     enum SynchronousMode { AllowYield, ForceSynchronous };
+    void append(RefPtr<StringImpl>&&, SynchronousMode);
+
     void pumpTokenizer(SynchronousMode);
     bool pumpTokenizerLoop(SynchronousMode, bool parsingFragment, PumpSession&);
     void pumpTokenizerIfPossible(SynchronousMode);

Modified: trunk/Source/WebCore/html/parser/HTMLParserScheduler.cpp (283802 => 283803)


--- trunk/Source/WebCore/html/parser/HTMLParserScheduler.cpp	2021-10-08 15:33:21 UTC (rev 283802)
+++ trunk/Source/WebCore/html/parser/HTMLParserScheduler.cpp	2021-10-08 15:44:53 UTC (rev 283803)
@@ -28,9 +28,11 @@
 #include "HTMLParserScheduler.h"
 
 #include "Document.h"
+#include "Frame.h"
 #include "FrameView.h"
 #include "HTMLDocumentParser.h"
 #include "Page.h"
+#include "ScriptController.h"
 #include "ScriptElement.h"
 
 namespace WebCore {
@@ -109,6 +111,9 @@
     if (!document->body())
         return false;
 
+    if (!document->frame() || !document->frame()->script().canExecuteScripts(NotAboutToExecuteScript))
+        return false;
+
     if (!document->haveStylesheetsLoaded())
         return false;
 
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to