Bug 191754 – Add DidFirstMeaningfulPaint milestone.

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[alan]

This milestone fires when we finished the very first paint triggered by the first visually non-empty layout.

[alan]

Created attachment 355096 [details]
Patch

[Saam Barati]

Comment on attachment 355096 [details]
Patch

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

LGTM but I'll let someone with more domain knowledge give the r+

> Source/WebCore/ChangeLog:8
> +        This milestone fires soone after the paint, triggered by the visually non-empty layout is finished.

soone => soon

> Tools/TestWebKitAPI/Tests/WebKit/FirstMeaningfulPaintMilestone_Bundle.cpp:2
> + * Copyright (C) 2012 Apple Inc. All rights reserved.

2018

> Tools/TestWebKitAPI/Tests/WebKit/FirstMeaningfulPaintMilestone_Bundle.cpp:52
> +static InjectedBundleTest::Register<FirstMeaningfulPaintMilestoneTest> registrar("FirstMeaningfulPaintMilestoneTest");

Maybe I'm missing something but what is this testing?

[Wenson Hsieh]

Comment on attachment 355096 [details]
Patch

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

> Source/WebKit/UIProcess/API/C/WKPageRenderingProgressEvents.h:42
> +    WKPageRenderingProgressEventFirstMeaningfulPaint = 1 << 8

Hm...why legacy C API instead of modern ObjC API? (see _WKRenderingProgressEvents.h)

> Tools/TestWebKitAPI/CMakeLists.txt:234
> +        ${TESTWEBKITAPI_DIR}/Tests/WebKit/FirstMeaningfulPaintMileston_Bundle.cpp

FirstMeaningfulPaintMileston_Bundle => FirstMeaningfulPaintMilestone_Bundle

[Simon Fraser (smfr)]

Comment on attachment 355096 [details]
Patch

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

>> Source/WebCore/ChangeLog:8
>> +        This milestone fires soone after the paint, triggered by the visually non-empty layout is finished.
> 
> soone => soon

"triggered by the visually non-empty layout is finished" doesn't parse.

"triggered by the first visually non-empty layout"?

[alan]

Created attachment 355107 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 355107 [details]
Patch

Clearing flags on attachment: 355107

Committed r238306: <https://trac.webkit.org/changeset/238306>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/46140654>

[Saam Barati]

Comment on attachment 355107 [details]
Patch

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

> Source/WebKit/UIProcess/API/C/WKPageRenderingProgressEvents.h:42
> +    WKPageRenderingProgressEventFirstMeaningfulPaint = 1 << 8

Should this be << 6?

[alan]

Comment on attachment 355107 [details]
Patch

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

>> Source/WebKit/UIProcess/API/C/WKPageRenderingProgressEvents.h:42
>> +    WKPageRenderingProgressEventFirstMeaningfulPaint = 1 << 8
> 
> Should this be << 6?

No, it needs to value match the other defines.