Closed Bug 615065 Opened 11 years ago Closed 11 years ago

NS_ERROR_NOT_IMPLEMENTED [nsIDOM3Document.documentURI] if methodjit is enabled

Categories

(Core :: JavaScript Engine, defect)

x86
Windows 7
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Tracking Status
blocking2.0 --- betaN+

People

(Reporter: spammaaja, Assigned: dmandelin)

References

()

Details

(Keywords: regression, Whiteboard: fixed-in-tracemonkey)

Attachments

(1 file, 1 obsolete file)

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).
blocking2.0: --- → ?
Version: unspecified → Trunk
Status: UNCONFIRMED → NEW
Ever confirmed: true
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
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.  :(
Assignee: nobody → general
Component: DOM: Core & HTML → JavaScript Engine
QA Contact: general → general
Keywords: regression
(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).
alert(document.documentURI) works just fine.  It'd be nice to get a reduced test case here.
Keywords: testcase-wanted
> 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...
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?
> or was that the first TM build with JM?

That was the first TM build with JM.
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...
blocking2.0: ? → betaN+
Assignee: general → 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.
Attached file Suspected reduced test case (obsolete) —
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
Depends on: 620838
Attached patch PatchSplinter Review
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.
Attachment #499166 - Attachment is obsolete: true
Attachment #499212 - Flags: review?(jwalden+bmo)
Can we add a jsreftest for this if we can't do a shell test?
Attachment #499212 - Flags: review?(jwalden+bmo) → review+
http://hg.mozilla.org/tracemonkey/rev/f3dae72f2b8e
Status: NEW → ASSIGNED
Whiteboard: fixed-in-tracemonkey
Duplicate of this bug: 621377
dmandelin, bug 621377 has a testcase half a step from being made into a shell test case.
Duplicate of this bug: 523234
http://hg.mozilla.org/mozilla-central/rev/f3dae72f2b8e
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.