Last Comment Bug 658714 - Many nsDOMEventRTTearoff instances allocated and destroyed when processing user input in a variety of circumstances
: Many nsDOMEventRTTearoff instances allocated and destroyed when processing us...
Status: RESOLVED FIXED
: dev-doc-needed
Product: Core
Classification: Components
Component: DOM: Events (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Jonas Sicking (:sicking)
:
Mentors:
http://gmail.com/
Depends on: 661980 661984 667612 668328
Blocks:
  Show dependency treegraph
 
Reported: 2011-05-20 17:20 PDT by K. Gadd (:kael)
Modified: 2012-08-13 00:56 PDT (History)
9 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Add simple Telemetry probes (viewable with about:telemetry if the extension is installed) for creation/destruction of dom event tearoffs (2.78 KB, patch)
2011-05-20 17:20 PDT, K. Gadd (:kael)
no flags Details | Diff | Review
Part 1: Merge nsPIDOMEventTarget into nsIDOMEventTarget (90.83 KB, patch)
2011-05-30 01:01 PDT, Jonas Sicking (:sicking)
no flags Details | Diff | Review
Part 2: Merge nsIDOMNSEventTarget into nsIDOMEventTarget (90.83 KB, patch)
2011-05-30 01:03 PDT, Jonas Sicking (:sicking)
bugs: review+
Details | Diff | Review
Part 3: Move nsIDOMEventTarget implementation to nsINode (34.85 KB, patch)
2011-05-30 01:05 PDT, Jonas Sicking (:sicking)
no flags Details | Diff | Review
Part 4: Make windows happy. WIP (10.09 KB, patch)
2011-05-30 01:07 PDT, Jonas Sicking (:sicking)
no flags Details | Diff | Review
Part 5: Search'n'replace nsPIDOMEventTarget to nsIDOMEventTarget (119.63 KB, patch)
2011-05-30 01:09 PDT, Jonas Sicking (:sicking)
bugs: review+
Details | Diff | Review
Part 6: Fixups after search'n'replace (50.69 KB, patch)
2011-05-30 01:11 PDT, Jonas Sicking (:sicking)
bugs: review+
Details | Diff | Review
Part 7: DeCOMtaminate nsEventListenerManager (118.70 KB, patch)
2011-05-30 01:14 PDT, Jonas Sicking (:sicking)
bugs: review+
Details | Diff | Review
Part 8: Improve the nsEventListenerManager API (42.92 KB, patch)
2011-05-30 01:17 PDT, Jonas Sicking (:sicking)
bugs: review+
Details | Diff | Review
Part 9: Don't use EventGroups for system-group (73.35 KB, patch)
2011-05-30 01:19 PDT, Jonas Sicking (:sicking)
bugs: review+
Details | Diff | Review
Part 10: Remove nsIDOM3EventTarget/nsIDOMEventGroup (98.38 KB, patch)
2011-05-30 01:20 PDT, Jonas Sicking (:sicking)
bugs: review+
Details | Diff | Review
Part 1: Merge nsPIDOMEventTarget into nsIDOMEventTarget (80.95 KB, patch)
2011-05-30 01:21 PDT, Jonas Sicking (:sicking)
bugs: review+
Details | Diff | Review
Part 1: Merge nsPIDOMEventTarget into nsIDOMEventTarget (81.02 KB, patch)
2011-05-30 12:06 PDT, Jonas Sicking (:sicking)
bent.mozilla: review+
Details | Diff | Review
Part 2: Merge nsIDOMNSEventTarget into nsIDOMEventTarget (90.74 KB, patch)
2011-05-31 10:04 PDT, Jonas Sicking (:sicking)
no flags Details | Diff | Review
Part 3: Move nsIDOMEventTarget implementation to nsINode (44.26 KB, patch)
2011-05-31 10:05 PDT, Jonas Sicking (:sicking)
bugs: review+
Details | Diff | Review
The whole thing (468.70 KB, patch)
2011-05-31 10:07 PDT, Jonas Sicking (:sicking)
no flags Details | Diff | Review
Part 4: Make windows happy. (5.97 KB, patch)
2011-06-05 04:17 PDT, Jonas Sicking (:sicking)
bugs: review+
Details | Diff | Review

Description K. Gadd (:kael) 2011-05-20 17:20:01 PDT
Created attachment 534162 [details] [diff] [review]
Add simple Telemetry probes (viewable with about:telemetry if the extension is installed) for creation/destruction of dom event tearoffs

In various places, we use nsDOMEventRTTearoff to satisfy QueryInterface requests for nsIDOMEventTarget, nsIDOM3EventTarget, and nsIDOMNSEventTarget. Allocation profiles of many websites (and even the Firefox UI chrome itself) show that while nsDOMEventRTTearoff has a small cache designed to satisfy requests for short-lived tearoffs, in practice, we end up allocating thousands of instances of this class and then at some point throwing them away. In particular, this is the #1 source of allocations in the profile runs I've done of gmail.

Given the small number of members exposed by the interfaces in question, and given the frequency with which some of those members (like addEventListener) are used, I believe replacing this tearoff with an actual interface implementation, at least for nodes, would be a performance win for a large number of use cases. The fact that these tearoffs also seem to participate in cycle collection suggests that we could improve cycle collector pause times and reduce the frequency of pauses by eliminating them, as well.

I've included a patch that adds a couple telemetry probes that you can use to see how many of our requests to create tearoffs are actually serviced by the tearoff cache. In my tests, the hit rate for the cache was between 80-90%, which in practice meant we would allocate and then throw away a few thousand instances of the class in a few minutes of gmail use. 

I did some more analysis with more complex probes and it appears that in practice, even with a larger cache size, the cache almost remains filled to capacity due to the number of long-lived tearoff instances we create and hold onto. The average lifetime of a tearoff is well under 1 millisecond, but despite that in practice thousands of tearoffs will end up living as long as the document they're associated with. I was unable to determine in what specific cases a tearoff would become long-lived.
Comment 1 Olli Pettay [:smaug] 2011-05-20 17:28:05 PDT
nsDOMEventRTTearoff is ancient code. I think it worked quite well before
cycle collector. Nowadays the cache is probably pretty useless.

(I don't remember seeing nsDOMEventRTTearoff in the performance profiles though.)
Comment 2 Boris Zbarsky [:bz] 2011-05-20 18:22:46 PDT
We should collapse nsIDOM3EventTarget and nsIDOMNSEventTarget into nsIDOMEventTarget.

And if there is agreement about having Node and Window inherit from EventTarget, I would _love_ for that to happen.  Then we wouldn't need tearoffs here at all.

Kevin, right now making this a non-tearoff would make all nodes bigger by a word (if we collapse the interfaces).  Maybe that's OK...
Comment 3 Jonas Sicking (:sicking) 2011-05-20 20:02:53 PDT
Now that DOM Core specifies that Node should inherit EventTarget, this is a no-brainer.

I'll take this.
Comment 4 Olli Pettay [:smaug] 2011-05-21 02:25:09 PDT
It would be great if nsPIDOMEventTarget could extend nsIDOMEventTarget (to which other nsIDOM*EventTarget interfaces were collapsed).
There is in fact a bug open for that, IIRC.
Comment 5 Jonas Sicking (:sicking) 2011-05-25 20:22:14 PDT
Here's my plan of attack:

1. Merge nsPIDOMEventTarget into nsIDOMEventTarget. All classes that implement
   the former also implement the latter, so leave all these classes implementing
   nsIDOMEventTarget.
   This leaves nsINode inheriting nsIDOMEventTarget.
2. Merge nsIDOMNSEventTarget into nsIDOMEventTarget.

At this point we no longer use any tearoffs for event targets in the vast majority of cases, so this bug would essentially be fixed. However, I will also

3. Move nsIDOMEventTarget from being base for nsINode to be base of nsIDOMNode.
   This will require more forwarding, but aligns us with specs and makes the
   EventTarget interface object actually useful. And we can avoid calling
   through the forwards for scripts by using quickstubs.
4. Move nsIDOMEventTarget to be a base for Window/XHR/FileReader/IndexedDB stuff

I might do these last two in separate bugs, but wanted to mention the full plan in here for completeness.
Comment 6 Olli Pettay [:smaug] 2011-05-26 03:23:54 PDT
1 and 2 sounds good and something I've been thinking for years.
It just haven't been possible because of frozen interfaces.

3 is a bit worrying.
nsGenericElement doesn't inherit nsIDOMNode, yet in PreHandleEvent it
should be able to set the parent event target in event target chain.
Any QIing (in common case) in nsGenericElement::PreHandleEvent is not
acceptable. And adding virtual calls is out of question too.
Could we make nsINode to inherit nsIDOMNode?
Comment 7 Boris Zbarsky [:bz] 2011-05-26 08:01:44 PDT
> Could we make nsINode to inherit nsIDOMNode?

Not without increasing the size of all elements by a word...  However the right solution there is probably to continue having an internal event target API that we use in PreHandleEvent or something.  Jonas is assuming something like that anyway for the quickstub business.

I admit that I'd like to understand what the plan for this internal API is, exactly.
Comment 8 Jonas Sicking (:sicking) 2011-05-26 08:37:49 PDT
My plan for 3 was to implement the whole nsIDOMEventTarget API on nsINode directly (but make nsINode not inherit nsIDOMEventTarget) so that subclasses can forward the API somewhere. This is similar to how we do nsIDOMNode for example.

Quickstubs could then simply call the version on nsINode.

However this does not solve how to handle PreHandleEvent. I really would like to avoid haveing two separate interfaces like we do now (nsIDOM/nsPIDOMEventTarget).

So no ideas yet, I'll keep thinking about it.
Comment 9 Jonas Sicking (:sicking) 2011-05-30 01:01:06 PDT
Created attachment 536015 [details] [diff] [review]
Part 1: Merge nsPIDOMEventTarget into nsIDOMEventTarget

This moves the contents of nsPIDOMEventTarget into the nsIDOMEventTarget interface.

I tried to do as minimal changes as possible to get this building as to keep it easier to review. However since C++ doesn't let you forward declare a typedef I still had to make a few s/nsPIDOMEventTarget/nsIDOMEventTarget/

There are currently two classes that implement nsIDOMEventTarget but not nsPIDOMEventTarget: nsDOMWorkerMessageHandler and nsEventListenerManager. I had to add stub implementations to those for now, but the nsEventListenerManager stubs are going away in later patches, and the nsDOMWorkerMessageHandler stubs will go away with the worker rewrite bent is working on elsewhere.
Comment 10 Jonas Sicking (:sicking) 2011-05-30 01:03:10 PDT
Created attachment 536016 [details] [diff] [review]
Part 2: Merge nsIDOMNSEventTarget into nsIDOMEventTarget

This merges nsIDOMNSEventTarget into nsIDOMEventTarget. I got rid of the scriptTypeID getter/setter as they were unused.
Comment 11 Jonas Sicking (:sicking) 2011-05-30 01:05:36 PDT
Created attachment 536018 [details] [diff] [review]
Part 3: Move nsIDOMEventTarget implementation to nsINode

As things stand, nsIDOMEventTarget were implemented in nsGenericElement, nsGenericDOMDataNode, nsDocument and nsDOMAttribute. Instead this moves the bulk of the implementation to nsINode and only adds overrides where needed (mostly PreHandleEvent).
Comment 12 Jonas Sicking (:sicking) 2011-05-30 01:07:37 PDT
Created attachment 536019 [details] [diff] [review]
Part 4: Make windows happy. WIP

Since these functions are now declared in .idl, we need to use the NS_IMETHOD macros in the signatures.

This patch isn't done yet, unfortunately I need to add the macro in all overrides, but I need to get hold of a windows box to do that properly.
Comment 13 Jonas Sicking (:sicking) 2011-05-30 01:09:37 PDT
Created attachment 536020 [details] [diff] [review]
Part 5: Search'n'replace nsPIDOMEventTarget to nsIDOMEventTarget

Written entirely using a sed script which changed
nsPIDOMEventTarget->nsIDOMEventTarget
GetPIDOMEventTarget->GetDOMEventTarget
Comment 14 Jonas Sicking (:sicking) 2011-05-30 01:11:17 PDT
Created attachment 536021 [details] [diff] [review]
Part 6: Fixups after search'n'replace

This removes a few QIs and nullchecks that were needed when nsPIDOMEventTarget and nsIDOMEventTarget were different interfaces.
Comment 15 Jonas Sicking (:sicking) 2011-05-30 01:14:00 PDT
Created attachment 536023 [details] [diff] [review]
Part 7: DeCOMtaminate nsEventListenerManager

This changes nsEventListenerManager to not use interfaces. Instead callers use the class directly.

I suspect that we can get rid of the refcounting on nsEventListenerManager entirely, and make the object owned by the respective DOMEventTarget, but I figured that was a bit risky for now.
Comment 16 Jonas Sicking (:sicking) 2011-05-30 01:17:47 PDT
Created attachment 536025 [details] [diff] [review]
Part 8: Improve the nsEventListenerManager API

Makes functions non-virtual, makes functions return void where possible, add better functions to adding/removing event listeners to avoid having callers do flag logic.
Comment 17 Jonas Sicking (:sicking) 2011-05-30 01:19:14 PDT
Created attachment 536026 [details] [diff] [review]
Part 9: Don't use EventGroups for system-group

Use a flag rather than a magic EventGroup to indicate eventlisteners that are system event listeners.
Comment 18 Jonas Sicking (:sicking) 2011-05-30 01:20:19 PDT
Created attachment 536027 [details] [diff] [review]
Part 10: Remove nsIDOM3EventTarget/nsIDOMEventGroup

Remove the now unused nsIDOM3EventTarget and nsIDOMEventGroup interfaces.
Comment 19 Jonas Sicking (:sicking) 2011-05-30 01:21:59 PDT
Created attachment 536029 [details] [diff] [review]
Part 1: Merge nsPIDOMEventTarget into nsIDOMEventTarget

Attached the wrong patch for part 1. This one's correct. Description in comment 9 still applies.
Comment 20 :Ms2ger 2011-05-30 01:49:20 PDT
nsIDOMEventTarget is now included twice in at least

content/events/src/nsDOMEventTargetHelper.h
content/events/src/nsEventListenerService.h
editor/libeditor/base/nsEditor.cpp
editor/libeditor/html/nsHTMLObjectResizer.cpp
extensions/spellcheck/src/mozInlineSpellChecker.cpp
Comment 21 :Ms2ger 2011-05-30 01:50:19 PDT
Comment on attachment 536023 [details] [diff] [review]
Part 7: DeCOMtaminate nsEventListenerManager

>diff --git a/content/events/src/nsEventListenerManager.h b/content/events/src/nsEventListenerManager.h
>--- a/content/events/src/nsEventListenerManager.h
>+++ b/content/events/src/nsEventListenerManager.h
>@@ -33,59 +33,69 @@
>  * the provisions above, a recipient may use your version of this file under
>  * the terms of any one of the MPL, the GPL or the LGPL.
>  *
>  * ***** END LICENSE BLOCK ***** */
> 
> #ifndef nsEventListenerManager_h__
> #define nsEventListenerManager_h__
> 
>-#include "nsIEventListenerManager.h"
>+#include "nsEventListenerManager.h"

That's a fun dependency
Comment 22 :Ms2ger 2011-05-30 02:07:00 PDT
I have to note that I love what you're doing here, and not only because some of these patches were on my todo list too :)
Comment 23 Olli Pettay [:smaug] 2011-05-30 05:25:11 PDT
Comment on attachment 536016 [details] [diff] [review]
Part 2: Merge nsIDOMNSEventTarget into nsIDOMEventTarget


> nsEventListenerManager::AddEventListener(const nsAString& aType, 
>                                          nsIDOMEventListener* aListener, 
>-                                         PRBool aUseCapture)
>+                                         PRBool aUseCapture,
>+                                         PRBool aWantsUntrusted,
>+                                         PRUint8 optional_argc)
aOptionalArgc

> nsGlobalWindow::AddEventListener(const nsAString& aType,
>                                  nsIDOMEventListener *aListener,
>                                  PRBool aUseCapture, PRBool aWantsUntrusted,
>                                  PRUint8 optional_argc)
> {
>+  // XXX the old AddEventListener used FORWARD_TO_INNER_CREATE here
>+  // should this AddEventListener too?
GetListenerManager forward-create inner window few lines below.
That should be enough.

>+++ b/dom/interfaces/events/nsIDOMEventTarget.idl
update uuid


> NS_INTERFACE_MAP_BEGIN(nsDOMWorker)
>   NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsIWorker)
>   NS_INTERFACE_MAP_ENTRY(nsIClassInfo)
>   NS_INTERFACE_MAP_ENTRY(nsIXPCScriptable)
>   NS_INTERFACE_MAP_ENTRY(nsIWorker)
>   NS_INTERFACE_MAP_ENTRY(nsIAbstractWorker)
>-  NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsIDOMNSEventTarget,
>+  NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsIDOMEventTarget,
>                                    nsDOMWorkerMessageHandler)
>   NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsIDOMEventTarget, nsDOMWorkerMessageHandler)
two entries for nsIDOMEventTarget?
Comment 24 Olli Pettay [:smaug] 2011-05-30 07:33:58 PDT
Comment on attachment 536029 [details] [diff] [review]
Part 1: Merge nsPIDOMEventTarget into nsIDOMEventTarget


