861807 - DOM nodes are substituted by copies in some circumstances

Status: RESOLVED DUPLICATE of bug 876098

(Core :: JavaScript Engine, defect)

Description

[Marijn Haverbeke]

(This appears to be an optimization that's too aggressive, but it's extremely subtle, so my understanding of the mechanism at work might be completely wrong.)

I have a library (CodeMirror) which adds a property to some DOM nodes it creates. This property sometimes vanishes in FF nightly. But there's a kind of observer effect, in that if I save a (2nd, not in the DOM itself) reference to the DOM node that I added the property to, the problem goes away for that node. Thus, I was unable to verify whether an actual new node object is created or not. But I guess there is -- randomly deleting my property from an existing object would be very strange.

The most obvious way to see this happen is to:

 - Open http://marijnhaverbeke.nl/upload/ff_console_issue.html in FF 23.0a1 (2013-04-14)
 - Open the ctrl-shift-k web console
 - DO NOT open Firebug. Firebug also makes the problem vanish.
 - Watch the console for a few seconds. There's a `setInterval` reporting on whether our node still has its property. For me, it still has it at this point.
 - Now call `report()` from the console. For me, it now reports that the property is undefined. (In fact, any activity at the console seems to trigger it).
 - Further output from the `setInterval` call now also reports undefined.

My guess is that some optimization triggered by a bunch of different factors 'collects' DOM nodes that it feels can be easily restored (this also doesn't happen if the node is scrolled into view). That optimization should probably be turned off for nodes that have custom properties.

(This is a spin-off from CodeMirror https://github.com/marijnh/CodeMirror/issues/1455 )

Comment 1

[Tim Taubert [:ttaubert] (inactive)]

Can't reproduce with the given Nightly version on Linux64. Did you try in safe-mode or with all add-ons disabled? I guess that Firebug might interfere even if it doesn't seem to be active.

Comment 2

[Tim Taubert [:ttaubert] (inactive)]

I can reproduce it now if I close and re-open the Web Console while the script is running.

Comment 3

[Tim Taubert [:ttaubert] (inactive)]

So I saved |editor.display.lineDiv.lastChild| while .lineObj is defined to some variable. I played around with the Web Console until .lineObj became undefined. Now |editor.display.lineDiv.lastChild| does not match the DOMNode I backed up before. They're both HTMLPreElements containing ' ' but they're not the same.

Comment 4

[Marijn Haverbeke]

An obvious suspicion here is that the CodeMirror library might be doing this damage itself (replacing the node with a bad one). It's a big blob of code full of event handler and timeouts. I made an attempt to reproduce this with just a small piece of code that tries to do something similar to what CodeMirror does, but that did not produce the same effect.

Still, as the author of CodeMirror I know the code well and am positive that it's not messing with that node unless a redraw happens, which will not happen when the page is idle and the user is screwing around inspecting thing in the console. You can, to a certain degree, add console.log output to the library to verify this, though there were some strange effects where logging something, even if it's not the DOM node in question, also made the bug vanish.

Comment 5

[Tim Taubert [:ttaubert] (inactive)]

Despite the GitHub issue saying there is a report in Fx 20, I could only reproduce it in the current Nightly. Using mozregression I could find this range:

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=97cfc16ba5dc&tochange=c232bec6974d

This includes the merge of the new baseline compiler which could very likely cause bugs like this.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Hmm.  Firefox 20 is when we first switched <div> to WebIDL bindings... but that really should not have broken wrapper preservation.

As for baseline...  Is it possible baseline doesn't call addProperty hooks in some cases?  Or that ion doesn't?

Comment 7

[Tim Taubert [:ttaubert] (inactive)]

Bisecting leads me into the BC merge:

http://hg.mozilla.org/mozilla-central/rev/79542849f3f3

Not sure if bisecting further makes any sense because it's probably just a regression introduced by the whole new BC.

Comment 8

[Tim Taubert [:ttaubert] (inactive)]

I can't reproduce the missing property with javascript.options.ion.content=false.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Depending on how motivated you feel, you could bisect on ionmonkey.

Comment 10

[Tim Taubert [:ttaubert] (inactive)]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #9)
> Depending on how motivated you feel, you could bisect on ionmonkey.

I tried to but somehow it got stuck on one changeset and didn't really want to bisect any further :|

Comment 11

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

CCing some baseline compiler people.

Comment 12

[Kannan Vijayan [:djvj]]

I had a heck of a time replicating this on tip.  Finally just downloaded the specific nightly mentioned in comment 1, and was able to reproduce it.

Some additional info:

Problem still reproduces when Baseline is turned off, but not when Ion is turned off.

When the |lineObj| goes from an object to undefined, it's because _all the child nodes_ of |editor.display.lineDiv| have changed, not just the |lastChild|.  Somehow something is triggering replacement of the entire chunk of DOM under |lineDiv|.

Also, it's almost definitely a case of the DOM nodes getting replaced instead of "ion optimizations reading the wrong thing", because |report()| does not run often enough to get ion compiled.  The problem shows up within 4 or 5 runs of |report|, and there's no way it's getting ion compiled at that time (it won't even be baseline compiled at that time).


I'm considering whether the enabling/disabling of ion leading to this is more of a red herring than the actual cause of the behaviour.

Comment 13

[Kannan Vijayan [:djvj]]

This is caused by the same issue as bug 876098.

This particular bug only reproduces on revisions close to Apr 14 or so, but reverting to that revision shows the issue, and applying the 876098 patch on top of that resolves it.

Comment 14

[Bogdan Maris, Desktop Test Engineering]

Mozilla/5.0 (X11; Linux x86_64; rv:23.0) Gecko/20100101 Firefox/23.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:23.0) Gecko/20100101 Firefox/23.0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:23.0) Gecko/20100101 Firefox/23.0

Loading the test case from bug 876098 I get pass on all platforms using Firefox 23 RC (buildID: 20130730113002) so I can call this verified fixed.