[webkit-reviews] review granted: [Bug 187682] Add an SPI hook to allow clients to yield document parsing and script execution : [Attachment 345114] Address feedback.

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has granted Wenson Hsieh
<wenson_hsieh at apple.com>'s request for review:
Bug 187682: Add an SPI hook to allow clients to yield document parsing and
script execution
https://bugs.webkit.org/show_bug.cgi?id=187682

Attachment 345114: Address feedback.

https://bugs.webkit.org/attachment.cgi?id=345114&action=review




--- Comment #10 from Ryosuke Niwa <rniwa at webkit.org> ---
Comment on attachment 345114
  --> https://bugs.webkit.org/attachment.cgi?id=345114
Address feedback.

View in context: https://bugs.webkit.org/attachment.cgi?id=345114&action=review

> Source/WebCore/dom/Document.h:1269
> +    class ParserYieldToken {

I don't think this needs to be an inner class of Document.

> Source/WebCore/html/parser/HTMLParserScheduler.h:90
> +    void didBeginYieldingParser() { m_documentHasActiveParserYieldTokens =
true; }
> +    void didEndYieldingParser() { m_documentHasActiveParserYieldTokens =
false; }

Let's assert that m_documentHasActiveParserYieldTokens is false & true
respectively here.

> Tools/TestWebKitAPI/Tests/WebKitCocoa/ParserYieldTokenTests.mm:93
> +    waitForDelay(1_s);

Instead of waiting for 1s like this, can we have:
1. <script src="a.js" async></a>
2. <script>fetch('a.js').then(/* 3. post-to-test */)</script>
and make sure (2) happens before (1) runs?

> Tools/TestWebKitAPI/Tests/WebKitCocoa/ParserYieldTokenTests.mm:110
> +    waitForDelay(0.5_s);

Ditto.

> Tools/TestWebKitAPI/Tests/WebKitCocoa/ParserYieldTokenTests.mm:134
> +    waitForDelay(0.5_s);

Ditto.