This is in JSParser::updateErrorMessageSpecialCase.
Do you know which specific test(s) fail because of the assert?
Created attachment 99851 [details] patch
Comment on attachment 99851 [details] patch Ah! I should have guessed it was this! Cheers for the fix JC. Do you have commit privileges? - or shall I cq+?
(In reply to comment #3) > (From update of attachment 99851 [details]) > Ah! I should have guessed it was this! Cheers for the fix JC. Do you have commit privileges? - or shall I cq+? Not yet ;) so you should cq+
(In reply to comment #4) > (In reply to comment #3) > > (From update of attachment 99851 [details] [details]) > > Ah! I should have guessed it was this! Cheers for the fix JC. Do you have commit privileges? - or shall I cq+? > > Not yet ;) so you should cq+ ...but wait until I make sure this patch wont fail any tests... I was focused on 262 so I forgot about the webkit tests.
Comment on attachment 99851 [details] patch Actually, I gave a bad review here. We really should have a regression test covering this, if possible. Could you look into this? We should be able to add something to a regression test to check this syntax error is thrown correctly. Could you try to produce a simple test that triggers the bug, and add to a layout test? (LayoutTests/fast/js/reserved-words might be a candidate if this is reserved-words specific, or there may be a existing syntax-errors related test, or you can add a new test case). Hope that sounds reasonable.
(In reply to comment #6) > (From update of attachment 99851 [details]) > Actually, I gave a bad review here. We really should have a regression test covering this, if possible. Could you look into this? We should be able to add something to a regression test to check this syntax error is thrown correctly. Could you try to produce a simple test that triggers the bug, and add to a layout test? (LayoutTests/fast/js/reserved-words might be a candidate if this is reserved-words specific, or there may be a existing syntax-errors related test, or you can add a new test case). Hope that sounds reasonable. On it!
Created attachment 99892 [details] updated patch Layout test actually exposed a logic bug that is fixed with this patch.
Attachment 99892 [details] did not pass style-queue: Failed to run "['Tools/Scripts/update-webkit', '--chromium']" exit_code: 9 Updating OpenSource RA layer request failed: OPTIONS of 'http://svn.webkit.org/repository/webkit': timed out waiting for server (http://svn.webkit.org) at /usr/lib/git-core/git-svn line 2295 Died at Tools/Scripts/update-webkit line 146. If any of these errors are false positives, please file a bug against check-webkit-style.
(In reply to comment #9) > Attachment 99892 [details] did not pass style-queue: > > Failed to run "['Tools/Scripts/update-webkit', '--chromium']" exit_code: 9 > I ran check-webkit-style locally and it passed with no errors.
Comment on attachment 99892 [details] updated patch Attachment 99892 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/8991356 New failing tests: fast/js/reserved-words-strict.html
Created attachment 99899 [details] Archive of layout-test-results from ec2-cr-linux-01 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: ec2-cr-linux-01 Port: Chromium Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Created attachment 99903 [details] updated patch Hopefully will work with chromium now
Comment on attachment 99903 [details] updated patch Looks good to me, cheers JC.
Comment on attachment 99903 [details] updated patch Clearing flags on attachment: 99903 Committed r90535: <http://trac.webkit.org/changeset/90535>
All reviewed patches have been landed. Closing bug.