Summary: | Move MouseEvent-dispatching logic into MouseEventDispatchMediator. | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Dimitri Glazkov (Google) <dglazkov> | ||||||
Component: | New Bugs | Assignee: | Dimitri Glazkov (Google) <dglazkov> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | buildbot, darin, jer.noble, webkit-ews, webkit.review.bot | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Other | ||||||||
OS: | OS X 10.5 | ||||||||
Bug Depends on: | 57562 | ||||||||
Bug Blocks: | 55515, 57642 | ||||||||
Attachments: |
|
Description
Dimitri Glazkov (Google)
2011-04-01 09:54:00 PDT
Created attachment 87874 [details]
Patch
Created attachment 88158 [details]
Rebased to ToT.
This patch doesn't build on Mac because of r82833 (http://trac.webkit.org/changeset/82733/trunk/Source/WebKit2/WebProcess/WebPage/WebPage.cpp). Turns out Jer liked my newly added constructor so much, he used it right away -- and now I can't remove it! :) Jer, is there a way you could use one of the old constructors instead? Or should I keep it? WDYT, Darin? Attachment 88158 [details] did not build on qt: Build output: http://queues.webkit.org/results/8334111 Comment on attachment 88158 [details] Rebased to ToT. View in context: https://bugs.webkit.org/attachment.cgi?id=88158&action=review > Source/WebCore/dom/Event.h:207 > + EventDispatchMediator(); > + > + void setEvent(PassRefPtr<Event>); > Event* event() const; I’m surprised none of these are inlined in the header > Source/WebCore/dom/EventDispatcher.cpp:67 > +Node* EventDispatcher::node() const > +{ > + return m_node.get(); > +} Seems like an inlining candidate. I’m getting a little paranoid about slicing and dicing the event code into pieces that are too small. (In reply to comment #6) > I’m getting a little paranoid about slicing and dicing the event code into pieces that are too small. I think it'll look good in the end. No worries, cap'n! Everything's peachy! :) (In reply to comment #5) > (From update of attachment 88158 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=88158&action=review > > > Source/WebCore/dom/Event.h:207 > > + EventDispatchMediator(); > > + > > + void setEvent(PassRefPtr<Event>); > > Event* event() const; > > I’m surprised none of these are inlined in the header > > > Source/WebCore/dom/EventDispatcher.cpp:67 > > +Node* EventDispatcher::node() const > > +{ > > + return m_node.get(); > > +} > > Seems like an inlining candidate. Will do. Attachment 88158 [details] did not build on win: Build output: http://queues.webkit.org/results/8330194 Attachment 88158 [details] did not build on win: Build output: http://queues.webkit.org/results/8338007 Attachment 88158 [details] did not build on mac: Build output: http://queues.webkit.org/results/8331332 (In reply to comment #3) > This patch doesn't build on Mac because of r82833 (http://trac.webkit.org/changeset/82733/trunk/Source/WebKit2/WebProcess/WebPage/WebPage.cpp). Turns out Jer liked my newly added constructor so much, he used it right away -- and now I can't remove it! :) > > Jer, is there a way you could use one of the old constructors instead? Or should I keep it? WDYT, Darin? Jer and I discussed this just now and I am going to leave his shiny constructor alone. This will also make the patch simpler. Committed r82948: <http://trac.webkit.org/changeset/82948> Comment on attachment 88158 [details]
Rebased to ToT.
removing flags on a landed patch.
|