JM: mochitest-plain failure on modules/plugin/test/test_npruntime_npnevaluate.html

RESOLVED FIXED

Status

()

RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: dmandelin, Assigned: dmandelin)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(1 attachment)

(Assignee)

Description

8 years ago
NEXT ERROR 81377 ERROR TEST-UNEXPECTED-FAIL | /tests/modules/plugin/test/test_npruntime_npnevaluate.html | npnEvaluateTest returned an unexpected value - got "null", expected "3"
NEXT ERROR 81393 ERROR TEST-UNEXPECTED-FAIL | /tests/modules/plugin/test/test_npruntime_npnevaluate.html | npnEvaluateTest returned an unexpected value - got "{\"h\":null,\"s\":null,\"l\":null}", expected "{\"a\":\"1\",\"b\":\"2\"}"
(Assignee)

Comment 1

8 years ago
Just spoke with jst and brendan about this. The problem is that _evaluate is passing an outer window to EvaluateStringWithValue as the obj (i.e., scope chain), which forwards that to EvaluateUCScript. Apparently that is considered a bug already.

This doesn't cause this Mochitest to fail in TM/m-c, but the bug gets exposed in JM. The problem is that the script-compile step creates globals on the scope chain (outer window here), while Execute runs on the innerized scope chain.

Part 1 of the bugfix is to innerize in _evaluate. But it seems we should try to detect and prevent this sort of thing going forward. Any opinion on how to do that? jst suggested throwing an exception from EvaluateUCScript if the scope chain is an outer window, detected by trying to innerize the scope chain.
(Assignee)

Comment 2

8 years ago
Created attachment 464253 [details] [diff] [review]
Patch

I'm going to go ahead and land this patch to moo so we can cycle the tests. But we might as well have it properly reviewed now and landed on TM as well.
Attachment #464253 - Flags: review?(mrbkap)
Comment on attachment 464253 [details] [diff] [review]
Patch

This is fine, but IMO we should be doing this inside the API boundary. I'll file a new bug on that though.
Attachment #464253 - Flags: review?(mrbkap) → review+
(Assignee)

Comment 5

8 years ago
http://hg.mozilla.org/tracemonkey/rev/1140fcfc807a
Whiteboard: fixed-in-tracemonkey
(Assignee)

Updated

8 years ago
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.