| Summary: | Add SPI to defer running async script until after document load | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Wenson Hsieh <wenson_hsieh> | ||||||
| Component: | WebKit Misc. | Assignee: | Wenson Hsieh <wenson_hsieh> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | bdakin, cdumez, commit-queue, dbates, esprehn+autocc, ews-watchlist, ggaren, kangil.han, rniwa, thorton, webkit-bug-importer | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
|
Description
Wenson Hsieh
2018-07-17 23:27:53 PDT
Created attachment 345229 [details]
First pass
Comment on attachment 345229 [details] First pass View in context: https://bugs.webkit.org/attachment.cgi?id=345229&action=review Seems reasonable but Iād like a rniwa or someone like that review. > Source/WebKit/UIProcess/API/Cocoa/WKWebViewConfigurationPrivate.h:76 > +@property (nonatomic, setter=_setShouldRunAsynchronousScriptsAfterDocumentLoad:) BOOL _shouldRunAsynchronousScriptsAfterDocumentLoad WK_API_AVAILABLE(macosx(WK_MAC_TBA), ios(WK_IOS_TBA)); maybe shouldDeferAsynchronousScriptsUntilAfterDocumentLoad? Or maybe your name is ok. something is a little weird Comment on attachment 345229 [details] First pass View in context: https://bugs.webkit.org/attachment.cgi?id=345229&action=review > Source/WebCore/dom/ScriptRunner.cpp:109 > + if (m_document.shouldDeferAsynchronousScriptsUntilParsingFinishes()) > + m_scriptsToExecuteAfterDocumentFinishesParsing.append(pendingAsyncScript); > + else > + m_scriptsToExecuteSoon.append(pendingAsyncScript); I don't think it makes sense to have a separate vector for this since m_scriptsToExecuteAfterDocumentFinishesParsing and m_scriptsToExecuteSoon should never have entires at the same time. Instead, ScriptRunner::timerFired() should check m_document.shouldDeferAsynchronousScriptsUntilParsingFinishes() and avoid executing scripts in m_scriptsToExecuteSoon. Comment on attachment 345229 [details] First pass View in context: https://bugs.webkit.org/attachment.cgi?id=345229&action=review > Tools/TestWebKitAPI/Tests/WebKitCocoa/RunScriptAfterDocumentLoad.mm:39 > + "<head><script src='async.js' async></script></head>" Make sure load event on this script element isn't fired until the parsing finishes as well. Comment on attachment 345229 [details] First pass View in context: https://bugs.webkit.org/attachment.cgi?id=345229&action=review >> Source/WebCore/dom/ScriptRunner.cpp:109 >> + m_scriptsToExecuteSoon.append(pendingAsyncScript); > > I don't think it makes sense to have a separate vector for this since > m_scriptsToExecuteAfterDocumentFinishesParsing and m_scriptsToExecuteSoon > should never have entires at the same time. > > Instead, ScriptRunner::timerFired() should check m_document.shouldDeferAsynchronousScriptsUntilParsingFinishes() > and avoid executing scripts in m_scriptsToExecuteSoon. Good idea. This makes the patch even simpler, as well! >> Source/WebKit/UIProcess/API/Cocoa/WKWebViewConfigurationPrivate.h:76 >> +@property (nonatomic, setter=_setShouldRunAsynchronousScriptsAfterDocumentLoad:) BOOL _shouldRunAsynchronousScriptsAfterDocumentLoad WK_API_AVAILABLE(macosx(WK_MAC_TBA), ios(WK_IOS_TBA)); > > maybe shouldDeferAsynchronousScriptsUntilAfterDocumentLoad? Or maybe your name is ok. something is a little weird I like shouldDeferAsynchronousScriptsUntilAfterDocumentLoad better too! Renamed. >> Tools/TestWebKitAPI/Tests/WebKitCocoa/RunScriptAfterDocumentLoad.mm:39 >> + "<head><script src='async.js' async></script></head>" > > Make sure load event on this script element isn't fired until the parsing finishes as well. š Added this to my test Created attachment 345250 [details]
Patch for EWS
Comment on attachment 345250 [details] Patch for EWS Clearing flags on attachment: 345250 Committed r233915: <https://trac.webkit.org/changeset/233915> |