>+  NS_IMETHOD AddEventListener(const nsAString& aType,
>+                              nsIDOMEventListener *aListener,
>+                              PRBool aUseCapture,
>+                              PRBool aWantsUntrusted,
>+                              PRUint8 optional_argc);
Nit, aOptionalArgc


>+nsDOMAttribute::AddEventListener(const nsAString& aType,
>+                                 nsIDOMEventListener *aListener,
>+                                 PRBool useCapture)
aUseCapture


>+nsDOMAttribute::DispatchEvent(nsIDOMEvent *aEvt, PRBool* _retval)
aRetVal or somthing like that


>+  nsresult AddEventListener(const nsAString& aType,
>+                            nsIDOMEventListener *aListener,
>+                            PRBool aUseCapture,
>+                            PRBool aWantsUntrusted,
>+                            PRUint8 optional_argc)
aOptionalArgc


>+nsGenericDOMDataNode::RemoveEventListener(const nsAString& aType,
>+                                          nsIDOMEventListener* aListener,
>+                                          PRBool aUseCapture)
>+{
>+  nsCOMPtr<nsIDOMEventTarget> event_target =
>+    do_QueryInterface(GetListenerManager(PR_TRUE));
>+  NS_ENSURE_STATE(event_target);
>+
>+  return event_target->RemoveEventListener(aType, aListener, aUseCapture);
I wish we wouldn't need to do this QI, but perhaps some other patch is fixing that...
Also, GetListenerManager(PR_TRUE) is wrong. No need to create ELM when we're removing a listener.
If there isn't ELM, there isn't any listener to remove.
So, 
nsCOMPtr<nsIDOMEventTarget> elm =
  do_QueryInterface(GetListenerManager(PR_TRUE));
return elm ? elm->event_target->RemoveEventListener(aType, aListener, aUseCapture) : NS_OK;

Please check also other cases when GetListenerManager is used.
>+nsINode::AddEventListener(const nsAString& aType,
>+                          nsIDOMEventListener *aListener,
>+                          PRBool aUseCapture,
>+                          PRBool aWantsUntrusted,
>+                          PRUint8 optional_argc)
aOptionalArgc


>+nsGenericElement::RemoveEventListener(const nsAString& aType,
>+                                      nsIDOMEventListener* aListener,
>+                                      PRBool aUseCapture)
>+{
>+  nsCOMPtr<nsIDOMEventTarget> event_target =
>+    do_QueryInterface(GetListenerManager(PR_TRUE));
PR_FALSE etc.


>@@ -309,45 +305,42 @@ class nsGenericElement : public mozilla:
> public:
>   nsGenericElement(already_AddRefed<nsINodeInfo> aNodeInfo);
>   virtual ~nsGenericElement();
> 
>   friend class nsNSElementTearoff;
> 
>   NS_DECL_CYCLE_COLLECTING_ISUPPORTS
> 
>+  NS_DECL_NSIDOMEVENTTARGET
>+
>+  nsresult AddEventListener(const nsAString& aType,
>+                            nsIDOMEventListener *aListener,
>+                            PRBool aUseCapture,
>+                            PRBool aWantsUntrusted,
>+                            PRUint8 optional_argc)
aOptionalArgc


> nsXMLHttpRequest::RootResultArrayBuffer()
> {
>-  nsContentUtils::PreserveWrapper(static_cast<nsPIDOMEventTarget*>(this), this);
>+  nsContentUtils::PreserveWrapper(
>+    static_cast<nsIDOMEventTarget*>(
>+      static_cast<nsDOMEventTargetHelper*>(_obj)), this);
What is _obj?


>+NS_IMETHODIMP_(nsIDOMEventTarget *)
>+nsEventListenerManager::GetTargetForDOMEvent()
>+{
>+  NS_ERROR("Should not be called");
>+  return nsnull;
>+}
>+
>+NS_IMETHODIMP_(nsIDOMEventTarget *)
>+nsEventListenerManager::GetTargetForEventTargetChain()
>+{
>+  NS_ERROR("Should not be called");
>+  return nsnull;
>+}
These error should really be runtime aborts 


>   // Hooks up the global key event handlers to the document root.
>-  NS_IMETHOD AttachGlobalKeyHandler(nsPIDOMEventTarget* aTarget)=0;
>-  NS_IMETHOD DetachGlobalKeyHandler(nsPIDOMEventTarget* aTarget)=0;
>+  NS_IMETHOD AttachGlobalKeyHandler(nsIDOMEventTarget* aTarget)=0;
>+  NS_IMETHOD DetachGlobalKeyHandler(nsIDOMEventTarget* aTarget)=0;
While you're here, please fix the syntax. Space before and after =


>-class nsPIWindowRoot : public nsPIDOMEventTarget {
>+class nsPIWindowRoot : public nsIDOMEventTarget {
While you're here, please fix the syntax.
{ should be in the next line.


> [scriptable, uuid(1c773b30-d1cf-11d2-bd95-00805f8ae3f4)]
> interface nsIDOMEventTarget : nsISupports
update uuid.

And, does XPConnect now prevent JS to implement this interface?


For workers part, bent really should look at this.
Comment 25 Jonas Sicking (:sicking) 2011-05-30 11:16:39 PDT
(In reply to comment #24)
> Comment on attachment 536029 [details] [diff] [review] [review]
> Part 1: Merge nsPIDOMEventTarget into nsIDOMEventTarget
> 
> 
> >+  NS_IMETHOD AddEventListener(const nsAString& aType,
> >+                              nsIDOMEventListener *aListener,
> >+                              PRBool aUseCapture,
> >+                              PRBool aWantsUntrusted,
> >+                              PRUint8 optional_argc);
> Nit, aOptionalArgc

Mind if I fix this, and other argument naming issues, in the part 3 patch since a lot of the implementations of these functions are consolidated there?

> >+nsGenericDOMDataNode::RemoveEventListener(const nsAString& aType,
> >+                                          nsIDOMEventListener* aListener,
> >+                                          PRBool aUseCapture)
> >+{
> >+  nsCOMPtr<nsIDOMEventTarget> event_target =
> >+    do_QueryInterface(GetListenerManager(PR_TRUE));
> >+  NS_ENSURE_STATE(event_target);
> >+
> >+  return event_target->RemoveEventListener(aType, aListener, aUseCapture);
> I wish we wouldn't need to do this QI, but perhaps some other patch is
> fixing that...

Yes, part 7 gets rid of the QI. That was in fact the original reason I wrote part 7.

> Also, GetListenerManager(PR_TRUE) is wrong. No need to create ELM when we're
> removing a listener.
> If there isn't ELM, there isn't any listener to remove.

This is fixed in later patches. Mind if I just leave it that way?

> Please check also other cases when GetListenerManager is used.

This should be fixed in the part 8 and part 10 patches.

> > nsXMLHttpRequest::RootResultArrayBuffer()
> > {
> >-  nsContentUtils::PreserveWrapper(static_cast<nsPIDOMEventTarget*>(this), this);
> >+  nsContentUtils::PreserveWrapper(
> >+    static_cast<nsIDOMEventTarget*>(
> >+      static_cast<nsDOMEventTargetHelper*>(_obj)), this);
> What is _obj?

Ugh. Last-minute rejuggling of code as I did a self-review before attaching. That should say 'this' instead of '_obj'.

> >+NS_IMETHODIMP_(nsIDOMEventTarget *)
> >+nsEventListenerManager::GetTargetForDOMEvent()
> >+{
> >+  NS_ERROR("Should not be called");
> >+  return nsnull;
> >+}
> >+
> >+NS_IMETHODIMP_(nsIDOMEventTarget *)
> >+nsEventListenerManager::GetTargetForEventTargetChain()
> >+{
> >+  NS_ERROR("Should not be called");
> >+  return nsnull;
> >+}
> These error should really be runtime aborts 

As mentioned in the comment when attaching this, these stubs are going away. In part 7 to be precise.

> > [scriptable, uuid(1c773b30-d1cf-11d2-bd95-00805f8ae3f4)]
> > interface nsIDOMEventTarget : nsISupports
> update uuid.

Will do.

> And, does XPConnect now prevent JS to implement this interface?

I don't actually know what happens. Do we have any APIs that take nsIDOMEventTargets?
Comment 26 Jonas Sicking (:sicking) 2011-05-30 12:03:34 PDT
(In reply to comment #20)
> nsIDOMEventTarget is now included twice in at least
> 
> content/events/src/nsDOMEventTargetHelper.h
> content/events/src/nsEventListenerService.h
> editor/libeditor/base/nsEditor.cpp
> editor/libeditor/html/nsHTMLObjectResizer.cpp
> extensions/spellcheck/src/mozInlineSpellChecker.cpp

I fixed these as well as a few others. Including a few where both the X.h and X.cpp file included nsIDOMEventTarget.
Comment 27 Jonas Sicking (:sicking) 2011-05-30 12:06:50 PDT
Created attachment 536141 [details] [diff] [review]
Part 1: Merge nsPIDOMEventTarget into nsIDOMEventTarget

Updated part 1
Comment 28 Jonas Sicking (:sicking) 2011-05-31 10:04:30 PDT
Created attachment 536334 [details] [diff] [review]
Part 2: Merge nsIDOMNSEventTarget into nsIDOMEventTarget

Fix review comments.
Comment 29 Jonas Sicking (:sicking) 2011-05-31 10:05:29 PDT
Created attachment 536336 [details] [diff] [review]
Part 3: Move nsIDOMEventTarget implementation to nsINode

Fix review comments. Just change argument names.
Comment 30 Jonas Sicking (:sicking) 2011-05-31 10:07:00 PDT
Created attachment 536337 [details] [diff] [review]
The whole thing

Figured I'd add a roll-up patch as well, so that you can more easily verify my claims that later patches will address various things.

(Not planning on landing this one, will land the individual parts)
Comment 31 Ben Turner (not reading bugmail, use the needinfo flag!) 2011-05-31 14:57:57 PDT
Comment on attachment 536337 [details] [diff] [review]
The whole thing

The worker changes look good to me fwiw.
Comment 32 Ben Turner (not reading bugmail, use the needinfo flag!) 2011-05-31 14:58:52 PDT
Comment on attachment 536141 [details] [diff] [review]
Part 1: Merge nsPIDOMEventTarget into nsIDOMEventTarget

r=me on the worker changes
Comment 33 Craig Topper 2011-06-01 07:19:13 PDT
I think you forgot to remove NS_DOMEVENTGROUP_CID from nsContentCID.h.
Comment 34 Olli Pettay [:smaug] 2011-06-01 08:23:43 PDT
Comment on attachment 536336 [details] [diff] [review]
Part 3: Move nsIDOMEventTarget implementation to nsINode

>diff --git a/content/base/public/nsIContent.h b/content/base/public/nsIContent.h
>--- a/content/base/public/nsIContent.h
>+++ b/content/base/public/nsIContent.h
Update IID

>   // nsIDOMEventTarget
>-  NS_DECL_NSIDOMEVENTTARGET
>+  NS_IMETHODIMP PreHandleEvent(nsEventChainPreVisitor& aVisitor);
>+  NS_IMETHOD_(nsIEventListenerManager*)
>+  GetListenerManager(PRBool aCreateIfNotFound);
Add 2 spaces before GetListenerManager 

>+nsINode::RemoveEventListener(const nsAString& aType,
>+                             nsIDOMEventListener* aListener,
>+                             PRBool aUseCapture)
>+{
>+  nsCOMPtr<nsIDOMEventTarget> event_target =
>+    do_QueryInterface(GetListenerManager(PR_TRUE));
PR_FALSE


>+  NS_ENSURE_STATE(event_target);
And then
if (!event_target) {
  return NS_OK;
}

>+nsINode::PreHandleEvent(nsEventChainPreVisitor& aVisitor)
>+{
>+  // This is only here so that we can use the NS_DECL_NSIDOMTARGET macro
>+  NS_ERROR("Override PreHandleEvent in subclasses");
This could be runtime abort.
Comment 35 Jonas Sicking (:sicking) 2011-06-01 08:31:56 PDT
Comment on attachment 536336 [details] [diff] [review]
Part 3: Move nsIDOMEventTarget implementation to nsINode

Actually, updating the nsIContent IID shouldn't be needed since this only overrides a function on a baseclass. No binary compat changes.
Comment 36 Olli Pettay [:smaug] 2011-06-01 08:36:32 PDT
Comment on attachment 536026 [details] [diff] [review]
Part 9: Don't use EventGroups for system-group


>-[scriptable, uuid(663e33d7-eca2-42e8-af92-5df6a5e222df)]
>+[scriptable, uuid(68c4a660-1384-4fcc-8636-48793ee73b7a)]
> interface nsIDOMWindowUtils : nsISupports {
> 
>   /**
>    * Image animation mode of the window. When this attribute's value
>    * is changed, the implementation should set all images in the window
>    * to the given value. That is, when set to kDontAnimMode, all images
>    * will stop animating. The attribute's value must be one of the
>    * animationMode values from imgIContainer.
>@@ -881,9 +883,25 @@ interface nsIDOMWindowUtils : nsISupport
>    * the bounds of the window. Always returns true in non-DEBUG builds.
>    */
>   boolean leafLayersPartitionWindow();
> 
>   /**
>    * true if the (current inner) window may have event listeners for touch events.
>    */
>   readonly attribute boolean mayHaveTouchEventListeners;
>+
>+  /**
>+   * Add a system-group eventlistener to a event target.
>+   */
>+  void addSystemEventListener(in nsIDOMEventTarget target,
>+                              in DOMString type,
>+                              in nsIDOMEventListener listener,
>+                              in boolean useCapture);
>+
>+  /**
>+   * Remove a system-group eventlistener from a event target.
>+   */
>+  void removeSystemEventListener(in nsIDOMEventTarget target,
>+                                 in DOMString type,
>+                                 in nsIDOMEventListener listener,
>+                                 in boolean useCapture);


Actually, put these to nsIEventListenerService. That way listeners
can be added / removed even without a window.
And the listeners don't have anything to do with window.

>@@ -199,16 +199,25 @@ SpecialPowers.prototype = {
> 
>   createSystemXHR: function() {
>     return Cc["@mozilla.org/xmlextras/xmlhttprequest;1"]
>              .createInstance(Ci.nsIXMLHttpRequest);
>   },
> 
>   gc: function() {
>     this.DOMWindowUtils.garbageCollect();
>+  },
>+
>+  addSystemEventListener: function(target, type, listener, useCapture) {
>+    this.DOMWindowUtils.addSystemEventListener(target, type, listener,
>+                                               useCapture);
>+  },
>+  removeSystemEventListener: function(target, type, listener, useCapture) {
>+    this.DOMWindowUtils.removeSystemEventListener(target, type, listener,
>+                                                  useCapture);
FYI, all the methods from DOMWindowUtils are automatically forwarded to SpecialPowers.
But since I'd like to see the method to be in EventListenerService, you need these two methods -
just make them use ELS.


r=me with the DOMWindowUtils->ELS change.
Comment 37 Olli Pettay [:smaug] 2011-06-03 03:40:14 PDT
Comment on attachment 536020 [details] [diff] [review]
Part 5: Search'n'replace nsPIDOMEventTarget to nsIDOMEventTarget

>diff --git a/content/events/public/nsPIDOMEventTarget.h b/content/events/public/nsPIDOMEventTarget.h
>--- a/content/events/public/nsPIDOMEventTarget.h
>+++ b/content/events/public/nsPIDOMEventTarget.h
>@@ -30,18 +30,18 @@
>  * use your version of this file under the terms of the MPL, indicate your
>  * decision by deleting the provisions above and replace them with the notice
>  * and other provisions required by the GPL or the LGPL. If you do not delete
>  * the provisions above, a recipient may use your version of this file under
>  * the terms of any one of the MPL, the GPL or the LGPL.
>  *
>  * ***** END LICENSE BLOCK ***** */
> 
>-#ifndef nsPIDOMEventTarget_h_
>-#define nsPIDOMEventTarget_h_
>+#ifndef nsIDOMEventTarget_h_
>+#define nsIDOMEventTarget_h_
> 
> #include "nsIDOMEventTarget.h"
> 
> #if 0
> #include "nsEvent.h"
> 
> class nsIDOMEvent;
> class nsPresContext;
>@@ -53,36 +53,36 @@ class nsIDOMEventGroup;
> class nsIScriptContext;
> struct JSContext;
> 
> // 89292f3a-535d-4ba0-882a-10cff9e21bcc
> #define NS_PIDOMEVENTTARGET_IID \
>   { 0x89292f3a, 0x535d, 0x4ba0, \
>     { 0x88, 0x2a, 0x10, 0xcf, 0xf9, 0xe2, 0x1b, 0xcc } }
> 
>-class nsPIDOMEventTarget : public nsISupports
>+class nsIDOMEventTarget : public nsISupports
> {
> public:
>   NS_DECLARE_STATIC_IID_ACCESSOR(NS_PIDOMEVENTTARGET_IID)

I don't understand this at all.
Perhaps some other patch removes this file



>-GetDOMEventTarget(nsPIDOMEventTarget* aTarget,
>+GetDOMEventTarget(nsIDOMEventTarget* aTarget,
>                   nsIDOMEventTarget** aDOMTarget)
> {
>-  nsPIDOMEventTarget* realTarget =
>+  nsIDOMEventTarget* realTarget =
>     aTarget ? aTarget->GetTargetForDOMEvent() : aTarget;
>   if (realTarget) {
>     return CallQueryInterface(realTarget, aDOMTarget);
Since  this QI shows up in the profiles, could you file a followup to get rid of it.


> NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(nsDOMEventTargetHelper)
>-  NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsPIDOMEventTarget)
>+  NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsIDOMEventTarget)
>   NS_INTERFACE_MAP_ENTRY(nsIDOMEventTarget)
> NS_INTERFACE_MAP_END
2 entries for nsIDOMEventTarget



> #define nsDOMEventTargetHelper_h_
> 
> #include "nsCOMPtr.h"
> #include "nsAutoPtr.h"
>-#include "nsPIDOMEventTarget.h"
>+#include "nsIDOMEventTarget.h"
> #include "nsIDOMEventListener.h"
> #include "nsIDOMEventTarget.h"
included twice


>   static void ResetMaxEtciCount()
>   {
>     NS_ASSERTION(!sCurrentEtciCount, "Wrong time to call ResetMaxEtciCount()!");
>     sMaxEtciCount = 0;
>   }
> 
>-  nsCOMPtr<nsPIDOMEventTarget>      mTarget;
>+  nsCOMPtr<nsIDOMEventTarget>      mTarget;
align properly


>   // Event retargeting must happen whenever mNewTarget is non-null.
>-  nsCOMPtr<nsPIDOMEventTarget>      mNewTarget;
>+  nsCOMPtr<nsIDOMEventTarget>      mNewTarget;
ditto



r+ if some other bug explain the nsPIDOMEventTarget.h changes.
Comment 38 Olli Pettay [:smaug] 2011-06-03 03:54:06 PDT
Comment on attachment 536021 [details] [diff] [review]
Part 6: Fixups after search'n'replace

> static nsresult
> GetDOMEventTarget(nsIDOMEventTarget* aTarget,
>                   nsIDOMEventTarget** aDOMTarget)
> {
>   nsIDOMEventTarget* realTarget =
>     aTarget ? aTarget->GetTargetForDOMEvent() : aTarget;
>-  if (realTarget) {
>-    return CallQueryInterface(realTarget, aDOMTarget);
>-  }
> 
>-  *aDOMTarget = nsnull;
>+  NS_IF_ADDREF(*aDOMTarget = aTarget);
>+
You want NS_IF_ADDREF(*aDOMTarget = realTarget);
Comment 39 Olli Pettay [:smaug] 2011-06-03 04:16:16 PDT
Comment on attachment 536023 [details] [diff] [review]
Part 7: DeCOMtaminate nsEventListenerManager

>-nsIEventListenerManager*
>+    NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NATIVE_PTR(entry->mListenerManager,
>+                                                 nsEventListenerManager,
>+                                  "[via hash] mListenerManager")
Strange indentation

> nsINode::RemoveEventListener(const nsAString& aType,
>                              nsIDOMEventListener* aListener,
>                              PRBool aUseCapture)
> {
>-  nsCOMPtr<nsIDOMEventTarget> event_target =
>-    do_QueryInterface(GetListenerManager(PR_TRUE));
>-  NS_ENSURE_STATE(event_target);
>-
>-  return event_target->RemoveEventListener(aType, aListener, aUseCapture);
>+  nsEventListenerManager* elm = GetListenerManager(PR_TRUE);
PR_FALSE

>-class nsEventListenerManager : public nsIEventListenerManager,
>-                               public nsIDOMEventTarget,
>-                               public nsIDOM3EventTarget
>+class nsEventListenerManager
> {
> 
> public:
>-  nsEventListenerManager();
>+  nsEventListenerManager(nsISupports* aTarget);
>   virtual ~nsEventListenerManager();
> 
>-  NS_DECL_CYCLE_COLLECTING_ISUPPORTS
>+  NS_INLINE_DECL_REFCOUNTING(nsEventListenerManager)
>+
>+  NS_DECL_CYCLE_COLLECTION_NATIVE_CLASS(nsEventListenerManager)
>+
>+  NS_IMETHOD RemoveEventListener(const nsAString& aType,
>+                                 nsIDOMEventListener* aListener,
>+                                 PRBool aUseCapture);
>+  NS_IMETHOD DispatchEvent(nsIDOMEvent* aEvent, PRBool *_retval);
> 
>   /**
>   * Sets events listeners of all types. 
>   * @param an event listener
>   */
>   NS_IMETHOD AddEventListenerByIID(nsIDOMEventListener *aListener,
>                                    const nsIID& aIID, PRInt32 aFlags);
>   NS_IMETHOD RemoveEventListenerByIID(nsIDOMEventListener *aListener,
>@@ -151,48 +161,66 @@ public:
>                                nsIDOMEvent** aDOMEvent,
>                                nsIDOMEventTarget* aCurrentTarget,
>                                PRUint32 aFlags,
>                                nsEventStatus* aEventStatus,
>                                nsCxPusher* aPusher);
> 
>   NS_IMETHOD Disconnect();
> 
>-  NS_IMETHOD SetListenerTarget(nsISupports* aTarget);
>-
>   NS_IMETHOD HasMutationListeners(PRBool* aListener);
> 
>   NS_IMETHOD GetSystemEventGroupLM(nsIDOMEventGroup** aGroup);
> 
>   virtual PRBool HasUnloadListeners();
> 
>   virtual PRUint32 MutationListenerBits();
> 
>   virtual PRBool HasListenersFor(const nsAString& aEventName);
> 
>   virtual PRBool HasListeners();
> 
>   virtual nsresult GetListenerInfo(nsCOMArray<nsIEventListenerInfo>* aList);
> 
>   static PRUint32 GetIdentifierForEvent(nsIAtom* aEvent);
I assume these are all virtual so that external code can use the methods.
I believe there are addons which use for example HasListenersFor


> protected:
>+  PRUint32 mMayHavePaintEventListener : 1;
>+  PRUint32 mMayHaveMutationListeners : 1;
>+  PRUint32 mMayHaveCapturingListeners : 1;
>+  PRUint32 mMayHaveSystemGroupListeners : 1;
>+  PRUint32 mMayHaveAudioAvailableEventListener : 1;
>+  PRUint32 mMayHaveTouchEventListener : 1;
>+  PRUint32 mNoListenerForEvent : 26;
>+
Please put these close to other member variables.
Comment 40 Olli Pettay [:smaug] 2011-06-03 04:48:22 PDT
Comment on attachment 536025 [details] [diff] [review]
Part 8: Improve the nsEventListenerManager API


>+nsINode::DispatchEvent(nsIDOMEvent *aEvent, PRBool* _retval)
While you're here, please s/_retval/aRetVal/

>-  virtual PRBool HasListenersFor(const nsAString& aEventName);
>+  PRBool HasListenersFor(const nsAString& aEventName);
Addons need to be able to access this information, AFAIK.
Making this non-virtual would limit access to libxul-only, right?
If you could add a helper method to nsIEventListenerService, that
should be enough.


r+ if you add that helper method to nsIEventListenerService
Comment 41 Olli Pettay [:smaug] 2011-06-03 04:56:07 PDT
Comment on attachment 536027 [details] [diff] [review]
Part 10: Remove nsIDOM3EventTarget/nsIDOMEventGroup

nsIDOM3EventTarget is such a good example why very early drafts shouldn't
be implemented.
Comment 42 Jonas Sicking (:sicking) 2011-06-03 13:56:21 PDT
(In reply to comment #34)
> >   // nsIDOMEventTarget
> >-  NS_DECL_NSIDOMEVENTTARGET
> >+  NS_IMETHODIMP PreHandleEvent(nsEventChainPreVisitor& aVisitor);
> >+  NS_IMETHOD_(nsIEventListenerManager*)
> >+  GetListenerManager(PRBool aCreateIfNotFound);
> Add 2 spaces before GetListenerManager 

Done

> >+nsINode::RemoveEventListener(const nsAString& aType,
> >+                             nsIDOMEventListener* aListener,
> >+                             PRBool aUseCapture)
> >+{
> >+  nsCOMPtr<nsIDOMEventTarget> event_target =
> >+    do_QueryInterface(GetListenerManager(PR_TRUE));
> PR_FALSE
> 
> 
> >+  NS_ENSURE_STATE(event_target);
> And then
> if (!event_target) {
>   return NS_OK;
> }

This is fixed in part 8.

> >+nsINode::PreHandleEvent(nsEventChainPreVisitor& aVisitor)
> >+{
> >+  // This is only here so that we can use the NS_DECL_NSIDOMTARGET macro
> >+  NS_ERROR("Override PreHandleEvent in subclasses");
> This could be runtime abort.

Done
Comment 43 Jonas Sicking (:sicking) 2011-06-03 14:36:21 PDT
(In reply to comment #37)
> Comment on attachment 536020 [details] [diff] [review] [review]
> Part 5: Search'n'replace nsPIDOMEventTarget to nsIDOMEventTarget

All these comments are fixed in the fixups in part 6.

> > NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(nsDOMEventTargetHelper)
> >-  NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsPIDOMEventTarget)
> >+  NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsIDOMEventTarget)
> >   NS_INTERFACE_MAP_ENTRY(nsIDOMEventTarget)
> > NS_INTERFACE_MAP_END
> 2 entries for nsIDOMEventTarget

Except this one. This isn't actually two entries for nsIDOMEventTarget, but rather one for nsISupports and one for nsIDOMEventTarget. But the _AMBIGUOUS is removed in the fixup patch.

Sorry, I had hoped that doing the search'n'replace separate from the fixups would have made reviewing easier, but it appears to have caused more confusion :(
Comment 44 Jonas Sicking (:sicking) 2011-06-03 14:41:16 PDT
(In reply to comment #38)
> Comment on attachment 536021 [details] [diff] [review] [review]
> Part 6: Fixups after search'n'replace
..
> You want NS_IF_ADDREF(*aDOMTarget = realTarget);

Yay!! Good catch! I wonder if this is what's causing the regression...
Comment 45 Jonas Sicking (:sicking) 2011-06-03 14:56:20 PDT
(In reply to comment #39)
> Comment on attachment 536023 [details] [diff] [review] [review]
> Part 7: DeCOMtaminate nsEventListenerManager
> 
> >-nsIEventListenerManager*
> >+    NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NATIVE_PTR(entry->mListenerManager,
> >+                                                 nsEventListenerManager,
> >+                                  "[via hash] mListenerManager")
> Strange indentation

I couldn't figure out a nice way to indent this so I just used the pattern that we use elsewhere for this macro :(

> > nsINode::RemoveEventListener(const nsAString& aType,
> >                              nsIDOMEventListener* aListener,
> >                              PRBool aUseCapture)
> > {
> >-  nsCOMPtr<nsIDOMEventTarget> event_target =
> >-    do_QueryInterface(GetListenerManager(PR_TRUE));
> >-  NS_ENSURE_STATE(event_target);
> >-
> >-  return event_target->RemoveEventListener(aType, aListener, aUseCapture);
> >+  nsEventListenerManager* elm = GetListenerManager(PR_TRUE);
> PR_FALSE

Hehe, this one keeps coming up :) Fixed in part 8.


> > protected:
> >+  PRUint32 mMayHavePaintEventListener : 1;
> >+  PRUint32 mMayHaveMutationListeners : 1;
> >+  PRUint32 mMayHaveCapturingListeners : 1;
> >+  PRUint32 mMayHaveSystemGroupListeners : 1;
> >+  PRUint32 mMayHaveAudioAvailableEventListener : 1;
> >+  PRUint32 mMayHaveTouchEventListener : 1;
> >+  PRUint32 mNoListenerForEvent : 26;
> >+
> Please put these close to other member variables.

Done.
Comment 46 Jonas Sicking (:sicking) 2011-06-03 15:10:59 PDT
(In reply to comment #40)
> Comment on attachment 536025 [details] [diff] [review] [review]
> Part 8: Improve the nsEventListenerManager API
> 
> 
> >+nsINode::DispatchEvent(nsIDOMEvent *aEvent, PRBool* _retval)
> While you're here, please s/_retval/aRetVal/

Done (fixed this when you commented on it earlier)

> >-  virtual PRBool HasListenersFor(const nsAString& aEventName);
> >+  PRBool HasListenersFor(const nsAString& aEventName);
> Addons need to be able to access this information, AFAIK.
> Making this non-virtual would limit access to libxul-only, right?
> If you could add a helper method to nsIEventListenerService, that
> should be enough.
> 
> 
> r+ if you add that helper method to nsIEventListenerService

Done
Comment 47 Jonas Sicking (:sicking) 2011-06-03 16:34:36 PDT
(In reply to comment #33)
> I think you forgot to remove NS_DOMEVENTGROUP_CID from nsContentCID.h.

Done
Comment 48 Jonas Sicking (:sicking) 2011-06-03 19:49:14 PDT
Comment on attachment 536026 [details] [diff] [review]
Part 9: Don't use EventGroups for system-group

Review of attachment 536026 [details] [diff] [review]:
-----------------------------------------------------------------

::: editor/libeditor/text/tests/test_bug569988.html
@@ -61,1 @@
>    gPromptInput.removeEventListener("focus", onPromptFocus, false);

I located the test failure.

This enablePrivilege call can't be removed since gPromptInput is a chrome object and so the call to removeEventListener fails without it.

So no code problems!

I've fixed it locally as well as removed the enablePrivilege call a bit down which *can* be removed.
Comment 49 Jonas Sicking (:sicking) 2011-06-05 04:17:20 PDT
Created attachment 537441 [details] [diff] [review]
Part 4: Make windows happy.

This builds on the patch in bug 661984. I also changed the Part 3 patch so that it doesn't use the NS_IMETHOD macros when moving the methods to nsINode.
Comment 50 Olli Pettay [:smaug] 2011-06-06 10:41:43 PDT
Comment on attachment 537441 [details] [diff] [review]
Part 4: Make windows happy.

>+++ b/dom/interfaces/events/nsIDOMEventTarget.idl
>@@ -162,58 +162,61 @@ interface nsIDOMEventTarget : nsISupport

Just to be safe, update uuid.

Also, do you need to update the implementations of the interface?
Comment 52 Boris Zbarsky [:bz] 2011-07-22 10:29:41 PDT
So shouldn't the nsIEventListenerManager deCOM have moved the comments over to nsEventListenerManager?
Comment 53 Olli Pettay [:smaug] 2011-07-23 06:23:06 PDT
Yes!
Comment 54 Boris Zbarsky [:bz] 2011-08-04 11:29:37 PDT
I guess I'll do that in bug 659350....
Comment 55 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2012-08-12 21:09:41 PDT
I added the document about the change in nsIEventListenerService. Please check it.
https://developer.mozilla.org/en-US/docs/XPCOM_Interface_Reference/nsIEventListenerService

Thanks.

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