Bug 46534 - Fill in more of PluginProcess
Summary: Fill in more of PluginProcess
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Anders Carlsson
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-09-24 15:42 PDT by Anders Carlsson
Modified: 2010-09-24 17:22 PDT (History)
2 users (show)

See Also:


Attachments
Patch (18.21 KB, patch)
2010-09-24 15:46 PDT, Anders Carlsson
aroben: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Anders Carlsson 2010-09-24 15:42:51 PDT
Fill in more of PluginProcess
Comment 1 Anders Carlsson 2010-09-24 15:46:30 PDT
Created attachment 68776 [details]
Patch
Comment 2 WebKit Review Bot 2010-09-24 15:50:21 PDT
Attachment 68776 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebKit2/UIProcess/Plugins/PluginProcessProxy.h:38:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 1 in 12 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Adam Roben (:aroben) 2010-09-24 16:02:03 PDT
Comment on attachment 68776 [details]
Patch

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!
Comment 4 Anders Carlsson 2010-09-24 16:13:16 PDT
Committed r68309: <http://trac.webkit.org/changeset/68309>
Comment 5 Ryosuke Niwa 2010-09-24 17:00:22 PDT
I believe you forgot to add PluginProcessProxyMessageReceiver.cpp or that the change to WebKit2/WebKit2.xcodeproj/project.pbxproj wasn't intentional.  WebKit2 fails to compile on my local check (r68314).  I'm waiting for the bots to catch up in order to confirm this.
Comment 6 Anders Carlsson 2010-09-24 17:22:58 PDT
(In reply to comment #5)
> I believe you forgot to add PluginProcessProxyMessageReceiver.cpp or that the change to WebKit2/WebKit2.xcodeproj/project.pbxproj wasn't intentional.  WebKit2 fails to compile on my local check (r68314).  I'm waiting for the bots to catch up in order to confirm this.

PluginProcessProxyMessageReceiver.cpp is autogenerated by DerivedSources.make from PluginProcessProxy.messages.in, all of which are checked in.