Bug 66458 - [Refactoring] Add EventDispatchMediator.{h,cpp} and move related classes to that files.
Summary: [Refactoring] Add EventDispatchMediator.{h,cpp} and move related classes to t...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: UI Events (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Hayato Ito
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-08-18 05:08 PDT by Hayato Ito
Modified: 2011-08-22 05:49 PDT (History)
6 users (show)

See Also:


Attachments
refactoring (20.12 KB, patch)
2011-08-18 06:01 PDT, Hayato Ito
rniwa: review+
Details | Formatted Diff | Diff
to make sure that the patch can be compiled on all platforms. not for review (23.77 KB, patch)
2011-08-18 21:05 PDT, Hayato Ito
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Hayato Ito 2011-08-18 05:08:09 PDT
Now Event.{h,cpp} contain EventDispatchMediator and Focus/BlurEventDispatchMediator's declarations and definitions.

It might be good to have EventDispatchMediator.{h,cpp} and move related classes to that files from Event.{h,cpp}.
Comment 1 Hayato Ito 2011-08-18 06:01:55 PDT
Created attachment 104329 [details]
refactoring
Comment 2 Ryosuke Niwa 2011-08-18 08:43:39 PDT
Comment on attachment 104329 [details]
refactoring

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

> Source/WebCore/WebCore.xcodeproj/project.pbxproj:26192
> +				4AF1AD3E13FD23A400AA9590 /* EventDispatchMediator.cpp in Sources */,

Please sort this lexicologically.

> Source/WebCore/dom/EventDispatchMediator.h:62
> +inline EventDispatchMediator::EventDispatchMediator()
> +{
> +}

It appears that this can be included in the class declaration.

> Source/WebCore/dom/EventDispatchMediator.h:67
> +inline Event* EventDispatchMediator::event() const
> +{
> +    return m_event.get();
> +}

Ditto.

> Source/WebCore/dom/EventDispatchMediator.h:72
> +inline void EventDispatchMediator::setEvent(PassRefPtr<Event> event)
> +{
> +    m_event = event;
> +}

Ditto.
Comment 3 Hayato Ito 2011-08-18 20:49:14 PDT
Comment on attachment 104329 [details]
refactoring

Thank you for the review.
I'll land this patch after addressing your comments and confirming that can be compiled on all platforms.

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

>> Source/WebCore/WebCore.xcodeproj/project.pbxproj:26192
>> +				4AF1AD3E13FD23A400AA9590 /* EventDispatchMediator.cpp in Sources */,
> 
> Please sort this lexicologically.

Done. Thank you.
I wrongly assumed that these don't have to be sorted because I spotted these are not 'locally' sorted around there. I should have taken a look at that globally. :)

>> Source/WebCore/dom/EventDispatchMediator.h:62
>> +}
> 
> It appears that this can be included in the class declaration.

Done.

>> Source/WebCore/dom/EventDispatchMediator.h:67
>> +}
> 
> Ditto.

Done.

>> Source/WebCore/dom/EventDispatchMediator.h:72
>> +}
> 
> Ditto.

Done.
Comment 4 Hayato Ito 2011-08-18 21:05:31 PDT
Created attachment 104451 [details]
to make sure that the patch can be compiled on all platforms. not for review
Comment 5 Hayato Ito 2011-08-18 21:46:56 PDT
Committed r93385: <http://trac.webkit.org/changeset/93385>
Comment 6 Adam Roben (:aroben) 2011-08-19 08:11:40 PDT
Windows build fix in r93408
Comment 7 Hayato Ito 2011-08-22 05:49:59 PDT
Hi, Adam

(In reply to comment #6)
> Windows build fix in r93408

Thank you for the fix, and I'm sorry for the build broken in Windows.
The lesson is that I have to edit dom/DOMAllInOne.cpp also.