Closed
Bug 684489
Opened 13 years ago
Closed 13 years ago
Firefox won't show user-generated maps on Google Maps
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
VERIFIED
FIXED
mozilla10
People
(Reporter: mathieu.marquer, Assigned: jorendorff)
References
Details
(4 keywords, Whiteboard: [testcase: comment 26][qa!])
Attachments
(1 file, 13 obsolete files)
(deleted),
patch
|
Waldo
:
review+
akeybl
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Comment 1•13 years ago
|
||
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•13 years ago
|
||
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
Assignee: nobody → general
Status: UNCONFIRMED → NEW
Component: General → JavaScript Engine
Ever confirmed: true
Keywords: regression
OS: Linux → All
Product: Firefox → Core
QA Contact: general → general
Hardware: x86_64 → All
Comment 3•13 years ago
|
||
triggered by;
9eaca4ef5880 Jason Orendorff — Bug 561359 - Predication of method optimization is too dynamic, causing "Assertion failure: &shape.methodObject() == &prev.toObject()". r=dvander.
Blocks: 561359
Assignee | ||
Comment 4•13 years ago
|
||
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.
Assignee | ||
Comment 5•13 years ago
|
||
Taking. I suspect this is *not* a duplicate, but it's hard to tell.
Assignee: general → jorendorff
Updated•13 years ago
|
tracking-firefox9:
--- → +
Assignee | ||
Comment 6•13 years ago
|
||
Comment 7•13 years ago
|
||
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.
Attachment #559969 -
Attachment is obsolete: true
Comment 8•13 years ago
|
||
Checkpoint: after a night of reduction, the main gmaps.html file is now reduced by >40% from jorendorff's testcase set.
Attachment #560103 -
Attachment is obsolete: true
Comment 9•13 years ago
|
||
< 30 lines total, including 8 crucial lines:
try {
t = function() {
b = function() {}
}
} catch (e) {}
setTimeout(function() {
o(0, "").k()
})
Attachment #560169 -
Attachment is obsolete: true
Comment 10•13 years ago
|
||
> Created attachment 560321 [details]
> fully reduced testcase
I'm not entirely sure if this testcase exhibits the bug yet, will confirm later.
Assignee | ||
Comment 11•13 years ago
|
||
No, this does not exhibit the bug. The same exception occurs both in buggy and non-buggy builds.
Comment 12•13 years ago
|
||
Comment on attachment 560321 [details]
fully reduced testcase
Starting all over again.
:(
Attachment #560321 -
Attachment is obsolete: true
Comment 13•13 years ago
|
||
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•13 years ago
|
||
(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•13 years ago
|
||
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•13 years ago
|
||
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•13 years ago
|
||
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.
Attachment #560521 -
Attachment is obsolete: true
Attachment #560522 -
Attachment is obsolete: true
Comment 18•13 years ago
|
||
Reduced over the weekend. Shows the "IP is not defined" error in Nightly 13092011 but not in Aurora 14092011.
Attachment #560758 -
Attachment is obsolete: true
Comment 19•13 years ago
|
||
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.
Attachment #560914 -
Attachment is obsolete: true
Comment 20•13 years ago
|
||
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.
Attachment #561007 -
Attachment is obsolete: true
Comment 21•13 years ago
|
||
< 1,000 line fully beautified checkpoint
Attachment #561296 -
Attachment is obsolete: true
Comment 22•13 years ago
|
||
Off to langDDMin for decoder, testcase tested on Win7 nightly vs aurora.
Attachment #561727 -
Attachment is obsolete: true
Updated•13 years ago
|
tracking-firefox10:
--- → ?
Comment 24•13 years ago
|
||
> 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?
Updated•13 years ago
|
Assignee | ||
Comment 25•13 years ago
|
||
The HTML file for this is:
<!DOCTYPE html>
<html>
<head>
</head>
<body>
<script type="text/javascript" src="gmapsLong.js"></script>
</body>
</html>
Assignee | ||
Comment 26•13 years ago
|
||
Shell test case:
"use strict";
eval("var x = {}; ({p: function() { x.m; }}).p();");
Expected: does nothing
Observed: throws ReferenceError: x is not defined
Assignee | ||
Comment 27•13 years ago
|
||
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.
Assignee | ||
Comment 28•13 years ago
|
||
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 +.
Updated•13 years ago
|
Attachment #562266 -
Attachment is obsolete: true
Updated•13 years ago
|
Attachment #569588 -
Attachment is obsolete: true
Assignee | ||
Comment 29•13 years ago
|
||
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.
Assignee | ||
Comment 30•13 years ago
|
||
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.
Attachment #569792 -
Flags: review?(jwalden+bmo)
Comment 31•13 years ago
|
||
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...
Attachment #569792 -
Flags: review?(jwalden+bmo) → review+
Updated•13 years ago
|
Assignee | ||
Comment 32•13 years ago
|
||
(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•13 years ago
|
||
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla10
Comment 34•13 years ago
|
||
How about Aurora?
This bug affects Aurora9.0a2.
Updated•13 years ago
|
Attachment #569792 -
Flags: approval-mozilla-aurora?
Comment 35•13 years ago
|
||
Comment on attachment 569792 [details] [diff] [review]
v1
[Triage Comment]
* Approved for Aurora since 690157 suggests that this is a regression in 9
Attachment #569792 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 36•13 years ago
|
||
status-firefox10:
--- → fixed
status-firefox9:
--- → fixed
Comment 37•13 years ago
|
||
Adding [qa+] to add visibility during bug verification. High impact, has a testcase, and it should be easy to verify.
Whiteboard: [testcase: comment 26] → [testcase: comment 26][qa+]
Comment 38•13 years ago
|
||
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
Status: RESOLVED → VERIFIED
Keywords: verified-aurora,
verified-beta
Whiteboard: [testcase: comment 26][qa+] → [testcase: comment 26][qa!]
You need to log in
before you can comment on or make changes to this bug.
Description
•