Bug 74160 - Use [Supplemental] IDL in WebSocket
Summary: Use [Supplemental] IDL in WebSocket
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kentaro Hara
URL:
Keywords:
Depends on:
Blocks: 72138
  Show dependency treegraph
 
Reported: 2011-12-08 20:20 PST by Kentaro Hara
Modified: 2011-12-22 02:33 PST (History)
5 users (show)

See Also:


Attachments
Patch (5.49 KB, patch)
2011-12-08 21:12 PST, Kentaro Hara
no flags Details | Formatted Diff | Diff
rebased patch for commit (5.56 KB, patch)
2011-12-11 20:48 PST, Kentaro Hara
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kentaro Hara 2011-12-08 20:20:35 PST
By using the [Supplemental] IDL, we can move declarations of WebSocket attributes from DOMWindow.idl to websocket/DOMWindowWebSocket.idl, which helps make WebSocket a self-contained module.

See bug 72138 for more details of our motivation.
Comment 1 Kentaro Hara 2011-12-08 21:12:48 PST
Created attachment 118526 [details]
Patch
Comment 2 Adam Barth 2011-12-08 21:35:41 PST
Comment on attachment 118526 [details]
Patch

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

> Source/WebCore/websockets/DOMWindowWebSocket.idl:37
> +        attribute CloseEventConstructor CloseEvent;

Is CloseEvent specific to WebSockets?  I guess it is!
Comment 3 Kentaro Hara 2011-12-08 21:37:09 PST
(In reply to comment #2)
> (From update of attachment 118526 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=118526&action=review
> 
> > Source/WebCore/websockets/DOMWindowWebSocket.idl:37
> > +        attribute CloseEventConstructor CloseEvent;
> 
> Is CloseEvent specific to WebSockets?  I guess it is!

Yes, it is!
http://dev.w3.org/html5/websockets/#closeevent
Comment 4 Adam Barth 2011-12-08 21:38:52 PST
We should do more build system integration.  I can help.  I'll try to do the Mac build tomorrow.
Comment 5 Kentaro Hara 2011-12-08 21:44:39 PST
(In reply to comment #4)
> We should do more build system integration.  I can help.  I'll try to do the Mac build tomorrow.

What do you mean by "more build system integration"?

After this patch is landed, I am planning to change build flows of all platforms as follows:

[1] Make a change on DerivedSources.make. (for Mac)
[2] Make a change on GNUmakefile.am and bindings/gobject/GNUmakefile.am. (for GTK)
[3] Make a change on DerivedSources.pri. (for Qt)
[4] Make a change on WebCore.vcproj/MigrateScripts and WebCore.vcproj/WebCore.vcproj. (for Win)
[5] Make a change on UseJSC.cmake, UseV8.cmake and PlatformBlackBerry.cmake. (for Efl, WinCE and BlackBerry)

Since ENABLE_WEBSOCKET is enabled on all platforms, I hope that now we can confirm if our build flow change is correct or not.
Comment 6 Adam Barth 2011-12-08 23:00:03 PST
Sounds great.
Comment 7 WebKit Review Bot 2011-12-09 15:32:45 PST
Comment on attachment 118526 [details]
Patch

Attachment 118526 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/10833373
Comment 8 Kentaro Hara 2011-12-11 20:48:37 PST
Created attachment 118726 [details]
rebased patch for commit
Comment 9 WebKit Review Bot 2011-12-11 23:01:45 PST
Comment on attachment 118726 [details]
rebased patch for commit

Clearing flags on attachment: 118726

Committed r102558: <http://trac.webkit.org/changeset/102558>
Comment 10 Kentaro Hara 2011-12-12 05:39:25 PST
Reverted r102558 for reason:

clobber build failure

Committed r102571: <http://trac.webkit.org/changeset/102571>
Comment 11 WebKit Review Bot 2011-12-14 06:13:48 PST
Comment on attachment 118726 [details]
rebased patch for commit

Clearing flags on attachment: 118726

Committed r102774: <http://trac.webkit.org/changeset/102774>