Bug 60391 – Media Stream no-interface-object test should not have ReferenceError message in expected results

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Adam Bergkvist]

The message for ReferenceError differs between JSC and V8 and having it in the expected results file requires unnecessary platform specific expected results.

[Adam Bergkvist]

Created attachment 92615 [details]
Proposed patch

[Darin Adler]

(In reply to comment #0)
> The message for ReferenceError differs between JSC and V8 and having it in the expected results file requires unnecessary platform specific expected results.

Does it have to differ? Can we make the messages the same?

[Adam Bergkvist]

(In reply to comment #2)
> (In reply to comment #0)
> > The message for ReferenceError differs between JSC and V8 and having it in the expected results file requires unnecessary platform specific expected results.
> 
> Does it have to differ? Can we make the messages the same?

I think it would be good to have the same message. The question is which message it should be? The V8 message seems to match the one you get in Firefox (aoeu is not defined), but perhaps the JSC message (Can't find variable: aoeu) is better since "is not defined" can be confused with undefined (an undefined value of a declared variable).

[Eric Seidel (no email)]

(In reply to comment #2)
> (In reply to comment #0)
> > The message for ReferenceError differs between JSC and V8 and having it in the expected results file requires unnecessary platform specific expected results.
> 
> Does it have to differ? Can we make the messages the same?

:(

[Adam Barth]

> Does it have to differ? Can we make the messages the same?

I would very much like to make the messages the same.  Historically when I've discussed this issue with the V8 team, they haven't been very receptive.  (I can dig up the bug numbers if you're interested.)  Some of the messages are generated by the bindings layer, which are easier to synchronize.  Unfortunately, I believe the ReferenceError is generated by the JS engine itself.

[Leandro Graciá Gil]

This should be fixed by now. Can you please confirm it? All the tests passed in mac using JSC last time I tried.

Thanks.

[Adam Bergkvist]

(In reply to comment #6)
> This should be fixed by now. Can you please confirm it? All the tests passed in mac using JSC last time I tried.
> 
> Thanks.

Still got the same problem with a build from this morning (r87680).

Here's the diff:

--- /tmp/layout-test-results/fast/dom/MediaStream/no-interface-object-expected.txt	2011-05-30 13:28:59.468002394 +0200
+++ /tmp/layout-test-results/fast/dom/MediaStream/no-interface-object-actual.txt	2011-05-30 13:28:59.468002394 +0200
@@ -4,13 +4,13 @@
 
 
 PASS typeof NavigatorUserMedia is "undefined"
-PASS NavigatorUserMedia.prototype threw exception ReferenceError: NavigatorUserMedia is not defined.
+PASS NavigatorUserMedia.prototype threw exception ReferenceError: Can't find variable: NavigatorUserMedia.
 PASS typeof NavigatorUserMediaError is "undefined"
-PASS NavigatorUserMediaError.prototype threw exception ReferenceError: NavigatorUserMediaError is not defined.
+PASS NavigatorUserMediaError.prototype threw exception ReferenceError: Can't find variable: NavigatorUserMediaError.
 PASS typeof NavigatorUserMediaSuccessCallback is "undefined"
-PASS NavigatorUserMediaSuccessCallback.prototype threw exception ReferenceError: NavigatorUserMediaSuccessCallback is not defined.
+PASS NavigatorUserMediaSuccessCallback.prototype threw exception ReferenceError: Can't find variable: NavigatorUserMediaSuccessCallback.
 PASS typeof NavigatorUserMediaErrorCallback is "undefined"
-PASS NavigatorUserMediaErrorCallback.prototype threw exception ReferenceError: NavigatorUserMediaErrorCallback is not defined.
+PASS NavigatorUserMediaErrorCallback.prototype threw exception ReferenceError: Can't find variable: NavigatorUserMediaErrorCallback.
 PASS successfullyParsed is true
 
 TEST COMPLETE

[Leandro Graciá Gil]

Sorry, I assigned the bug to Tommyw accidentally. I'm resetting it.

[Tommy Widenflycht]

The patch looks just fine, but just for clarification have you verified it in V8?

[Adam Bergkvist]

(In reply to comment #9)
> The patch looks just fine, but just for clarification have you verified it in V8?

No, but the chromium ews seems happy.

[Leandro Graciá Gil]

(In reply to comment #10)
> (In reply to comment #9)
> > The patch looks just fine, but just for clarification have you verified it in V8?
> 
> No, but the chromium ews seems happy.

Yes, it looks like. The Chromium ews bot doesn't seem to turn red when some tests fail (which was the case), but fortunately these were completely unrelated to this. So it seems it's working fine in V8 too.

[Adam Bergkvist]

Comment on attachment 92615 [details]
Proposed patch

Filed a new bug for the continued discussion about aligning the error messages between JSC and V8: http://webkit.org/b/62540

[WebKit Review Bot]

Comment on attachment 92615 [details]
Proposed patch

Clearing flags on attachment: 92615

Committed r89203: <http://trac.webkit.org/changeset/89203>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.