Last Comment Bug 684489 - Firefox won't show user-generated maps on Google Maps
: Firefox won't show user-generated maps on Google Maps
Status: VERIFIED FIXED
[testcase: comment 26][qa!]
: regression, testcase, verified-aurora, verified-beta
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: 9 Branch
: All All
: -- normal (vote)
: mozilla10
Assigned To: Jason Orendorff [:jorendorff]
:
:
Mentors:
: 690157 (view as bug list)
Depends on: 702572 706911 706972
Blocks: 561359
  Show dependency treegraph
 
Reported: 2011-09-03 05:44 PDT by Mathieu Marquer
Modified: 2011-12-02 10:29 PST (History)
16 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
fixed
+
fixed


Attachments
deterministic test case! works offline! :) (302.64 KB, application/x-gzip)
2011-09-13 09:24 PDT, Jason Orendorff [:jorendorff]
no flags Details
slightly reduced testcase (254.46 KB, application/x-zip-compressed)
2011-09-13 22:20 PDT, Gary Kwong [:gkw] [:nth10sd]
no flags Details
Reduced by 40% from jorendorff's (193.21 KB, application/x-zip-compressed)
2011-09-14 09:08 PDT, Gary Kwong [:gkw] [:nth10sd]
no flags Details
fully reduced testcase (601 bytes, text/html)
2011-09-15 01:14 PDT, Gary Kwong [:gkw] [:nth10sd]
no flags Details
Checkpoint reduced testcase (266.10 KB, application/zip)
2011-09-16 00:28 PDT, Gary Kwong [:gkw] [:nth10sd]
no flags Details
another testcase (246.30 KB, application/zip)
2011-09-16 00:32 PDT, Gary Kwong [:gkw] [:nth10sd]
no flags Details
another checkpoint (219.10 KB, application/zip)
2011-09-17 23:59 PDT, Gary Kwong [:gkw] [:nth10sd]
no flags Details
~1,800 line checkpoint (149.94 KB, application/zip)
2011-09-19 08:20 PDT, Gary Kwong [:gkw] [:nth10sd]
no flags Details
~1,350 line checkpoint (147.04 KB, application/zip)
2011-09-19 13:43 PDT, Gary Kwong [:gkw] [:nth10sd]
no flags Details
13,500 line fully beautified checkpoint (155.92 KB, application/zip)
2011-09-20 14:18 PDT, Gary Kwong [:gkw] [:nth10sd]
no flags Details
close to fully reduced testcase (20.05 KB, application/zip)
2011-09-22 07:14 PDT, Gary Kwong [:gkw] [:nth10sd]
no flags Details
< 700 line testcase (18.34 KB, application/zip)
2011-09-24 16:25 PDT, Gary Kwong [:gkw] [:nth10sd]
no flags Details
~400 line testcase (10.35 KB, application/x-javascript)
2011-10-25 19:22 PDT, Jason Orendorff [:jorendorff]
no flags Details
v1 (1.57 KB, patch)
2011-10-26 14:19 PDT, Jason Orendorff [:jorendorff]
jwalden+bmo: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description Mathieu Marquer 2011-09-03 05:44:15 PDT
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:9.0a1) Gecko/20110902 Firefox/9.0a1
Build ID: 20110902030838

Steps to reproduce:

Opening a user-generated map on Google Maps. For example, http://maps.google.fr/maps/ms?ie=UTF8&hl=fr&msa=0&msid=210131682796347244485.000472ea3aa4ad224cdfb&ll=48.872393,2.3909&spn=0.279109,0.700378&t=p&z=11&lci=transit_comp


Actual results:

Google Maps opens correctly, but :
* Only one symbol is listed on the left menu
* No symbol is shown on the user map


Expected results:

Google Maps should have been showing the full list on symbols, both on the left menu and on the map.
Comment 1 Mathieu Marquer 2011-09-03 05:45:24 PDT
Bug appears on my 2 test configurations :
* Firefox nightly, Ubuntu 11.04 64 bits
* Firefox nightly, Windows XP 32 bits
Both use French locale.
Comment 2 Alice0775 White 2011-09-03 06:15:57 PDT
Confirmed

And an an error in error console as follows:

Error: YP is not defined
Source file: http://maps.gstatic.com/cat_js/intl/fr_fr/mapfiles/363c/maps2/%7Bmain,mod_util,mod_actbr,mod_appiw,mod_info,mod_kml,mod_mp,mod_ms,mod_mssvt,mod_rst%7D.js
Line: 803



