Bug 122533 - Turn EventPath into a real class
Summary: Turn EventPath into a real class
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: Ryosuke Niwa
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-10-08 16:47 PDT by Ryosuke Niwa
Modified: 2013-10-08 17:40 PDT (History)
9 users (show)

See Also:


Attachments
Cleanup (15.16 KB, patch)
2013-10-08 16:50 PDT, Ryosuke Niwa
koivisto: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryosuke Niwa 2013-10-08 16:47:56 PDT
Turn EventPath into a real class
Comment 1 Ryosuke Niwa 2013-10-08 16:50:09 PDT
Created attachment 213734 [details]
Cleanup
Comment 2 Antti Koivisto 2013-10-08 17:04:25 PDT
Comment on attachment 213734 [details]
Cleanup

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

> Source/WebCore/ChangeLog:8
> +        Since we can't forward-declare typedef, turning EventPath into a real class helps our defactoring.

The comment about forward-declaring typedef is not necessary. Refactoring strategies not chosen are not that interesting.

> Source/WebCore/dom/EventDispatcher.h:62
> +    const EventContext& item(size_t i) const { return *m_path[i]; }
> +    EventContext& item(size_t i) { return *m_path[i]; }

The interface is not great. item() is vague and and it is not clear which way the path goes. But I suppose you plan to improve this later.
Comment 3 Anders Carlsson 2013-10-08 17:06:02 PDT
Comment on attachment 213734 [details]
Cleanup

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

> Source/WebCore/dom/EventDispatcher.h:68
> +    void shrink(size_t newSize) { m_path.shrink(newSize); }

Can't you just get rid of this?

> Source/WebCore/dom/EventDispatcher.h:73
> +    Vector<OwnPtr<EventContext>, 32> m_path;

This should be std::unique_ptr.

> Source/WebCore/dom/EventRetargeter.cpp:94
> +                m_path.append(adoptPtr(new MouseOrFocusEventContext(node, &currentTarget, target)));

Instead of using adoptPtr(new Foo(, you should use std::make_unique<Foo>(
Comment 4 Ryosuke Niwa 2013-10-08 17:40:34 PDT
Committed r157152: <http://trac.webkit.org/changeset/157152>