[webkit-reviews] review granted: [Bug 225615] Promote `-[WKWebView _pageExtendedBackgroundColor]` SPI to `-[WKWebView underPageBackgroundColor]` API : [Attachment 428209] Patch

[bugzilla-daemon at webkit.org]

Wenson Hsieh <wenson_hsieh at apple.com> has granted Devin Rousso
<drousso at apple.com>'s request for review:
Bug 225615: Promote `-[WKWebView _pageExtendedBackgroundColor]` SPI to
`-[WKWebView underPageBackgroundColor]` API
https://bugs.webkit.org/show_bug.cgi?id=225615

Attachment 428209: Patch

https://bugs.webkit.org/attachment.cgi?id=428209&action=review




--- Comment #3 from Wenson Hsieh <wenson_hsieh at apple.com> ---
Comment on attachment 428209
  --> https://bugs.webkit.org/attachment.cgi?id=428209
Patch

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

> Source/WebCore/page/Page.cpp:2577
> +    if (auto* frameView = mainFrame().view()) {
> +	   if (auto* renderView = frameView->renderView()) {

Nit - `if (auto … = makeRefPtr(…))`

> Source/WebKit/UIProcess/API/ios/WKWebViewIOS.mm:493
> +    return webView.underPageBackgroundColor.CGColor;

We seem to have lost the null check for `webView->_page` here, since
`-underPageBackgroundColor` dereferences `_page`.

> Source/WebKit/UIProcess/WebPageProxy.cpp:8622
> +#if !PLATFORM(MAC) && !PLATFORM(IOS_FAMILY)

Nit - !PLATFORM(COCOA)?

> Source/WebKit/UIProcess/WebPageProxy.h:2661
> +    bool m_pendingUnderPageBackgroundColorOverrideToDispatch { false };

Nit - m_hasPendingUnderPageBackgroundColorOverrideToDispatch? (The current name
makes it sound like it should be a `Color` or `Optional<Color>`)