Bug 26903 - Need to enable CHANNEL_MESSAGING by default
Summary: Need to enable CHANNEL_MESSAGING by default
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit API (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P2 Normal
Assignee: Andrew Wilson
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-07-01 13:48 PDT by Andrew Wilson
Modified: 2009-07-09 20:21 PDT (History)
2 users (show)

See Also:


Attachments
Proposed patch (74.88 KB, patch)
2009-07-01 15:46 PDT, Andrew Wilson
no flags Details | Formatted Diff | Diff
Patch addressing ap's comments (91.22 KB, patch)
2009-07-08 11:08 PDT, Andrew Wilson
ap: review+
Details | Formatted Diff | Diff
Patch with enabled security test and removing merge conflict. (95.43 KB, patch)
2009-07-09 10:43 PDT, Andrew Wilson
ap: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Wilson 2009-07-01 13:48:19 PDT
Now that the spec has stabilized and we've resolved the cross-thread GC issues, we can turn on CHANNEL_MESSAGING by default.

There's one open API issue (https://bugs.webkit.org/show_bug.cgi?id=26902), but I'm not certain whether this is something we want to support (posting an array of ports to postMessage), and whether that should block enabling CHANNEL_MESSAGING.

ap, your thoughts?
Comment 1 Andrew Wilson 2009-07-01 15:46:04 PDT
Created attachment 32152 [details]
Proposed patch

This patch is actually much smaller than it appears - I've re-enabled a bunch of the previously disabled tests (I didn't modify them, I just renamed them) which makes it look large.
Comment 2 Alexey Proskuryakov 2009-07-08 07:40:41 PDT
Comment on attachment 32152 [details]
Proposed patch

This enables channel messaging for Xcode and for autoconf builds - are any changes to Windows Visual Studio project files necessary?

It would be nice to add a few words about why we're turning CHANNEL_MESSAGING on (e.g. "because the MessageChannel API can now be implemented for Web Workers, and is reasonably stable thus").
Comment 3 Andrew Wilson 2009-07-08 11:08:58 PDT
Created attachment 32461 [details]
Patch addressing ap's comments

Good catch on the visual studio stuff - I've done a search to make sure I haven't missed any other places, and also updated the ChangeLogs to have a less terse change description.
Comment 4 Alexey Proskuryakov 2009-07-08 11:29:59 PDT
Comment on attachment 32461 [details]
Patch addressing ap's comments

> +<<<<<<< HEAD:LayoutTests/ChangeLog

> +=======

Please fix conflict markers before landing.

Does the Visual Studio project actually build? I'm not sure if it has all the necessary source files in it.

Oh, and please also enable LayoutTests/http/tests/security/MessagePort/event-listener-context.html-disabled.

I'm going to say r=me now, but please address these comments.
Comment 5 Andrew Wilson 2009-07-09 10:43:31 PDT
Created attachment 32523 [details]
Patch with enabled security test and removing merge conflict.

Sigh, fixed merge errors - tragically git didn't notice that I'd committed a file with a merge conflict.

I enabled that test - as I mentioned on IRC, you can no longer use a MessageChannel constructor from a closed frame, so now I simplified the test to just check for a thrown exception.

I was able to build on Windows just fine - what files were you concerned were missing from the vcproj file?
Comment 6 Alexey Proskuryakov 2009-07-09 14:20:09 PDT
Comment on attachment 32523 [details]
Patch with enabled security test and removing merge conflict.

r=me
Comment 7 Dmitry Titov 2009-07-09 20:21:52 PDT
Landed: http://trac.webkit.org/changeset/45695