Regression window(cached m-c hourly),
Works:
http://hg.mozilla.org/mozilla-central/rev/005bce677a00
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:9.0a1) Gecko/20110831 Firefox/9.0a1 ID:20110831030834
Fails:
http://hg.mozilla.org/mozilla-central/rev/c7e6f57e1732
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:9.0a1) Gecko/20110831 Firefox/9.0a1 ID:20110831014759
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=005bce677a00&tochange=c7e6f57e1732


Regression window(cached m-i hourly),
Works:
http://hg.mozilla.org/integration/mozilla-inbound/rev/f1dbc0a63703
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:9.0a1) Gecko/20110830 Firefox/9.0a1 ID:20110830125405
Fails:
http://hg.mozilla.org/integration/mozilla-inbound/rev/9eaca4ef5880
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:9.0a1) Gecko/20110830 Firefox/9.0a1 ID:20110830142636
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=f1dbc0a63703&tochange=9eaca4ef5880
Comment 3 Alice0775 White 2011-09-03 08:56:12 PDT
triggered by;
9eaca4ef5880	Jason Orendorff — Bug 561359 - Predication of method optimization is too dynamic, causing "Assertion failure: &shape.methodObject() == &prev.toObject()". r=dvander.
Comment 4 Jason Orendorff [:jorendorff] 2011-09-06 10:26:51 PDT
I actually noticed this bug in Nightly yesterday. Never dreamed it could actually be my fault! I'll look into it today.

It might be a duplicate of bug 684178, in which case the fix is already in hand and awaiting review.
Comment 5 Jason Orendorff [:jorendorff] 2011-09-06 10:43:01 PDT
Taking. I suspect this is *not* a duplicate, but it's hard to tell.
Comment 6 Jason Orendorff [:jorendorff] 2011-09-13 09:24:01 PDT
Created attachment 559969 [details]
deterministic test case! works offline! :)
Comment 7 Gary Kwong [:gkw] [:nth10sd] 2011-09-13 22:20:39 PDT
Created attachment 560103 [details]
slightly reduced testcase

Slightly reduced by 20% but still large. No longer seems to require one to be offline to reproduce.

More reduction in progress but checkpointing is important.
Comment 8 Gary Kwong [:gkw] [:nth10sd] 2011-09-14 09:08:39 PDT
Created attachment 560169 [details]
Reduced by 40% from jorendorff's

Checkpoint: after a night of reduction, the main gmaps.html file is now reduced by >40% from jorendorff's testcase set.
Comment 9 Gary Kwong [:gkw] [:nth10sd] 2011-09-15 01:14:01 PDT
Created attachment 560321 [details]
fully reduced testcase

< 30 lines total, including 8 crucial lines:

try {
  t = function() {
	b = function() {}
  }
} catch (e) {}
setTimeout(function() {
  o(0, "").k()
})
Comment 10 Gary Kwong [:gkw] [:nth10sd] 2011-09-15 01:21:45 PDT
> Created attachment 560321 [details]
> fully reduced testcase

I'm not entirely sure if this testcase exhibits the bug yet, will confirm later.
Comment 11 Jason Orendorff [:jorendorff] 2011-09-15 12:09:45 PDT
No, this does not exhibit the bug. The same exception occurs both in buggy and non-buggy builds.
Comment 12 Gary Kwong [:gkw] [:nth10sd] 2011-09-15 12:50:13 PDT
Comment on attachment 560321 [details]
fully reduced testcase

Starting all over again.

