Bug 217400 – Adjust heuristic for checking whether view reaches visually non-empty state

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Sihui Liu]

We decide that view reaches visually non-empty state and fire corresponding events when main document is parsed and no more content (font or css resources) to be loaded. 
An edge case is when the page has an empty document and depends on async scripts to fill content (like wpt.fyi), then we may fire the events too early.

[Sihui Liu]

rdar://problem/69894849

[Sihui Liu]

Created attachment 410677 [details]
Patch

[Darin Adler]

Comment on attachment 410677 [details]
Patch

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

> Source/WebCore/page/FrameView.cpp:4580
> +            bool hasScriptResource = false;

Should be named isLoadingScript or something more like that. This definitely does not answer the question, “Does this have a script resource?”

> Source/WebCore/page/FrameView.cpp:4584
> +                auto resourceType = resource.value->type();

In this narrow context just “type” would be a good variable name.

> Source/WebCore/page/FrameView.cpp:4587
> +                if (!hasScriptResource && resourceType == CachedResource::Type::Script)

Don’t need to check !hasScriptResource here.

[Sihui Liu]

Created attachment 410754 [details]
Patch

[Geoffrey Garen]

Comment on attachment 410754 [details]
Patch

r=me

Please double-check the iOS-wk2 EWS failure.

[zalan]

I hope we eventually send out the non-empty milestone even when the script does not generate any content.

[Geoffrey Garen]

(In reply to zalan from comment #6)
> I hope we eventually send out the non-empty milestone even when the script
> does not generate any content.

Good point. I think you can add an API test for this by using an empty document with a <script src="..."> that points to an empty script, and then verifying that the visually non-empty milestone fires. Seems worth doing (but probably doesn't need to block landing the patch).

[Geoffrey Garen]

Now that I think about it, the test should also verify this new behavior: that the mouth stone does not fire before the script has loaded. You can detect when the script loads by using the api testing url scheme. Or by having the script signal the api test somehow.

[Sihui Liu]

Created attachment 410781 [details]
Patch

[Sihui Liu]

Created attachment 410786 [details]
Patch

[EWS]

ChangeLog entry in Tools/ChangeLog contains OOPS!.

[Sihui Liu]

Created attachment 410849 [details]
Patch for landing

[EWS]

Committed r268192: <https://trac.webkit.org/changeset/268192>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 410849 [details].

[Sihui Liu]

Reopening to attach new patch.

[Sihui Liu]

Created attachment 412670 [details]
Patch for landing

[EWS]

Committed r269160: <https://trac.webkit.org/changeset/269160>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 412670 [details].