1455514 - mozilla::dom::EventTarget should have utility methods to avoid QI

Status: RESOLVED FIXED

(Core :: DOM: Events, enhancement, P3)

Description

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

QI is expensive. So, even if utility methods need to be virtual methods, it should have at least:

* bool IsNode()
* bool IsContent()
* bool IsElement()
* bool IsText()
* nsINode* AsNode()
* nsIContent* AsContent()
* Element* AsElement()
* Text* AsText()

and optionally:
* nsINode* GetAsNode()
* nsIContent* GetAsContent()
* Element* GetAsElement()
* Text* GetAsText()

I don't know about:
nsGlobalWindowInner, nsGlobalWindowOuter, nsPIWindowRoot, DOMEventTargetHelper and its derived classes.

Comment 1

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1455514 - part 1: Add accessors and static helper methods to retrieve `nsINode` or its concrete classes from `EventTarget` r=smaug!

Currently, checking whether an EventTarget is nsINode (or its concrete
classes) or not requires a QI, but it's expensive and used a lot while we
handle each event. Therefore, it'd be nicer for creating a virtual method,
EventTarget::IsNode() and use it for the check.

If trying to convert EventTarget to a concrete class, it may require two
virtual method calls. I'm not sure whether it's cheaper than a QI, but at
least, it won't depend on the UUID check order of QueryInterface() when
multiple interfaces are implemented.

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1455514 - part 2: Add accessors and static helper methods to retrieve `nsPIDOMWindowInner`, `nsPIDOMWindowOuter` and `nsPIWindowRoot` r=smaug!

Similar to the previous patch, there are a lot of QI from EventTarget to
window interfaces. This patch adds them, but unfortunately,
nsPIDOMWindowInner and nsPIDOMWindowOuter do not inherit EventTarget
directly, and they are in a public header instead of their concrete classes
(nsGlobalWindowInner and nsGlobalWindowOuter). So we cannot cast them
in nsPIDOMWindow.h, and it causes it's impossible to use the macros added
by the previous patch.

Depends on D129781

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ad00dd869711
part 1: Add accessors and static helper methods to retrieve `nsINode` or its concrete classes from `EventTarget` r=smaug
https://hg.mozilla.org/integration/autoland/rev/27cd1206ecd9
part 2: Add accessors and static helper methods to retrieve `nsPIDOMWindowInner`, `nsPIDOMWindowOuter` and `nsPIWindowRoot` r=smaug

Comment 4

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/ad00dd869711
https://hg.mozilla.org/mozilla-central/rev/27cd1206ecd9

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Posted to dev-platform: https://groups.google.com/a/mozilla.org/g/dev-platform/c/sKcNy_s11Is