[webkit-reviews] review granted: [Bug 233719] [WK2] Make Web Lock API work across multiple WebProcesses : [Attachment 445670] Patch

[bugzilla-daemon at webkit.org]

Alex Christensen <achristensen at apple.com> has granted Chris Dumez
<cdumez at apple.com>'s request for review:
Bug 233719: [WK2] Make Web Lock API work across multiple WebProcesses
https://bugs.webkit.org/show_bug.cgi?id=233719

Attachment 445670: Patch

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




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

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

Would it be worth adding a test using _relatedWebView to verify what happens
when you have two pages in the same process?

> Source/WebCore/Modules/web-locks/WebLockRegistry.h:76
> +    HashMap<ClientOrigin, PerOriginRegistry*> m_perOriginRegistries;

Can this be a weak pointer?

> Source/WebKit/WebProcess/WebCoreSupport/RemoteWebLockRegistry.messages.in:28
> +    DidCompleteLockRequest(WebCore::WebLockIdentifier lockIdentifier,
WebCore::ScriptExecutionContextIdentifier clientID, bool success);
> +    DidStealLock(WebCore::WebLockIdentifier lockIdentifier,
WebCore::ScriptExecutionContextIdentifier clientID);

It seems like this can be a completion handler on the RequestLock message that
just contains whether it was success, failure, or stolen.  That would be nicer
than introducing a new messages.in just for replies.