604924 - only top left of custom google map displays

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Timothy Nikkel (:tnikkel)]

When I visit http://davidpritchard.org/maps/vantransit.html only the top left chunk of the map is drawn. Bisected this to rev 0ab712643a66 or bug 591981.

Comment 1

[Alice0775 White]

If I set  html5.enable  to false, the problem does not happen.

Comment 2

[Henri Sivonen (:hsivonen)]

(In reply to comment #1)
> If I set  html5.enable  to false, the problem does not happen.

Strange. Bug 591981 affected the old parser, too.

Comment 3

[Henri Sivonen (:hsivonen)]

Hmm. Thinking about where the effect of bug 591981 differs with the old and new parser, this may have something to do with how document.written inline scripts react to pending style sheets.

Comment 4

[Alice0775 White]

The map is drawn entirely when I resize a browser after the loading of the page.

Comment 5

[Henri Sivonen (:hsivonen)]

I have diagnosed this. A perf optimization is faulty and doesn't wait for style sheets.

Comment 6

[Henri Sivonen (:hsivonen)]

Attachment: Fix: Check for pending sheets before optimizing parser-inserted preloaded external scripts

Comment 7

[Henri Sivonen (:hsivonen)]

Attachment: Mochitest that doesn't work

I tried to write a test case for this and failed. I'd appreciate it if someone could point out to me why this test case doesn't fail without the fix for this bug.

Anyway, attachment 483949 [details] [diff] [review] fixes the reported problem.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Don't you need the stylesheet in that case to also load slowly?  In fact slowly enough that we restart the parser after the first <script> finishes loading and parse the second script and kick off its load before the stylesheet loads?

Comment 9

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 483949 [details] [diff] [review]
Fix: Check for pending sheets before optimizing parser-inserted preloaded external scripts

r=me if you also land a test :)

Comment 10

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

For what it's worth, it's now possible to stall a response indefinitely using

http://mxr.mozilla.org/mozilla-central/source/netwerk/test/httpserver/nsIHttpServer.idl#594

together with

https://developer.mozilla.org/En/Mochitest#How_do_I_keep_state_across_loads_of_different_server-side_scripts.3f

Comment 11

[Henri Sivonen (:hsivonen)]

Attachment: Mochitest

(In reply to comment #8)
> Don't you need the stylesheet in that case to also load slowly?  In fact slowly
> enough that we restart the parser after the first <script> finishes loading and
> parse the second script and kick off its load before the stylesheet loads?

Indeed, I had the timers in the style sheet and the script sjs the wrong way round. Here's test with timer values that make it fail without the bug fix.

Comment 12

[Henri Sivonen (:hsivonen)]

beltzner, given that bug 604660 was upgraded to beta7, I think it would make sense to upgrade this one to beta7, too, since the same landing that caused bug 604660 after beta6 also caused this bug. Furthermore, this bug has a super-simple fix and is only lacking test case review.

Comment 13

[Mike Beltzner [:beltzner, not reading bugmail]]

Sound logic. Thanks for bringing it to my attention, Henri!

Comment 14

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 485038 [details] [diff] [review]
Mochitest

So this isn't entirely good. In general things based on timeouts are very fragile. Timeouts have been notoriously problematic, especially on our automated testing servers.

While in this case things are somewhat better since we'd randomly go green in case there is bustage, rather than randomly go orange when things work. However it's still good to avoid when we can, and in this case I think we can. Or at least get it much better.

A better solution would be to use a state [1] variables to trigger loads finishing.

Make the two sjs files wait for a state variable to be set (by polling using a timer). When the variable is set let script_bug604924-slow.sjs immediately finish its request, but let file_bug604924.sjs wait for another half a second before finishing its request.

Replace the current static .js file with a third sjs file which sets the state variable.

An alternative solution is to have the two current sjs files store their requests in state variables, and have the third sjs-file pick them up and call finish on them as needed. The only complication there is that the third sjs request might reach the server first, so you'd have to handle that situation.

In the meantime, in order to unblock b7, lets just land the code, mark this fixed and file a separate bug on the test. (This in order to keep the tracking for the b7 blocker easier).

[1] https://developer.mozilla.org/En/Mochitest#How_do_I_keep_state_across_loads_of_different_server-side_scripts.3f

Comment 15

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/fe1263a246bd