Bug 26903

Summary: Need to enable CHANNEL_MESSAGING by default
Product: WebKit Reporter: Andrew Wilson <atwilson>
Component: WebKit APIAssignee: Andrew Wilson <atwilson>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, dimich
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.5   
Attachments:
Description Flags
Proposed patch
none
Patch addressing ap's comments
ap: review+
Patch with enabled security test and removing merge conflict. ap: review+

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