[webkit-reviews] review granted: [Bug 206310] Add WKContentWorld SPI, and use it in JavaScript execution : [Attachment 387864] Patch

[bugzilla-daemon at webkit.org]

Alex Christensen <achristensen at apple.com> has granted Brady Eidson
<beidson at apple.com>'s request for review:
Bug 206310: Add WKContentWorld SPI, and use it in JavaScript execution
https://bugs.webkit.org/show_bug.cgi?id=206310

Attachment 387864: Patch

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




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

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

r=me

> Source/WebKit/UIProcess/API/APIContentWorld.cpp:68
> +ContentWorld::ContentWorld(uint64_t identifier)

Could you add a FIXME here saying we should use ObjectIdentifier?

> Source/WebKit/UIProcess/API/APIUserContentWorld.h:49
> +    friend class ContentWorld;

I still don't think this is great.

> Source/WebKit/WebProcess/WebPage/WebPage.cpp:3385
> +	   send(Messages::WebPageProxy::ScriptValueCallback({ },
ExceptionDetails { "Unable to execute JavaScript: Cannot find specified content
world"_s }, callbackID));

Could you add a FIXME somewhere saying to use sendWithAsyncReply rather than
two messages with a CallbackID?