Bug 60391 - Media Stream no-interface-object test should not have ReferenceError message in expected results
Summary: Media Stream no-interface-object test should not have ReferenceError message ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Adam Bergkvist
URL:
Keywords:
Depends on:
Blocks: 60394
  Show dependency treegraph
 
Reported: 2011-05-06 11:44 PDT by Adam Bergkvist
Modified: 2011-06-18 13:18 PDT (History)
7 users (show)

See Also:


Attachments
Proposed patch (3.17 KB, patch)
2011-05-06 11:48 PDT, Adam Bergkvist
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Bergkvist 2011-05-06 11:44:51 PDT
The message for ReferenceError differs between JSC and V8 and having it in the expected results file requires unnecessary platform specific expected results.
Comment 1 Adam Bergkvist 2011-05-06 11:48:20 PDT
Created attachment 92615 [details]
Proposed patch
Comment 2 Darin Adler 2011-05-10 09:27:18 PDT
(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?
Comment 3 Adam Bergkvist 2011-05-11 02:36:54 PDT
(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).
Comment 4 Eric Seidel (no email) 2011-05-11 19:43:28 PDT
(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?

:(
Comment 5 Adam Barth 2011-05-11 20:56:19 PDT
> 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.
Comment 6 Leandro Graciá Gil 2011-05-26 09:20:00 PDT
This should be fixed by now. Can you please confirm it? All the tests passed in mac using JSC last time I tried.

Thanks.
Comment 7 Adam Bergkvist 2011-05-30 07:50:08 PDT
(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
Comment 8 Leandro Graciá Gil 2011-05-31 05:30:09 PDT
Sorry, I assigned the bug to Tommyw accidentally. I'm resetting it.
Comment 9 Tommy Widenflycht 2011-05-31 06:59:13 PDT
The patch looks just fine, but just for clarification have you verified it in V8?
Comment 10 Adam Bergkvist 2011-05-31 07:40:10 PDT
(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.
Comment 11 Leandro Graciá Gil 2011-05-31 08:30:15 PDT
(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.
Comment 12 Adam Bergkvist 2011-06-13 05:48:22 PDT
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
Comment 13 WebKit Review Bot 2011-06-18 13:18:16 PDT
Comment on attachment 92615 [details]
Proposed patch

Clearing flags on attachment: 92615

Committed r89203: <http://trac.webkit.org/changeset/89203>
Comment 14 WebKit Review Bot 2011-06-18 13:18:21 PDT
All reviewed patches have been landed.  Closing bug.