Bug 57639 - Move MouseEvent-dispatching logic into MouseEventDispatchMediator.
Summary: Move MouseEvent-dispatching logic into MouseEventDispatchMediator.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Dimitri Glazkov (Google)
URL:
Keywords:
Depends on: 57562
Blocks: 55515 57642
  Show dependency treegraph
 
Reported: 2011-04-01 09:54 PDT by Dimitri Glazkov (Google)
Modified: 2012-06-28 08:31 PDT (History)
5 users (show)

See Also:


Attachments
Patch (14.06 KB, patch)
2011-04-01 09:57 PDT, Dimitri Glazkov (Google)
no flags Details | Formatted Diff | Diff
Rebased to ToT. (14.13 KB, patch)
2011-04-04 16:44 PDT, Dimitri Glazkov (Google)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dimitri Glazkov (Google) 2011-04-01 09:54:00 PDT
Move MouseEvent-dispatching logic into MouseEventDispatchMediator.
Comment 1 Dimitri Glazkov (Google) 2011-04-01 09:57:23 PDT
Created attachment 87874 [details]
Patch
Comment 2 Dimitri Glazkov (Google) 2011-04-04 16:44:29 PDT
Created attachment 88158 [details]
Rebased to ToT.
Comment 3 Dimitri Glazkov (Google) 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?
Comment 4 Early Warning System Bot 2011-04-04 16:55:22 PDT
Attachment 88158 [details] did not build on qt:
Build output: http://queues.webkit.org/results/8334111
Comment 5 Darin Adler 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.
Comment 6 Darin Adler 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.
Comment 7 Dimitri Glazkov (Google) 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! :)
Comment 8 Dimitri Glazkov (Google) 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.
Comment 9 Build Bot 2011-04-04 19:20:00 PDT
Attachment 88158 [details] did not build on win:
Build output: http://queues.webkit.org/results/8330194
Comment 10 Build Bot 2011-04-04 20:29:41 PDT
Attachment 88158 [details] did not build on win:
Build output: http://queues.webkit.org/results/8338007
Comment 11 WebKit Review Bot 2011-04-05 05:21:19 PDT
Attachment 88158 [details] did not build on mac:
Build output: http://queues.webkit.org/results/8331332
Comment 12 Dimitri Glazkov (Google) 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.
Comment 13 Dimitri Glazkov (Google) 2011-04-05 10:02:07 PDT
Committed r82948: <http://trac.webkit.org/changeset/82948>
Comment 14 Dimitri Glazkov (Google) 2012-06-28 08:31:11 PDT
Comment on attachment 88158 [details]
Rebased to ToT.

removing flags on a landed patch.