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 if 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/c/chromium/src/+/1506663
Commit-Queue: Dmitry Gozman <dgozman@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#639992}
diff --git a/html/browsers/browsing-the-web/read-media/pageload-image-in-popup.html b/html/browsers/browsing-the-web/read-media/pageload-image-in-popup.html
index 4d81119..e928482 100644
--- a/html/browsers/browsing-the-web/read-media/pageload-image-in-popup.html
+++ b/html/browsers/browsing-the-web/read-media/pageload-image-in-popup.html
@@ -13,18 +13,7 @@
var t = async_test("The document for a standalone media file should have one child in the body.");
var imgwin = window.open('/images/blue.png');
- // imgwin.onload doesn't work, check popup's URL to see if the loading of
- // the image and creation of image document is finished.
- function checkURL() {
- if (imgwin.location.href.indexOf('blue.png') == -1) {
- step_timeout(checkURL, 100);
- return;
- }
- t.step(frameLoaded);
- }
- checkURL();
-
- function frameLoaded() {
+ imgwin.onload = t.step_func_done(function() {
assert_equals(imgwin.opener, window);
assert_equals(imgwin.document.contentType, "image/png");
var imgwinChildren = imgwin.document.body.childNodes;
@@ -33,8 +22,7 @@
assert_equals(imgwinChildren[0].namespaceURI, "http://www.w3.org/1999/xhtml",
"Only child of body must be an HTML element");
imgwin.close();
- t.done();
- }
+ });
</script>
</head>
<body>
