[webkit-reviews] review granted: [Bug 46534] Fill in more of PluginProcess : [Attachment 68776] Patch

[bugzilla-daemon at webkit.org]

Adam Roben (aroben) <aroben at apple.com> has granted Anders Carlsson
<andersca at apple.com>'s request for review:
Bug 46534: Fill in more of PluginProcess
https://bugs.webkit.org/show_bug.cgi?id=46534

Attachment 68776: Patch
https://bugs.webkit.org/attachment.cgi?id=68776&action=review

------- Additional Comments from Adam Roben (aroben) <aroben at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=68776&action=review

> WebKit2/PluginProcess/PluginProcess.cpp:37
> +static const double ShutdownTimeout = 15.0;

We don't normally capitalize variable names. Or add ".0".

> WebKit2/PluginProcess/PluginProcess.cpp:114
> +void PluginProcess::createWebProcessConnection()
> +{
> +    // FIXME: This is platform specific!
> +
> +    // Create the listening port.
> +    mach_port_t listeningPort;
> +    mach_port_allocate(mach_task_self(), MACH_PORT_RIGHT_RECEIVE,
&listeningPort);
> +
> +    // Create a listening connection.
> +    RefPtr<WebProcessConnection> connection =
WebProcessConnection::create(listeningPort);
> +    m_webProcessConnections.append(connection.release());
> +
> +    CoreIPC::MachPort clientPort(listeningPort, MACH_MSG_TYPE_MAKE_SEND);
> +   
m_connection->send(Messages::PluginProcessProxy::DidCreateWebProcessConnection(
clientPort), 0);
> +
> +    // Stop the shutdown timer.
> +    m_shutdownTimer.stop();
> +}

Why not create PluginProcessMac.mm and put the platform-specific code there?
That will make it easier for other platforms to adopt.

No need for the local connection or clientPort variables.

> WebKit2/Scripts/webkit2/messages.py:288
>  def header_for_type(type):
>      special_cases = {
> +	   'CoreIPC::MachPort': '"MachPort.h"',
>	   'WTF::String': '<wtf/text/WTFString.h>',
>	   'WebKit::WebKeyboardEvent': '"WebEvent.h"',
>	   'WebKit::WebMouseEvent': '"WebEvent.h"',

Test please!