661297 - Kill AddEventListenerByIID/RemoveEventListenerByIID

Status: RESOLVED FIXED

(Core :: DOM: Events, defect)

Description

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

We should make users use the standard AddEventListener instead and register for the events they want.

The plan of attack here goes something like this:

1. Remove the Add/RemoveEventListenerByIID users a few at a time until none are
   left.
2. Remove the Add/RemoveEventListenerByIID API.
3. Cleanup internal code that deals with IID listeners.

I'll file separate bugs for step one and attach patches there so that we can space them out. There is a decent chance of regressions here so I'd rather land the removals a few at a time spaced out over multiple days as to make bisecting easier.

Steps 2 and 3 can probably happen in this bug though.

Comment 1

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Attachment: Part 1: Remove API part

Comment 2

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Attachment: Part 2: Remove implementation of by-iid listeners

Comment 3

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 540546 [details] [diff] [review]
Part 2: Remove implementation of by-iid listeners

>--- a/content/events/src/nsEventListenerManager.cpp
>+++ b/content/events/src/nsEventListenerManager.cpp
>@@ -1206,53 +1047,23 @@ nsEventListenerManager::GetListenerInfo(
>+    const nsDependentSubstring& eventType =
>+      Substring(nsDependentAtomString(ls.mTypeAtom), 2);
>+    nsRefPtr<nsEventListenerInfo> info =
>+      new nsEventListenerInfo(eventType, ls.mListener, capturing,
>+                              allowsUntrusted, systemGroup);
>+    NS_ENSURE_TRUE(info, NS_ERROR_OUT_OF_MEMORY);

You can remove this line, while you're here.

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 540541 [details] [diff] [review]
Part 1: Remove API part

Splitting the patches makes reviewing a bit harder :/

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 540546 [details] [diff] [review]
Part 2: Remove implementation of by-iid listeners


>+    const nsDependentSubstring& eventType =
>+      Substring(nsDependentAtomString(ls.mTypeAtom), 2);
It is guaranteed to have mTypeAtom, right?

Comment 6

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Yep. All entry points now specify a name in one way or another.

Comment 7

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Checked in to inbound!

http://hg.mozilla.org/integration/mozilla-inbound/rev/c1c58c2bf9df
http://hg.mozilla.org/integration/mozilla-inbound/rev/845547a3c281

Death to by-iid event-listeners! Woot!

Comment 8

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/c1c58c2bf9df
http://hg.mozilla.org/mozilla-central/rev/845547a3c281

Comment 9

[Daniel Holbert [:dholbert]]

Landed followup to remove reference to a now-missing Makefile from toolkit-makefiles.sh (now-missing because of the "dom/public" deletion in c1c58c2bf9df):
  http://hg.mozilla.org/integration/mozilla-inbound/rev/1127ccbf8f4e

This followup fixes the new build warning:
{
creating dom/public/coreEvents/Makefile
can't read /builds/slave/m-cen-lnx64-dbg/build/dom/public/coreEvents/Makefile.in: No such file or directory
}
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1313167929.1313174225.25821.gz

Comment 10

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

http://hg.mozilla.org/mozilla-central/rev/1127ccbf8f4e