912874 - New API to enumerate mutation observers

Status: RESOLVED FIXED

(Core :: DOM: Events, defect)

Description

[Jan Honza Odvarko [:Honza] (always need-info? me)]

DevTools (e.g Firebug) could use an API to enumerate mutation observers for a node from chrome. 

Ideally, the new API would give the following information for every mutation observer:

1) The mutation observer JS callback function
2) options (MutationObserverInit)

Honza

Comment 1

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

So you want a list of MutationObservers which is explicitly observing some node?
What about transient observers? I guess those aren't needed.

I guess we could add

dictionary MutationObservingInfo : MutationObserverInit
{
  Node observedNode;
  MutationCallback callback;
}
[ChromeOnly] readonly attribute sequence<MutationObservingInfo> mutationObservers;
to Node, assuming we don't need anything for C++ addons.

Sounds ok?

Comment 2

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

Or perhaps Node should just have
[ChromeOnly] sequence<MutationObserver> getAttachedMutationObservers();

And MutationObserver
[ChromeOnly] sequence<MutationObservingInfo> getObservingInfo();
and [ChromeOnly] readonly attribute MutationCallback callback;

Comment 3

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to Olli Pettay [:smaug] from comment #2)
> Or perhaps Node should just have
> [ChromeOnly] sequence<MutationObserver> getAttachedMutationObservers();
> 
> And MutationObserver
> [ChromeOnly] sequence<MutationObservingInfo> getObservingInfo();
> and [ChromeOnly] readonly attribute MutationCallback callback;
Yep, I like this.


Honza

Comment 4

[Jan Honza Odvarko [:Honza] (always need-info? me)]

What about dynamic tracking of added/removed observers?

I can imagine a tool that needs to update displayed list of observers when new one is added or existing one removed. Should this be done/discussed in another bug report?

Honza

Comment 5

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

added observers would be doable pretty easily (but would need some service probably), 
removed is hard, since observers may die when the node itself die.
(new MutationObserver(function() {})).observe(document.createElement("foo"), {childList: true})
keeps both the created element and mutation observer alive until cycle collector has run.
And we don't want to notify any scriptable service during cycle collection's unlink.

Comment 6

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

Or would you need notification only about explicit removal? Hmm, though, then the API would be just inconsistent.

Comment 7

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to Olli Pettay [:smaug] from comment #6)
> Or would you need notification only about explicit removal?
You are thinking about observer.disconnect(), correct?

> Hmm, though, then the API would be just inconsistent.
Right

I don't have any better idea than only hook explicit removals, but we can also wait till some tools really need it. For now I am thinking about a new Command Line command (in Firebug): getMutationObservers(node)

...similar to what is already for event listeners: getEventListeners()
http://www.softwareishard.com/blog/planet-mozilla/firebug-tip-geteventlisteners-command/

Honza

Comment 8

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

Patch coming

Comment 9

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

Attachment: patch + ad hoc tests

Removes IID for nsMutationReceiver since it isn't needed, but adds one for
nsDOMMutationObserver.

I think sicking is on vacation + work week, so perhaps you peterv could review this. Should be rather simple.

https://tbpl.mozilla.org/?tree=Try&rev=d8ca022e34aa

Comment 10

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Wow, that was quick!

Any chance to get a try build?
(Win, 32 bit)

Honza

Comment 11

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

https://tbpl.mozilla.org/?tree=Try&rev=d8ca022e34aa gives only debug builds

Comment 12

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

Let me push a new one with for opt win32

Comment 13

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

https://tbpl.mozilla.org/?tree=Try&rev=6bda2ae2bcfc

Comment 14

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to Olli Pettay [:smaug] from comment #13)
> https://tbpl.mozilla.org/?tree=Try&rev=6bda2ae2bcfc
Thanks!

I have tested the API, couple of questions:

1) getBoundMutationObservers returns list of observers for given element, but if an observer is registered for a parent element (with config.subtree=true) it isn't listed. Example:

<div id="content">
    <div id="testElement">Observe Me!</div>
</div>

If I register an observer for the 'content' element and call getBoundMutationObservers() on the 'testElement' the return value is empty. I think it should return all observers available in the parent chain. Or at least those, which have subtree=true...?

