Closed Bug 1535535 Opened 5 years ago Closed 5 years ago

[wpt-sync] Sync PR 15770 - DocumentLoader: commit navigation synchronously. Attempt #2

Categories

(Testing :: web-platform-tests, enhancement, P4)

Product:
Testing
Component:
web-platform-tests
Type:
enhancement

Tracking

(firefox68 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla68
Tracking Flags:
Tracking Status
firefox68 --- fixed

People

(Reporter: mozilla.org, Unassigned)

References

(
URL
)

Details

(Whiteboard: [wptsync downstream])

Sync web-platform-tests PR 15770 into mozilla-central (this bug is closed when the sync is complete).

PR: https://github.com/web-platform-tests/wpt/pull/15770
Details from upstream follow.

Dmitry Gozman <dgozman@chromium.org> wrote:

DocumentLoader: commit navigation synchronously. Attempt #2

We used to commit navigation after receiving the first byte of
document response. This CL moves commit earlier, synchronously done
from CommitNavigation call. The change should not be web-observable,
but some internal assumptions may have been affected.

Test changes:

  • ReplacingDocumentLoaderFiresLoadEvent was testing the old behavior,
    which is not applicable anymore.
  • MultiChunkWithReentrancy now uses a different method to trigger
    reentrancy (pdf plugin), since we no longer commit after first byte.
  • backdrop-object.html and anchor-change-href.svg relied on test finishing
    late enough, now they wait for onload to eliminate a race.
  • use-property-synchronization-crash.html now reports an error message
    synchronously and therefore has JS stack and a line number.
  • setting-allowpaymentrequest-timing.https.sub.html has a race as
    explained here [1], and now fails even without site isolation.

This corresponds to the step 8.b from the doc linked to the bug.

Difference from attempt #1 (https://chromium-review.googlesource.com/c/1399447):

  • PluginDocumentParser and MediaDocumentParser early return is not parsing
    before accessing GetDocument. This is because DocumentLoader calls Finish()
    even after parser was stopped/detached. For example in Document::Abort
    we cancel parsing, but committed DocumentLoader might be still receiving data.
    We should ideally clean up all calls into parser, there are numerous TODOs
    for that.
  • pageload-image-in-popup.html relies on small image being parsed in the same
    task as navigation commit. Using onload seems to fix the issue.
  • touch-handler-iframe-plugin-assert.js hopes that onload for about:blank
    happens after test has finished, which is racy now.

[1] https://bugs.chromium.org/p/chromium/issues/detail?id=819800#c6

Bug: 855189, 937639, 836242, 937358
Change-Id: I65048a27e6d249a608d4eb61e5c882292386026e

Reviewed-on: https://chromium-review.googlesource.com/1506663
WPT-Export-Revision: e71e759aa54d058a640909ba1d4e12303409d069

Ran 1 tests and 1 subtests
OK     : 1
PASS   : 1
Pushed by james@hoppipolla.co.uk:
https://hg.mozilla.org/integration/mozilla-inbound/rev/beba24c8179b
[wpt PR 15770] - DocumentLoader: commit navigation synchronously. Attempt #2, a=testonly

https://hg.mozilla.org/mozilla-central/rev/beba24c8179b

Status: NEW → RESOLVED
Closed: 5 years ago
status-firefox68: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
You need to log in before you can comment on or make changes to this bug.