615065 - NS_ERROR_NOT_IMPLEMENTED [nsIDOM3Document.documentURI] if methodjit is enabled

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[JK]

User-Agent:       Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b7) Gecko/20100101 Firefox/4.0b7
Build Identifier: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b8pre) Gecko/20101126 Firefox/4.0b8pre

The map in the URL (see above) doesn't show the dots if methodjit is enabled.

Error: uncaught exception: [Exception... "Component returned failure code: 0x80004001 (NS_ERROR_NOT_IMPLEMENTED) [nsIDOM3Document.documentURI]"  nsresult: "0x80004001 (NS_ERROR_NOT_IMPLEMENTED)"  location: "JS frame :: http://wals.info/tg_js/MochiKit.js :: <TOP_LEVEL> :: line 105"  data: no]

Reproducible: Always

Actual Results:  
Dots are not shown, except one (the tooltip of that dot is Aghul: Locational).


Expected Results:  
All dots should be visible (compare the results when javascript.options.methodjit.content is false).

Comment 1

[Alice0775 White]

Regression window(cached hourly):
Works:
http://hg.mozilla.org/mozilla-central/rev/16eac4b8b8e0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b6pre) Gecko/20100911 Firefox/4.0b6pre ID:20100911133215
Fails:
http://hg.mozilla.org/mozilla-central/rev/f1bd314e64ac
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b6pre) Gecko/20100911 Firefox/4.0b6pre ID:20100911134652
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=16eac4b8b8e0&tochange=f1bd314e64ac

Comment 2

[Boris Zbarsky [:bzbarsky]]

That's just the general mjit landing.  Given that the documentURI DOM getter is very much implemented, it sounds like the JS engine and/or XPConnect are getting confused here...

