Bug 208896

Created attachment 393194 [details]
WIP Patch

Created attachment 393239 [details]
Patch

Created attachment 393244 [details]
Patch

Comment on attachment 393244 [details]
Patch

Clearing flags on attachment: 393244

Committed r258268: <https://trac.webkit.org/changeset/258268>

All reviewed patches have been landed.  Closing bug.

<rdar://problem/60329406>

Hi Chris,

Thanks for the quick action, I really appreciate it!

I'm still very new to the WebKit source code, and am trying to understand how the first paint event "notifies" informs the pending async script to start execution.

For DOMContentLoaded, I see that the the ScriptRunner is receives this notice from Document.cpp in its documentFinishedParsing() method. I don't see something similar for first paint that would similarly clear the "don't execute yet" flag.

Maybe it is that the shouldDeferAsynchronousScriptsUntilParsingFinishes condition is automatically re-evaluated after important events such as FP and DCL, but I couldn't find it (and if so, perhaps documentFinishedParsing is redundant in that case?).

(In reply to krinklemail from comment #7)
> Hi Chris,
> 
> Thanks for the quick action, I really appreciate it!
> 
> I'm still very new to the WebKit source code, and am trying to understand
> how the first paint event "notifies" informs the pending async script to
> start execution.
> 
> For DOMContentLoaded, I see that the the ScriptRunner is receives this
> notice from Document.cpp in its documentFinishedParsing() method. I don't
> see something similar for first paint that would similarly clear the "don't
> execute yet" flag.
> 
> Maybe it is that the shouldDeferAsynchronousScriptsUntilParsingFinishes
> condition is automatically re-evaluated after important events such as FP
> and DCL, but I couldn't find it (and if so, perhaps documentFinishedParsing
> is redundant in that case?).

You are right that async scripts may still be delayed longer than they should after first paint. I am working on evaluating performance if we hook up the script runner to the first paint.

(In reply to Chris Dumez from comment #8)
> (In reply to krinklemail from comment #7)
> > Hi Chris,
> > 
> > Thanks for the quick action, I really appreciate it!
> > 
> > I'm still very new to the WebKit source code, and am trying to understand
> > how the first paint event "notifies" informs the pending async script to
> > start execution.
> > 
> > For DOMContentLoaded, I see that the the ScriptRunner is receives this
> > notice from Document.cpp in its documentFinishedParsing() method. I don't
> > see something similar for first paint that would similarly clear the "don't
> > execute yet" flag.
> > 
> > Maybe it is that the shouldDeferAsynchronousScriptsUntilParsingFinishes
> > condition is automatically re-evaluated after important events such as FP
> > and DCL, but I couldn't find it (and if so, perhaps documentFinishedParsing
> > is redundant in that case?).
> 
> You are right that async scripts may still be delayed longer than they
> should after first paint. I am working on evaluating performance if we hook
> up the script runner to the first paint.

I believe the current behavior is that async scripts would run whenever the parser yields after the first paint has happened. It is true that it is not exactly after first paint and that those async scripts may be delayed longer.

This is not an obviously bad behavior but I will compare this and running the async scripts right after first paint on our benchmarks.