Bug 217400

Summary: Adjust heuristic for checking whether view reaches visually non-empty state
Product: WebKit Reporter: Sihui Liu <sihui_liu>
Component: New BugsAssignee: Sihui Liu <sihui_liu>
Status: RESOLVED FIXED    
Severity: Normal CC: darin, ggaren, koivisto, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
ews-feeder: commit-queue-
Patch
none
Patch for landing
none
Patch for landing none

Sihui Liu
Reported 2020-10-06 12:08:21 PDT
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.
Attachments
Patch (2.44 KB, patch)
2020-10-06 12:13 PDT, Sihui Liu 		no flags
DetailsFormatted DiffDiff
Patch (2.80 KB, patch)
2020-10-07 10:00 PDT, Sihui Liu 		no flags
DetailsFormatted DiffDiff
Patch (10.45 KB, patch)
2020-10-07 14:37 PDT, Sihui Liu 		ews-feeder: commit-queue-
DetailsFormatted DiffDiff
Patch (10.97 KB, patch)
2020-10-07 14:54 PDT, Sihui Liu 		no flags
DetailsFormatted DiffDiff
Patch for landing (11.09 KB, patch)
2020-10-08 09:06 PDT, Sihui Liu 		no flags
DetailsFormatted DiffDiff
Patch for landing (10.22 KB, patch)
2020-10-29 11:44 PDT, Sihui Liu 		no flags
DetailsFormatted DiffDiff
Show Obsolete (5) View All Add attachment proposed patch, testcase, etc.
Sihui Liu
Comment 1 2020-10-06 12:10:09 PDT
rdar://problem/69894849
Sihui Liu
Comment 2 2020-10-06 12:13:17 PDT
Created attachment 410677 [details] Patch
Darin Adler
Comment 3 2020-10-07 09:13:34 PDT
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
Comment 4 2020-10-07 10:00:20 PDT
Created attachment 410754 [details] Patch
Geoffrey Garen
Comment 5 2020-10-07 12:37:58 PDT
Comment on attachment 410754 [details] Patch r=me Please double-check the iOS-wk2 EWS failure.
alan
Comment 6 2020-10-07 12:42:01 PDT
I hope we eventually send out the non-empty milestone even when the script does not generate any content.
Geoffrey Garen
Comment 7 2020-10-07 12:46:50 PDT
(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
Comment 8 2020-10-07 12:50:22 PDT
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
Comment 9 2020-10-07 14:37:13 PDT
Created attachment 410781 [details] Patch
Sihui Liu
Comment 10 2020-10-07 14:54:30 PDT
Created attachment 410786 [details] Patch
EWS
Comment 11 2020-10-07 17:53:21 PDT
ChangeLog entry in Tools/ChangeLog contains OOPS!.
Sihui Liu
Comment 12 2020-10-08 09:06:41 PDT
Created attachment 410849 [details] Patch for landing
EWS
Comment 13 2020-10-08 10:59:54 PDT
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
Comment 14 2020-10-29 11:44:21 PDT
Reopening to attach new patch.
Sihui Liu
Comment 15 2020-10-29 11:44:22 PDT
Created attachment 412670 [details] Patch for landing
EWS
Comment 16 2020-10-29 12:15:54 PDT
Committed r269160: <https://trac.webkit.org/changeset/269160> All reviewed patches have been landed. Closing bug and clearing flags on attachment 412670 [details].
Note You need to log in before you can comment on or make changes to this bug.