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}
bocoup · Mar 12, 2019 · 8f3d4f5 · 8f3d4f5
1 parent 652dc88
commit 8f3d4f5
Showing 1 changed file with 2 additions and 14 deletions.
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
@@ -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>