[webkit-reviews] review denied: [Bug 25030] Need to refactor MessagePort to support multi-process implementations : [Attachment 29241] proposed patch

[bugzilla-daemon at webkit.org]

Alexey Proskuryakov <ap at webkit.org> has denied Andrew Wilson
<atwilson at google.com>'s request for review:
Bug 25030: Need to refactor MessagePort to support multi-process
implementations
https://bugs.webkit.org/show_bug.cgi?id=25030

Attachment 29241: proposed patch
https://bugs.webkit.org/attachment.cgi?id=29241&action=review

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
> +	   * dom/RemotePort.h: Added.
> +	   (WebCore::RemotePort::~RemotePort):

RemotePort doesn't look like a great name to me - first, it doesn't say that
it's all about MessagePorts, and then, it's not clear in which sense it is
"remote". Other similar interfaces have the word Proxy in them, maybe
MessagePortProxy would fit better?

> +    m_entangledPort->deliverRemoteMessage(message, newMessagePort);

Again, a naming nitpick. Are there any non-remote messages? This function name
makes it look like there are.

> +	   virtual void deliverRemoteMessage(const String& message,
PassRefPtr<MessagePort>) = 0;

There is no need to include PlatformString.h just for this, a forward
declaration would suffice.

These are all nitpicks, but the patch mostly does renaming, so I think it's
fair to be picky about names.

Besides renaming, there are changes to how entangling is implemented, it would
be nice to provide a rationale in the ChangeLog.