[webkit-reviews] review denied: [Bug 211868] [PlayStation] Add minimal WKView API to enable TestWebKitAPI : [Attachment 399831] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed May 20 12:24:57 PDT 2020

Alex Christensen <achristensen at apple.com> has denied Yoshiaki Jitsukawa
<yoshiaki.jitsukawa at sony.com>'s request for review:
Bug 211868: [PlayStation] Add minimal WKView API to enable TestWebKitAPI

Attachment 399831: Patch


--- Comment #10 from Alex Christensen <achristensen at apple.com> ---
Comment on attachment 399831
  --> https://bugs.webkit.org/attachment.cgi?id=399831

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


> +    using namespace WebKit;

This seems excessive just to avoid WebKit:: on one line.

> +WK_EXPORT void WKContextConfigurationSetUserId(WKContextConfigurationRef
configuration, int32_t userId);

I'm not convinced this should be on the context.  Why not the page?  What
exactly is the user id?  why is it signed?

> Source/WebKit/UIProcess/API/C/playstation/WKPagePrivatePlayStation.h:35
> +WK_EXPORT void WKPageSetSize(WKPageRef page, WKSize size);

Let's not add this if it's not implemented.

> Source/WebKit/UIProcess/API/C/playstation/WKView.cpp:33
> +WKViewRef WKViewCreate(WKPageConfigurationRef configuration)

I'm not crazy about having "WKView" be the base of this name, but it is
WKViewRef, so that might not be the end of the world.

> Source/WebKit/UIProcess/playstation/WebView.cpp:41
> +RefPtr<WebView> WebView::create(const API::PageConfiguration& configuration)

Could we name this something other than "WebView"?  PlayStationWebView maybe? 
That way it'll get less confused with our other classes named WebView.

> Source/WebKit/UIProcess/playstation/WebView.h:41
> +class WebView : public API::ObjectImpl<API::Object::Type::View> {

Having a API::Object::Type::View and API::Object::Type::Page being separate
objects is something we want to merge into one object once we can, so this may
substantially change shape in the future.  I'm ok with adding this for now, but
be aware that this will need to change in the next few years.

More information about the webkit-reviews mailing list