776760 - ecma_5/extensions/cross-global-getPrototypeOf.js always fails

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jason Orendorff [:jorendorff]]

It reduces to this:

var g = newGlobal();
g.evaluate("var a = { a: 1 }; " +
           "var obj = Object.create(a);");
assertEq(Object.getPrototypeOf(g.obj), g.a);
g.evaluate("var b = { b: 1 }; " +
           "obj.__proto__ = b;");
assertEq(Object.getPrototypeOf(g.obj), g.b);

Weirdly, if you comment out the first assert, which ought not to have any side effects, the second one passes.

I'll flip Terrence for this one.

(Note that we *could* just add "fails" to the magic comment at the top of the file and wait for Waldo to get back.)

Comment 1

[David Mandelin [:dmandelin]]

(In reply to Jason Orendorff [:jorendorff] from comment #0)
> It reduces to this:
> 
> var g = newGlobal();
> g.evaluate("var a = { a: 1 }; " +
>            "var obj = Object.create(a);");
> assertEq(Object.getPrototypeOf(g.obj), g.a);
> g.evaluate("var b = { b: 1 }; " +
>            "obj.__proto__ = b;");
> assertEq(Object.getPrototypeOf(g.obj), g.b);
> 
> Weirdly, if you comment out the first assert, which ought not to have any
> side effects, the second one passes.
> 
> I'll flip Terrence for this one.
> 
> (Note that we *could* just add "fails" to the magic comment at the top of
> the file and wait for Waldo to get back.)

Do we know which patch caused it? We should just back out if possible.

Comment 2

[Steve Fink [:sfink] [:s:]]

It comes from bug 770344, which is also where the test was added.

Comment 3

[Luke Wagner [:luke]]

Attachment: fix

It looks like the failing test was actually added by bug 770344.  The Math.sin() call right before the failing assertEq would seem to indicate that Waldo was working on this and the fix was somehow accidentally lost.  I actually remember talking to Waldo about this problem and I think this patch is the intended solution (at least for now; hopefully we can remove the scripted proxy handler disjunct after the direct proxy work).

Comment 4

[Jason Orendorff [:jorendorff]]

Comment on attachment 645832 [details] [diff] [review]
fix

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

Looks reasonable to me.

(It still doesn't make any sense to me for Object.prototype to call the __proto__ getter. Instead, both should call the same C++ function. But yeah, let's land this.)

Comment 5

[Luke Wagner [:luke]]

Well, it would seem that my patch causes orange:
  https://tbpl.mozilla.org/?tree=Try&rev=137102959e2d

Since this bug existed before bug 770344 (filed as bug 764307), I think we should just remove the offending assert.

Comment 6

[Luke Wagner [:luke]]

Removed:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3238a5e6e0d8

Comment 7

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/3238a5e6e0d8