A regression range on the TM branch (or maybe even the JM branch) would be needed to really try to pin this down in time.  :(

Comment 3

[JK]

(In reply to comment #2)
> That's just the general mjit landing.  Given that the documentURI DOM getter is
> very much implemented, it sounds like the JS engine and/or XPConnect are
> getting confused here...
> 
> A regression range on the TM branch (or maybe even the JM branch) would be
> needed to really try to pin this down in time.  :(

Where are the JM branch nightlies?

On the TM branch the problem seems to start when the javascript.options.methodjit.content pref appears in about:config (2010-09-01-05-tracemonkey).

Comment 4

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

alert(document.documentURI) works just fine.  It'd be nice to get a reduced test case here.

Comment 5

[Boris Zbarsky [:bzbarsky]]

> Where are the JM branch nightlies?

There might not be any binary ones.  :(

Thanks for narrowing down the TM range; I'll take a look at this from here...

Comment 6

[JK]

Is there a secret way to enable JM in the TM branch before 2010-09-01-05-tracemonkey, or was that the first TM build with JM?

Comment 7

[Boris Zbarsky [:bzbarsky]]

> or was that the first TM build with JM?

That was the first TM build with JM.

Comment 8

[Boris Zbarsky [:bzbarsky]]

OK, the immediate reason for the exception is that we end up calling the documentURI _setter_.  And that's not implemented, indeed.

This setter does not get called when mjit is disabled.

The top of the JS stack when the setter is called is:

0 anonymous(obj = [object XMLDocument @ 0x2b259ef0 (native @ 0x171f3a00)], _17 = [object HTMLDocument @ 0x277f2ed0 (native @ 0x289c4400)]) ["http://wals.info/tg_js/MochiKit.js":105]
    v = "http://wals.info/feature/117?s=20&v1=c00d&v2=cd00&v3=cf6f&v4=cff0&v5=c000&z4=2997&z5=2996&z1=2999&z2=3000&z3=2998&tg_format=map&lat=5.5&lng=152.58&z=2&t=m"
    k = "documentURI"
    o = [object XMLDocument @ 0x2b259ef0 (native @ 0x171f3a00)]
    i = 1
    this = [object Window @ 0x29314560 (native @ 0x2654ad30)]
1 anonymous(_359 = [object Element @ 0x2b26fbb0 (native @ 0x2b25b060)], node = [object HTMLParagraphElement @ 0x2b43d738 (native @ 0x2b26e1f0)]) ["http://wals.info/tg_js/MochiKit.js":2820]
    _362 = undefined
    _361 = undefined
    v = [object XMLDocument @ 0x2b259ef0 (native @ 0x171f3a00)]
    k = "ownerDocument"
    _360 = [function]
    self = [MochiKit.DOM 1.3.1]
    elem = [object HTMLParagraphElement @ 0x2b43d738 (native @ 0x2b26e1f0)]
    this = [MochiKit.DOM 1.3.1]
2 anonymous(_366 = [object Element @ 0x2b26fbb0 (native @ 0x2b25b060)], name = "p", "http://wals.info/static/images/icons/c00d.png", "1", "Locational", 2999) ["http://wals.info/tg_js/MochiKit.js":2914]
    args = undefined
    m = [MochiKit.Base 1.3.1]
    self = [MochiKit.DOM 1.3.1]
    elem = [object HTMLParagraphElement @ 0x2b43d738 (native @ 0x2b26e1f0)]
    this = [object Window @ 0x29314560 (native @ 0x2654ad30)]

The relevant code in that topmost frame (which is for the function named "updatetree" is):

  for(var k in o){
    var v=o[k];
    if(typeof (_17[k])=="object"&&typeof (v)=="object"){
      arguments.callee(_17[k],v);
    }else{
      _17[k]=v;
    }
  }

In this case |v| is a string; |o| is an XMLDocument. _17 is an HTMLDocument, so we do the else clause for k === "documentURI" and lose.

We get into updatetree from a function called updateNodeAttributes, which, when given two nodes, for the case of "ownerDocument" will call updatetree on both documents.

I don't understand how JM is involved here.  :(  It looks like this code should always throw if we ever call updateNodeAttributes and pass it two nodes...

Comment 9

[David Mandelin [:dmandelin]]

I found a little bit more information. We are probably executing |updatetree| (in MochiKit.js) correctly. The bug is that we call it at all. I traced that all the way back to |createDOM|, which seems to get called only in the methodjit. I verified that by making that method immediately return, and the app still works with the methodjit off.

The app doesn't seem to try to call CreateDOM directly, instead it calls this:

  createDOMFunc:function(){
    var m=MochiKit.Base;
    return m.partial.apply(this,m.extend([MochiKit.DOM.createDOM],arguments));
  },

It's going to take more digging to figure out that difference.

Comment 10

[David Mandelin [:dmandelin]]

Attachment: Suspected reduced test case

This should print "local g" but it prints "global g" with or without -m.

This is extracted from code in [1] around 'GDownloadUrl(I, function (o, n) {'. The call to P calls the wrong function with the methodjit turned on. 

[1] http://wals.info/static/javascript/master-min.js

Comment 11

[David Mandelin [:dmandelin]]

Attachment: Patch

The suspected reduced test case is actually for bug 620838. That needs to be fixed for this, but there is another issue here, which is that we need to check for aliasing on the free-variable path. My attempts to produce a shell test case for that failed, because the JSOP_DEFFUN shape-changed the global (due to shadowing), so we don't use the MIC, which is required to trigger the bug. Anyway, this patch doesn't regress on our test suites and it fixes the linked site.

Comment 12

[Boris Zbarsky [:bzbarsky]]

Can we add a jsreftest for this if we can't do a shell test?

Comment 13

[David Mandelin [:dmandelin]]

http://hg.mozilla.org/tracemonkey/rev/f3dae72f2b8e

Comment 15

[Jeff Walden [:Waldo]]

dmandelin, bug 621377 has a testcase half a step from being made into a shell test case.

Comment 17

[Chris Leary [:cdleary] (not checking bugmail)]

http://hg.mozilla.org/mozilla-central/rev/f3dae72f2b8e