[webkit-reviews] review canceled: [Bug 95397] Need to merge didFirstVisuallyNonEmptyLayout and didNewFirstVisuallyNonEmptyLayout : [Attachment 162159] Patch with different design

[bugzilla-daemon at webkit.org]

Sam Weinig <sam at webkit.org> has canceled Beth Dakin <bdakin at apple.com>'s
request for review:
Bug 95397: Need to merge didFirstVisuallyNonEmptyLayout and
didNewFirstVisuallyNonEmptyLayout
https://bugs.webkit.org/show_bug.cgi?id=95397

Attachment 162159: Patch with different design
https://bugs.webkit.org/attachment.cgi?id=162159&action=review

------- Additional Comments from Sam Weinig <sam at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=162159&action=review


Lovin' it!

> Source/WebCore/page/LayoutMilestoneOptions.h:37
> +typedef unsigned LayoutMilestoneOptions;

I am not sure the word Options adds anything here.

> Source/WebKit2/UIProcess/API/C/WKPage.h:122
>      WKPageDidNewFirstVisuallyNonEmptyLayoutCallback			  
didNewFirstVisuallyNonEmptyLayout;
> +    WKPageDidLayoutCallback						  
didLayout;

This new function needs to be added at the bottom of the struct.

>
Source/WebKit2/WebProcess/InjectedBundle/InjectedBundlePageLoaderClient.cpp:259

> +void InjectedBundlePageLoaderClient::didLayout(WebPage* page,
LayoutMilestoneOptions options, WTF::RefPtr<APIObject>& userData)

The WTF:: should not be necessary here.

> Source/WebKit2/WebProcess/InjectedBundle/InjectedBundlePageLoaderClient.h:86
> +    void didLayout(WebPage*, WebCore::LayoutMilestoneOptions,
WTF::RefPtr<APIObject>& userData);

The WTF:: should not be needed here.

> Source/WebKit2/WebProcess/InjectedBundle/API/c/WKBundlePage.h:143
> +    WKBundlePageDidLayoutCallback					      
didLayout;

Function needs to be at the bottom of the struct.

> Source/WebKit2/WebProcess/InjectedBundle/API/c/WKBundlePagePrivate.h:-62
> -// FIXME: This is temporary. Ultimately WebKit should choose the threshold
itself.
> -WK_EXPORT void WKBundlePageSetPaintedObjectsCounterThreshold(WKBundlePageRef
page, uint64_t threshold);

Were sure this has no callers?