2) getObservingInfo returns the info, but why it's a list? There can be more info structures for one observer?

Honza

Comment 15

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

(In reply to Jan Honza Odvarko from comment #14)
> 1) getBoundMutationObservers returns list of observers for given element,
> but if an observer is registered for a parent element (with
> config.subtree=true) it isn't listed. Example:
> 
> <div id="content">
>     <div id="testElement">Observe Me!</div>
> </div>
> 
> If I register an observer for the 'content' element and call
> getBoundMutationObservers() on the 'testElement' the return value is empty.
> I think it should return all observers available in the parent chain. Or at
> least those, which have subtree=true...?
definitely not without subtree true, since then changes to testElement don't have anything to do with
obverser registered on 'content'
And to keep the API simple, I'd prefer to not rely on subtree=true - or otherwise we should probably also
handle transient observers somehow. 


> 2) getObservingInfo returns the info, but why it's a list? There can be more
> info structures for one observer?
Yes. One per node passed to observe()

Comment 16

[Jan Honza Odvarko [:Honza] (always need-info? me)]

OK, all make sense to me.

Honzas

Comment 17

[Peter Van der Beken [:peterv]]

Comment on attachment 800273 [details] [diff] [review]
patch + ad hoc tests

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

::: dom/webidl/MutationObserver.webidl
@@ +28,5 @@
>    void disconnect();
>    sequence<MutationRecord> takeRecords();
> +
> +  [ChromeOnly]
> +  sequence<MutationObservingInfo?> getObservingInfo();

Why is this nullable? You never set it to null. Please change this to sequence<MutationObservingInfo>.

@@ +47,5 @@
>  };
> +
> +dictionary MutationObservingInfo : MutationObserverInit
> +{
> +  Node? observedNode = null;

Again, why nullable? Change to just Node.

Comment 18

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

The reason is that our codegen can't handle those cases without nullable, and I didn't
want to fight with codegen just to get this done.

Comment 19

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

Ah, maybe it can deal with always non-null observedNode

Comment 20

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

Yes, that looks possible, but sequence<MutationObservingInfo> getObservingInfo(); isn't supported atm.

Comment 21

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

Node observedNode is also a bit ugly to handle on C++ side, but I do it anyway.

Comment 22

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

Actually, handling observedNode in C++ is just too ugly if there isn't ?, so I'll keep the ?.

Comment 23

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/050debae43a6

Comment 24

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/050debae43a6

Comment 25

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Olli, I am experiencing one problem. I need to find the script an observer.mutationCallback (a JS function) is defined in.

The |observer| is what I am getting from target.getBoundMutationObservers();

I am using JSD service:

var jsd = Cc["@mozilla.org/js/jsd/debugger-service;1"].getService(Ci.jsdIDebuggerService);
var wrapped = jsd.wrapValue(observer.mutationCallback);
var script = wrapped.script;

But I am getting an exception when executing the |jsd.wrapValue| API:
Component returned failure code: 0x80040111 (NS_ERROR_NOT_AVAILABLE) [jsdIDebuggerService.wrapValue]

Am I doing something wrong?
Or should I report a bug?

Honza

Comment 26

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

Hmm, I don't know where that error is coming. Do you get a valid value from observer.mutationCallback? I assume yes.
If I read the code correctly, http://mxr.mozilla.org/mozilla-central/source/js/jsd/jsd_xpc.cpp#2939 throws http://mxr.mozilla.org/mozilla-central/source/js/jsd/jsd_xpc.cpp#59
I'm not familiar with JSD, so I don't know what that !mCx means. Are you failing to initialize JSD somehow?

Comment 27

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to Olli Pettay [:smaug] from comment #26)
> Hmm, I don't know where that error is coming. Do you get a valid value from
> observer.mutationCallback? I assume yes.
> If I read the code correctly,
> http://mxr.mozilla.org/mozilla-central/source/js/jsd/jsd_xpc.cpp#2939 throws
> http://mxr.mozilla.org/mozilla-central/source/js/jsd/jsd_xpc.cpp#59
> I'm not familiar with JSD, so I don't know what that !mCx means. Are you
> failing to initialize JSD somehow?
The problem was on my side (and actually related to initialization, good call)
Thanks
Honza