Bug 222095 - Avoid heap allocation for EventContexts
Summary: Avoid heap allocation for EventContexts
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-02-18 00:59 PST by Ryosuke Niwa
Modified: 2021-02-25 00:31 PST (History)
12 users (show)

See Also:


Attachments
WIP (21.08 KB, patch)
2021-02-18 01:00 PST, Ryosuke Niwa
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
WIP2 (21.53 KB, patch)
2021-02-18 18:58 PST, Ryosuke Niwa
no flags Details | Formatted Diff | Diff
WIP2 + cleanup (27.51 KB, patch)
2021-02-19 17:59 PST, Ryosuke Niwa
no flags Details | Formatted Diff | Diff
Patch (33.71 KB, patch)
2021-02-24 21:51 PST, Ryosuke Niwa
simon.fraser: 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 2021-02-18 00:59:51 PST
We don't need to a class hierarchy to represent different kinds of EventContext.
Merge all the subclasses of EventContext onto itself to avoid heap allocation in EventPath.
Comment 1 Ryosuke Niwa 2021-02-18 01:00:10 PST
Created attachment 420804 [details]
WIP
Comment 2 Ryosuke Niwa 2021-02-18 18:58:12 PST
Created attachment 420901 [details]
WIP2
Comment 3 Ryosuke Niwa 2021-02-19 17:59:50 PST
Created attachment 421066 [details]
WIP2 + cleanup
Comment 4 Radar WebKit Bug Importer 2021-02-22 00:33:39 PST
<rdar://problem/74586915>
Comment 5 Ryosuke Niwa 2021-02-24 21:51:29 PST
Created attachment 421500 [details]
Patch
Comment 6 Simon Fraser (smfr) 2021-02-24 22:16:06 PST
Comment on attachment 421500 [details]
Patch

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

> Source/WebCore/dom/EventContext.h:62
> +    bool isMouseOrFocusEventContext() const { return m_type == Type::MouseOrFocus; }
> +    bool isTouchEventContext() const { return m_type == Type::Touch; }
> +    bool isWindowContext() const { return m_type == Type::Window; }

Not this patch, but it's weird how the context is sometimes about the type of event, and sometimes about the object it's fired on?

> Source/WebCore/dom/EventContext.h:104
> +    , m_type(type)

Can't this be m_type { type } ?
Comment 7 Ryosuke Niwa 2021-02-25 00:22:48 PST
(In reply to Simon Fraser (smfr) from comment #6)
> Comment on attachment 421500 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=421500&action=review
> 
> > Source/WebCore/dom/EventContext.h:62
> > +    bool isMouseOrFocusEventContext() const { return m_type == Type::MouseOrFocus; }
> > +    bool isTouchEventContext() const { return m_type == Type::Touch; }
> > +    bool isWindowContext() const { return m_type == Type::Window; }
> 
> Not this patch, but it's weird how the context is sometimes about the type
> of event, and sometimes about the object it's fired on?

Yup, we should do that. I was initially going to do that in this patch but I decided against it to keep this patch simple & mechanical.

> > Source/WebCore/dom/EventContext.h:104
> > +    , m_type(type)
> 
> Can't this be m_type { type } ?

Oops, fixed. Not sure what happened there.

Thanks for the reviews!
Comment 8 Ryosuke Niwa 2021-02-25 00:31:02 PST
Committed r273477 (234556@main): <https://commits.webkit.org/234556@main>