:(
Comment 13 Emanuel Hoogeveen [:ehoogeveen] 2011-09-15 16:36:53 PDT
As someone following along at home, is the exception from the previous reduced testcase a legitimate problem separate from this bug, or is Firefox behaving correctly?
Comment 14 Gary Kwong [:gkw] [:nth10sd] 2011-09-15 17:05:08 PDT
(In reply to Emanuel Hoogeveen from comment #13)
> As someone following along at home, is the exception from the previous
> reduced testcase a legitimate problem separate from this bug, or is Firefox
> behaving correctly?

Apparently I think it's Firefox behaving correctly. I've thrown that out and started from jorendorff's original testcase.

Just to update, I have a testcase 20% smaller than the original that shows the error in Nightlies but not in Aurora. (according to the regression window in comment 2 or 3) This should be the symptom that I should have looked out for, which the original exhibits.

Reducing this one is a pain due to its complexity. Beautified, the original one is about 27,000 lines. (Ouch!)
Comment 15 Gary Kwong [:gkw] [:nth10sd] 2011-09-16 00:28:08 PDT
Created attachment 560521 [details]
Checkpoint reduced testcase

Here's a zipped testcase w/ 2 files, 20+% smaller than the original.

Shows the reference error (IP is not defined) in Nightlies, but not in Aurora. (similar to the original testcase)
Comment 16 Gary Kwong [:gkw] [:nth10sd] 2011-09-16 00:32:02 PDT
Created attachment 560522 [details]
another testcase

Here's another zipped testcase w/ 2 files, even smaller.

Shows the reference error (IP is not defined) in Nightlies, but in Aurora, this shows a Reference error for $u being not defined (differing from the original testcase which does not pop up anything at all in Aurora)

Jason, is this set of testcase on the right track, or is the previous one the only correct one?
Comment 17 Gary Kwong [:gkw] [:nth10sd] 2011-09-17 23:59:08 PDT
Created attachment 560758 [details]
another checkpoint

Offline, Jason mentions that I should look out for the "IP is not defined" message. The attached zip file contains a testcase (not yet fully reduced) that gives the aforementioned ReferenceError on Nightly but not on Aurora. (down to ~10,000 lines)

Thanks go out to Waldo, Jesse and decoder for their help on tips to speeding up the reduction process amongst other stuff.
Comment 18 Gary Kwong [:gkw] [:nth10sd] 2011-09-19 08:20:59 PDT
Created attachment 560914 [details]
~1,800 line checkpoint

Reduced over the weekend. Shows the "IP is not defined" error in Nightly 13092011 but not in Aurora 14092011.
Comment 19 Gary Kwong [:gkw] [:nth10sd] 2011-09-19 13:43:03 PDT
Created attachment 561007 [details]
~1,350 line checkpoint

Reduced again. Shows the "IP is not defined" error in Nightly 16092011 but not in Aurora 14092011.

Handing off to decoder to see how he can further reduce.
Comment 20 Gary Kwong [:gkw] [:nth10sd] 2011-09-20 14:18:36 PDT
Created attachment 561296 [details]
13,500 line fully beautified checkpoint

Turns out that there were a couple lines which were hundreds of thousands of characters long, and when that was beautified, (self note: watch for escaped double quotes, back slashes, as well as Regular Expressions), the testcase blew up to 13,500 lines. Checkpointing.

decoder and I also found out that this error may have different testcases on different platforms.

The testcase that is attached here is tested to show up the "IP is not defined" error on Windows 20 Sep 2011 nightly rev 648d084ca28e but not on the Windows Aurora 14 Sep 2011 rev 63d1f8b24911.
Comment 21 Gary Kwong [:gkw] [:nth10sd] 2011-09-22 07:14:35 PDT
Created attachment 561727 [details]
close to fully reduced testcase

< 1,000 line fully beautified checkpoint
Comment 22 Gary Kwong [:gkw] [:nth10sd] 2011-09-24 16:25:50 PDT
Created attachment 562266 [details]
< 700 line testcase

Off to langDDMin for decoder, testcase tested on Win7 nightly vs aurora.
Comment 23 Alice0775 White 2011-09-28 19:44:31 PDT
*** Bug 690157 has been marked as a duplicate of this bug. ***
Comment 24 Gary Kwong [:gkw] [:nth10sd] 2011-09-29 04:26:17 PDT
> Created attachment 562266 [details]
> < 700 line testcase

jorendorff, it's starting to get extremely tough for the reducers to further reduce this testcase.

Does this testcase reproduce the "IP is not defined" error for you on Nightly but not on Release?
Comment 25 Jason Orendorff [:jorendorff] 2011-10-25 19:22:12 PDT
Created attachment 569588 [details]
~400 line testcase

The HTML file for this is:

<!DOCTYPE html>
<html>
<head>
</head>
<body>
<script type="text/javascript" src="gmapsLong.js"></script>
</body>
</html>
Comment 26 Jason Orendorff [:jorendorff] 2011-10-26 10:59:50 PDT
Shell test case:

"use strict";
eval("var x = {}; ({p: function() { x.m; }}).p();");

Expected: does nothing
Observed: throws ReferenceError: x is not defined
Comment 27 Jason Orendorff [:jorendorff] 2011-10-26 11:07:02 PDT
The same bytecode is emitted before and after rev 9eaca4ef5880:

...
00024:  newinit 1
00027:  lambda (function () {"use strict";x.m;})
00030:  nullblockchain
00031:  initmethod "p"
00034:  endinit
00035:  callprop "p"
00038:  call 0
...

This lambda was never a candidate for the method optimization, though. Both before and after, that JSOP_NULLBLOCKCHAIN is incorrect.

It must be that the old dynamic checks prevented the bug from biting, and the new static behavior doesn't.
Comment 28 Jason Orendorff [:jorendorff] 2011-10-26 11:08:27 PDT
I didn't mean to change tracking-firefox10 from + to ?. I don't know how that happened. I don't have privileges to set it back to +.
Comment 29 Jason Orendorff [:jorendorff] 2011-10-26 12:37:14 PDT
The "var x" is behaving correctly, I think, and creating a variable in the implicitly created strict direct eval scope (not the global object). I think the bug is that the lambda is compiled as a null closure, even though it references x.

I see two possible fixes.

1. Treat strict direct eval scopes more like function scopes. On leaving one, do all the stuff we do in LeaveFunction to fix up yet-to-be-resolved lexdeps.

2. Treat strict direct eval scopes as dynamic for the purposes of the inAnyDynamicScope() call in Parser::setFunctionKinds.

The latter is easier.
Comment 30 Jason Orendorff [:jorendorff] 2011-10-26 14:19:50 PDT
Created attachment 569792 [details] [diff] [review]
v1

The status quo in BindTopLevelVar makes option 1 implausible. Here's option 2. I'm building a browser now to see if it fixes Google Maps.
Comment 31 Jeff Walden [:Waldo] (remove +bmo to email) 2011-10-26 14:52:38 PDT
Comment on attachment 569792 [details] [diff] [review]
v1

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

Interesting to see that Maps is using strict mode...

::: js/src/jit-test/tests/closures/bug684489.js
@@ +1,1 @@
> +"use strict";

This testcase doesn't look like the one pasted in the bug...
Comment 32 Jason Orendorff [:jorendorff] 2011-10-27 09:14:38 PDT
(In reply to comment #30)
> I'm building a browser now to see if it fixes Google Maps.

It does!

(In reply to comment #31)
> This testcase doesn't look like the one pasted in the bug...

Splinter bug. The patch is correct.

https://hg.mozilla.org/integration/mozilla-inbound/rev/f7a30d961569
Comment 33 Ed Morley [:emorley] 2011-10-28 04:35:43 PDT
https://hg.mozilla.org/mozilla-central/rev/f7a30d961569
Comment 34 Alice0775 White 2011-10-28 05:03:00 PDT
How about Aurora? 
This bug affects Aurora9.0a2.
Comment 35 Alex Keybl [:akeybl] 2011-11-01 14:38:56 PDT
Comment on attachment 569792 [details] [diff] [review]
v1

[Triage Comment]
* Approved for Aurora since 690157 suggests that this is a regression in 9
Comment 36 David Mandelin [:dmandelin] 2011-11-01 17:19:47 PDT
http://hg.mozilla.org/releases/mozilla-aurora/rev/6db605a34b15
Comment 37 juan becerra [:juanb] 2011-11-03 09:23:23 PDT
Adding [qa+] to add visibility during bug verification. High impact, has a testcase, and it should be easy to verify.
Comment 38 Simona B [:simonab ] 2011-11-22 05:43:53 PST
Verified issue using the test case attached in comment 26 - the ReferenceError: "x is not defined" is not thrown. Verified on Windows XP, Windows 7, Ubuntu 11.10 and Mac OS X 10.6 with Firefox 9.0b2, the latest Nightly and Aurora.

Mozilla/5.0 (Windows NT 5.1; rv:9.0) Gecko/20100101 Firefox/9.0
Mozilla/5.0 (Windows NT 5.1; rv:10.0a2) Gecko/20111121 Firefox/10.0a2
Mozilla/5.0 (Windows NT 5.1; rv:9.0a1) Gecko/20110902 Firefox/9.0a1

Mozilla/5.0 (X11; Linux x86_64; rv:9.0) Gecko/20100101 Firefox/9.0
Mozilla/5.0 (X11; Linux x86_64; rv:10.0a2) Gecko/20111121 Firefox/10.0a2
Mozilla/5.0 (X11; Linux x86_64; rv:11.0a1) Gecko/20111121 Firefox/11.0a1

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:9.0) Gecko/20100101 Firefox/9.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:10.0a2) Gecko/20111121 Firefox/10.0a2
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:11.0a1) Gecko/20111121 Firefox/11.0a1

Mozilla/5.0 (Windows NT 6.1; rv:9.0) Gecko/20100101 Firefox/9.0
Mozilla/5.0 (Windows NT 6.1; rv:10.0a2) Gecko/20111121 Firefox/10.0a2
Mozilla/5.0 (Windows NT 6.1; rv:11.0a1) Gecko/20111121 Firefox/11.0a1

Note You need to log in before you can comment on or make changes to this bug.