Last Comment Bug 661297 - Kill AddEventListenerByIID/RemoveEventListenerByIID
: Kill AddEventListenerByIID/RemoveEventListenerByIID
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM: Events (show other bugs)
: Trunk
: x86 Mac OS X
: -- normal (vote)
: mozilla8
Assigned To: Jonas Sicking (:sicking) No longer reading bugmail consistently
:
:
Mentors:
Depends on: 663461 663749 663752 663768 663879 664058 664061 665586 665596 665599 665609 665632
Blocks:
  Show dependency treegraph
 
Reported: 2011-06-01 10:47 PDT by Jonas Sicking (:sicking) No longer reading bugmail consistently
Modified: 2011-08-14 05:06 PDT (History)
9 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Part 1: Remove API part (62.09 KB, patch)
2011-06-20 11:15 PDT, Jonas Sicking (:sicking) No longer reading bugmail consistently
bugs: review+
Details | Diff | Splinter Review
Part 2: Remove implementation of by-iid listeners (21.79 KB, patch)
2011-06-20 11:20 PDT, Jonas Sicking (:sicking) No longer reading bugmail consistently
bugs: review+
Details | Diff | Splinter Review

Description Jonas Sicking (:sicking) No longer reading bugmail consistently 2011-06-01 10:47:59 PDT
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 2011-06-20 11:15:46 PDT
Created attachment 540541 [details] [diff] [review]
Part 1: Remove API part
Comment 2 Jonas Sicking (:sicking) No longer reading bugmail consistently 2011-06-20 11:20:27 PDT
Created attachment 540546 [details] [diff] [review]
Part 2: Remove implementation of by-iid listeners
Comment 3 :Ms2ger (⌚ UTC+1/+2) 2011-06-21 08:47:22 PDT
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] (reviewing overload) 2011-06-27 08:55:37 PDT
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] (reviewing overload) 2011-06-27 10:59:10 PDT
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 2011-06-28 19:23:41 PDT
Yep. All entry points now specify a name in one way or another.
Comment 7 Jonas Sicking (:sicking) No longer reading bugmail consistently 2011-08-08 11:29:14 PDT
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 9 Daniel Holbert [:dholbert] 2011-08-12 14:52:54 PDT
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 [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-08-14 05:06:08 PDT
http://hg.mozilla.org/mozilla-central/rev/1127ccbf8f4e

Note You need to log in before you can comment on or make changes to this bug.