Bug 85764 - [Chromium] Move createMessagePortChannel to Platform.h
Summary: [Chromium] Move createMessagePortChannel to Platform.h
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Pilgrim (Google)
URL:
Keywords:
Depends on: 86559
Blocks: 82948
  Show dependency treegraph
 
Reported: 2012-05-06 19:36 PDT by Mark Pilgrim (Google)
Modified: 2012-06-06 19:41 PDT (History)
7 users (show)

See Also:


Attachments
Patch (4.98 KB, patch)
2012-05-06 19:36 PDT, Mark Pilgrim (Google)
no flags Details | Formatted Diff | Diff
Patch (16.12 KB, patch)
2012-05-15 14:59 PDT, Mark Pilgrim (Google)
no flags Details | Formatted Diff | Diff
Patch (20.35 KB, patch)
2012-06-06 06:57 PDT, Mark Pilgrim (Google)
no flags Details | Formatted Diff | Diff
Patch (18.63 KB, patch)
2012-06-06 12:02 PDT, Mark Pilgrim (Google)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Pilgrim (Google) 2012-05-06 19:36:09 PDT
[Chromium] Move createMessagePortChannel to Platform.h
Comment 1 Mark Pilgrim (Google) 2012-05-06 19:36:45 PDT
Created attachment 140456 [details]
Patch
Comment 2 WebKit Review Bot 2012-05-06 19:39:50 PDT
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.
Comment 3 Adam Barth 2012-05-06 20:38:24 PDT
Comment on attachment 140456 [details]
Patch

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

> Source/WebKit/chromium/public/platform/WebKitPlatformSupport.h:-60
> -class WebMessagePortChannel; // FIXME: Does this belong in platform?

We'll also want to move WebMessagePortChannel and WebMessagePortChannelClient int Source/Platform/chromium/public as well.  They look like they'll move cleanly, so the answer to this comment seems to be "yes".

> Source/WebKit/chromium/src/PlatformMessagePortChannel.cpp:46
>  namespace WebCore {

Once you move WebMessagePortChannel into Source/Platform/chromium/public, you can move all this code into WebCore/platform where it belongs.  :)
Comment 4 Mark Pilgrim (Google) 2012-05-15 14:59:57 PDT
Created attachment 142067 [details]
Patch
Comment 5 Mark Pilgrim (Google) 2012-05-15 15:00:31 PDT
Comment on attachment 142067 [details]
Patch

Now moves WebMessagePortChannel and WebMessagePortChannelClient.
Comment 6 Adam Barth 2012-05-15 15:07:03 PDT
Comment on attachment 142067 [details]
Patch

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

> Source/WebKit/chromium/src/PlatformMessagePortChannel.cpp:46
>  namespace WebCore {

This patch is great.  Thanks!

A good followup patch might be to try to move this code in the WebCore namespace into Source/WebCore/platform/chromium.
Comment 7 WebKit Review Bot 2012-05-15 19:29:25 PDT
Comment on attachment 142067 [details]
Patch

Clearing flags on attachment: 142067

Committed r117204: <http://trac.webkit.org/changeset/117204>
Comment 8 WebKit Review Bot 2012-05-15 19:29:29 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 WebKit Review Bot 2012-05-15 20:12:37 PDT
Re-opened since this is blocked by 86559
Comment 10 Mark Pilgrim (Google) 2012-06-06 06:57:40 PDT
Created attachment 146012 [details]
Patch
Comment 11 Adam Barth 2012-06-06 09:52:44 PDT
Comment on attachment 146012 [details]
Patch

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

> Source/WebKit/chromium/public/WebFrame.h:42
> +#include <public/WebMessagePortChannel.h>

The #includes in Source/WebKit/chromium/public should use forwarding headers, like WebURL.h above.

> Source/WebKit/chromium/src/PlatformMessagePortChannel.cpp:123
>  PlatformMessagePortChannel::PlatformMessagePortChannel()

This whole class should probably move into WebCore.
Comment 12 Adam Barth 2012-06-06 09:53:03 PDT
(Feel free to move PlatformMessagePortChannel in a followup patch.)
Comment 13 Mark Pilgrim (Google) 2012-06-06 12:02:28 PDT
Created attachment 146080 [details]
Patch
Comment 14 Mark Pilgrim (Google) 2012-06-06 12:03:16 PDT
Comment on attachment 146080 [details]
Patch

reverted headers in chromium/public/
Comment 15 WebKit Review Bot 2012-06-06 19:40:58 PDT
Comment on attachment 146080 [details]
Patch

Clearing flags on attachment: 146080

Committed r119666: <http://trac.webkit.org/changeset/119666>
Comment 16 WebKit Review Bot 2012-06-06 19:41:04 PDT
All reviewed patches have been landed.  Closing bug.