Bug 57639

Summary: Move MouseEvent-dispatching logic into MouseEventDispatchMediator.
Product: WebKit Reporter: Dimitri Glazkov (Google) <dglazkov>
Component: New BugsAssignee: 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 Flags
Patch
none
Rebased to ToT. none

Dimitri Glazkov (Google)
Reported 2011-04-01 09:54:00 PDT
Move MouseEvent-dispatching logic into MouseEventDispatchMediator.
Attachments
Patch (14.06 KB, patch)
2011-04-01 09:57 PDT, Dimitri Glazkov (Google) 		no flags
DetailsFormatted DiffDiff
Rebased to ToT. (14.13 KB, patch)
2011-04-04 16:44 PDT, Dimitri Glazkov (Google) 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Dimitri Glazkov (Google)
Comment 1 2011-04-01 09:57:23 PDT
Created attachment 87874 [details] Patch
Dimitri Glazkov (Google)
Comment 2 2011-04-04 16:44:29 PDT
Created attachment 88158 [details] Rebased to ToT.
Dimitri Glazkov (Google)
Comment 3 2011-04-04 16:46:58 PDT
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?
Early Warning System Bot
Comment 4 2011-04-04 16:55:22 PDT
Attachment 88158 [details] did not build on qt: Build output: http://queues.webkit.org/results/8334111
Darin Adler
Comment 5 2011-04-04 16:55:42 PDT
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.
Darin Adler
Comment 6 2011-04-04 16:56:20 PDT
I’m getting a little paranoid about slicing and dicing the event code into pieces that are too small.
Dimitri Glazkov (Google)
Comment 7 2011-04-04 19:03:29 PDT
(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! :)
Dimitri Glazkov (Google)
Comment 8 2011-04-04 19:16:18 PDT
(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.
Build Bot
Comment 9 2011-04-04 19:20:00 PDT
Attachment 88158 [details] did not build on win: Build output: http://queues.webkit.org/results/8330194
Build Bot
Comment 10 2011-04-04 20:29:41 PDT
Attachment 88158 [details] did not build on win: Build output: http://queues.webkit.org/results/8338007
WebKit Review Bot
Comment 11 2011-04-05 05:21:19 PDT
Attachment 88158 [details] did not build on mac: Build output: http://queues.webkit.org/results/8331332
Dimitri Glazkov (Google)
Comment 12 2011-04-05 09:26:26 PDT
(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.
Dimitri Glazkov (Google)
Comment 13 2011-04-05 10:02:07 PDT
Committed r82948: <http://trac.webkit.org/changeset/82948>
Dimitri Glazkov (Google)
Comment 14 2012-06-28 08:31:11 PDT
Comment on attachment 88158 [details] Rebased to ToT. removing flags on a landed patch.
Note You need to log in before you can comment on or make changes to this bug.