Add Websocket addEventListener and removeEventListner javascript bindings.
Created attachment 40292 [details] JavaScript bindings of WebSocket addEventListener/removeEventListener
Comment on attachment 40292 [details] JavaScript bindings of WebSocket addEventListener/removeEventListener This seems like pretty generic code. I'm surprised we don't have a way to share this with the other addEventListener implementations. Why does this one need customs bindings? Should we be extending the bindings generators instead?
(In reply to comment #2) > (From update of attachment 40292 [details]) > This seems like pretty generic code. I'm surprised we don't have a way to > share this with the other addEventListener implementations. Why does this one > need customs bindings? Should we be extending the bindings generators instead? Sounds good. May I introduce new extended attribute, like AddEventListener and RemoveEventListener, for addEventListener and removeEventListener respectively? Or should we generate the bindings code by method names?
(In reply to comment #3) > (In reply to comment #2) > > (From update of attachment 40292 [details] [details]) > > This seems like pretty generic code. I'm surprised we don't have a way to > > share this with the other addEventListener implementations. Why does this one > > need customs bindings? Should we be extending the bindings generators instead? > > Sounds good. May I introduce new extended attribute, like AddEventListener and > RemoveEventListener, for addEventListener and removeEventListener respectively? > Or should we generate the bindings code by method names? Or introduce new extended attribute, GenerateEventTarget, for interface?
Might want to ask weinig on #webkit as he's more up-to-date on the bindings generator code than I.
Created attachment 40613 [details] update for r48978
Created attachment 40614 [details] remove V8ObjectEventListener.h
Comment on attachment 40614 [details] remove V8ObjectEventListener.h I'm confused by the purpose of this last patch. All changes require ChangeLog entries, so r-.
Created attachment 40691 [details] JavaScript bindings of WebSocket addEventListener/removeEventListener
(In reply to comment #5) > Might want to ask weinig on #webkit as he's more up-to-date on the bindings > generator code than I. I catched weinig on @webkit and he said just copy the xhr code for now and he will clean that up in the future.
Comment on attachment 40691 [details] JavaScript bindings of WebSocket addEventListener/removeEventListener > - // [Custom] void addEventListener(in DOMString type, > - // in EventListener listener, > - // in boolean useCapture); > - // [Custom] void removeEventListener(in DOMString type, > - // in EventListener listener, > - // in boolean useCapture); > - // boolean dispatchEvent(in Event evt) > - // raises(EventException); > + [Custom] void addEventListener(in DOMString type, > + in EventListener listener, > + in boolean useCapture); > + [Custom] void removeEventListener(in DOMString type, > + in EventListener listener, > + in boolean useCapture); > + boolean dispatchEvent(in Event evt) > + raises(EventException); The indentation here is quite odd. This needs tests, so r-.
Created attachment 40700 [details] WebCore: JavaScript bindings of WebSocket addEventListener/removeEventListener
Comment on attachment 40700 [details] WebCore: JavaScript bindings of WebSocket addEventListener/removeEventListener Rejecting patch 40700 from commit-queue. Patch https://bugs.webkit.org/attachment.cgi?id=40700 from bug 29841 failed to download and apply.
Manually landed as r49488.
There were two problems with this landing: 1. It broke all the bots. 2. The commit message was missing the reviewer. bugzilla-tool land-diff can help with both, by building and running all the tests, and updating the reviewer correctly. (There are obviously ways to do the landing manually as well.)
(In reply to comment #15) > There were two problems with this landing: > > 1. It broke all the bots. > 2. The commit message was missing the reviewer. > > bugzilla-tool land-diff > can help with both, by building and running all the tests, and updating the > reviewer correctly. > (There are obviously ways to do the landing manually as well.) Sorry for that. I'll fix broken layout tests soon. I will use bugzilla-tool land-diff to commit the change in future. Thanks for the info.
Going back through logs from last week: The commit-queue rejected it due to the patch to DumpRenderTree.mm not applying.