Closed Bug 1537588 Opened 6 years ago Closed 6 years ago

Add crashtest to be sure we fire pagehide event in documents with sync XHR during pageload

Categories

(Core :: DOM: Core & HTML, enhancement, P2)

enhancement

Tracking

()

RESOLVED FIXED
mozilla68
Tracking Status
firefox68 --- fixed

People

(Reporter: dholbert, Assigned: dholbert)

Details

Attachments

(1 file, 1 obsolete file)

My initial patch in bug 1535388 would've prevented us from firing pageshow if there was a sync XHR during pageload.

It nearly passed on Try (aside from one arcane failure in test_disableScript.xul), so let's add some crashtests that more directly test that pageshow is indeed fired which would've caught my mistake.

Component: DOM: Events → DOM: Core & HTML
Priority: -- → P5
Priority: P5 → P2
Attachment #9052438 - Attachment description: Bug 1537588: Add crashtests to verify that pagehide and pageshow are fired, even if there's a sync XHR during pageload. r?bzbarsky → Bug 1537588: Add crashtest to verify that pageshow is fired, even if there's a sync XHR during pageload. r?bzbarsky

My "pagehide" variant doesn't seem to pass for some reason, actually, so I'll just add the pageshow one for now. That one reliably passes, locally, with a mozilla-central debug build.

(Updated patch)

Apparently this event isn't reliably fired in this scenario, even in normal mozilla-central. My crashtest was passing reliably yesterday, locally, and it passes on Android (it's shown as passing in the log for Android 4.3 opt C1 and debug C1 in my try run above) -- but it failed on all other Try platforms (specifically, it never fired pageshow, so it timed out).

I tried to rewrite the test with the sync XHR + navigation happening in an iframe, but that doesn't seem to help -- that still makes pageshow fire quite rarely (with "###!!! ASSERTION: Destroying a currently-showing document" being spammed most of the time).

I'm experimenting with an iframe-based pagehide test now, though, which seems more reliable locally (despite comment 2).
If that works, I might just settle for that at the moment, and hopefully we can extend it to cover pageshow as well once bug 1535388 or another followup is addressed.

Attachment #9052438 - Attachment is obsolete: true

Here's a Try run with a slightly older but functionally-equivalent version of this crashtest:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=0320684141902b2311e41e5747aef8227d79aa29

(That try run is layered on top of bug 1537588's patch -- without that patch, this crashtest causes an assertion-failure "Destroying a currently-showing document" as noted in comment 4, so we need that patch before we can land this crashtest.)

Summary: Add crashtest to be sure we fire pagehide & pageshow events in documents with sync XHR during pageload → Add crashtest to be sure we fire pagehide event in documents with sync XHR during pageload
Pushed by dholbert@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/ae7a26049a6c Add crashtest to verify that pagehide is fired, even if there's a sync XHR during pageload. r=bzbarsky
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
https://hg.mozilla.org/projects/ash/rev/ae7a26049a6c8e46b5a80c0cdbe9b7125219e1c9 Bug 1537588: Add crashtest to verify that pagehide is fired, even if there's a sync XHR during pageload. r=bzbarsky
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: