[webkit-reviews] review granted: [Bug 196744] RemoteObjectRegistry message receiver should be removed when WebPage::close is called instead of waiting until dealloc : [Attachment 367095] Patch

[bugzilla-daemon at webkit.org]

Chris Dumez <cdumez at apple.com> has granted Alex Christensen
<achristensen at apple.com>'s request for review:
Bug 196744: RemoteObjectRegistry message receiver should be removed when
WebPage::close is called instead of waiting until dealloc
https://bugs.webkit.org/show_bug.cgi?id=196744

Attachment 367095: Patch

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




--- Comment #6 from Chris Dumez <cdumez at apple.com> ---
Comment on attachment 367095
  --> https://bugs.webkit.org/attachment.cgi?id=367095
Patch

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

> Source/WebKit/Shared/API/Cocoa/RemoteObjectRegistry.mm:67
> +    if (m_shouldUnregisterMessageHandler) {

m_shouldUnregisterMessageHandler is a bit of a confusing name. It is not so
much about saying we should unregister, it is about knowing if we're
registered.
I think we should change the name to something like:
m_isRegisteredAsMessageReceiver.

Alternatively, we could drop m_shouldUnregisterMessageHandler /
m_messageReceiverID and replace them by a m_unregisterMessageReceiver
WTF::Function data member.

> Source/WebKit/Shared/API/Cocoa/_WKRemoteObjectRegistry.mm:112
> +    page.setRemoteObjectRegistry(*_remoteObjectRegistry);

Why don't we do this in the RemoteObjectRegistry constructor instead? Seems
safer.