Don't send location API navigations through NavigationScheduler

This is a small code change with a lot of test changes. The two most
common reasons for those changes are:
* Cases where the ordering of messages reaching the browser process
  changed in ways that browser_tests were sensitive to.
* A navigation scheduled by NavigationScheduler doesn't prevent a
  DidStopLoading() callback, but starting the navigation
  immediately does prevent DidStopLoading(). So location API
  navigations inside onload used to result in two pairs of
  DidStartLoading/DidStopLoading callbacks, where there is now only
  one pair.

Bug: 914587
Change-Id: I83b016041c4579a64cf9ccfd7876dba335b799d9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1526426
Commit-Queue: Nate Chapin <japhet@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#653352}

chrome/browser/history/redirect_browsertest.cc

@@ -145,11 +145,10 @@ IN_PROC_BROWSER_TEST_F(RedirectTest, ClientEmptyReferer) {
                             file_redirect_contents.data(),
                             file_redirect_contents.size()));
 
-  // Navigate to the file through the browser. The client redirect will appear
-  // as two page visits in the browser.
+  // Navigate to the file through the browser.
   GURL first_url = net::FilePathToFileURL(temp_file);
-  ui_test_utils::NavigateToURLBlockUntilNavigationsComplete(
-      browser(), first_url, 2);
+  ui_test_utils::NavigateToURLBlockUntilNavigationsComplete(browser(),
+                                                            first_url, 1);
 
   std::vector<GURL> redirects = GetRedirects(first_url);
   ASSERT_EQ(1U, redirects.size());
@@ -178,10 +177,10 @@ IN_PROC_BROWSER_TEST_F(RedirectTest, ClientCancelled) {
 
   std::vector<GURL> redirects = GetRedirects(first_url);
 
-  // There should be no redirects from first_url, because the anchor location
-  // change that occurs should not be flagged as a redirect and the meta-refresh
+  // There should be 1 redirect from first_url, because the anchor location
+  // change that occurs should be flagged as a redirect but the meta-refresh
   // won't have fired yet.
-  ASSERT_EQ(0U, redirects.size());
+  ASSERT_EQ(1U, redirects.size());
   EXPECT_EQ("myanchor", web_contents->GetURL().ref());
 }
 

chrome/test/data/extensions/api_test/activity_log_private/test/test.js

@@ -222,9 +222,13 @@ var domExpectedActivity = [
     'tabs.executeScript',
      // Location access
     'blinkSetAttribute LocalDOMWindow url',
+    'blinkRequestResource Main resource',
     'blinkSetAttribute LocalDOMWindow url',
+    'blinkRequestResource Main resource',
     'blinkSetAttribute LocalDOMWindow url',
+    'blinkRequestResource Main resource',
     'blinkSetAttribute LocalDOMWindow url',
+    'blinkRequestResource Main resource',
     // Dom mutations
     // Navigator access
     'Window.navigator',

chrome/test/data/extensions/api_test/tabs/on_updated/test.js

@@ -39,7 +39,6 @@ chrome.test.runTests([
     // renderer-initiated navigation.
     expect([
       { status: 'loading', url: getURL('browserThenRendererInitiated/a.html') },
-      { status: 'complete' },
       { status: 'loading', url: getURL('browserThenRendererInitiated/b.html') },
       { status: 'complete' },
     ]);

content/browser/security_exploit_browsertest.cc

@@ -1401,7 +1401,9 @@ IN_PROC_BROWSER_TEST_F(SecurityExploitBrowserTest,
 
   // Have the main frame navigate and lie that the initiator origin is b.com.
   injector.Activate();
-  EXPECT_TRUE(ExecJs(web_contents, "window.location = '/title2.html';"));
+  // Don't expect a response for the script, as the process may be killed
+  // before the script sends its completion message.
+  ExecuteScriptAsync(web_contents, "window.location = '/title2.html';");
 
   // Verify that the renderer was terminated.
   EXPECT_EQ(bad_message::INVALID_INITIATOR_ORIGIN, kill_waiter.Wait());
@@ -1424,7 +1426,9 @@ IN_PROC_BROWSER_TEST_F(SecurityExploitBrowserTest,
 
   // Have the main frame submit a BeginNavigation IPC with a missing initiator.
   injector.Activate();
-  EXPECT_TRUE(ExecJs(web_contents, "window.location = '/title2.html';"));
+  // Don't expect a response for the script, as the process may be killed
+  // before the script sends its completion message.
+  ExecuteScriptAsync(web_contents, "window.location = '/title2.html';");
 
   // Verify that the renderer was terminated.
   EXPECT_EQ(bad_message::RFHI_BEGIN_NAVIGATION_MISSING_INITIATOR_ORIGIN,

third_party/blink/renderer/bindings/core/v8/activity_logger_test.cc

@@ -544,12 +544,16 @@ TEST_F(ActivityLoggerTest, LocalDOMWindowAttribute) {
   const char* expected_activities =
       "blinkSetAttribute | LocalDOMWindow | url | about:blank | "
       "data:text/html;charset=utf-8,A\n"
+      "blinkRequestResource | Main resource | data:text/html;charset=utf-8,A\n"
       "blinkSetAttribute | LocalDOMWindow | url | about:blank | "
       "data:text/html;charset=utf-8,B\n"
+      "blinkRequestResource | Main resource | data:text/html;charset=utf-8,B\n"
       "blinkSetAttribute | LocalDOMWindow | url | about:blank | "
       "data:text/html;charset=utf-8,C\n"
+      "blinkRequestResource | Main resource | data:text/html;charset=utf-8,C\n"
       "blinkSetAttribute | LocalDOMWindow | url | about:blank | "
       "protocol:blank\n"
+      "blinkRequestResource | Main resource | protocol:blank\n"
       "blinkSetAttribute | LocalDOMWindow | url | about:blank | "
       "about:pathname\n"
       "blinkSetAttribute | LocalDOMWindow | url | about:blank | "

third_party/blink/renderer/core/frame/location.cc

@@ -34,7 +34,7 @@
 #include "third_party/blink/renderer/core/frame/csp/content_security_policy.h"
 #include "third_party/blink/renderer/core/frame/dom_window.h"
 #include "third_party/blink/renderer/core/frame/local_dom_window.h"
-#include "third_party/blink/renderer/core/frame/local_frame.h"
+#include "third_party/blink/renderer/core/loader/frame_load_request.h"
 #include "third_party/blink/renderer/core/loader/frame_loader.h"
 #include "third_party/blink/renderer/core/trustedtypes/trusted_types_util.h"
 #include "third_party/blink/renderer/core/url/dom_url_utils_read_only.h"
@@ -317,14 +317,16 @@ void Location::SetLocation(const String& url,
     argv.push_back(completed_url);
     activity_logger->LogEvent("blinkSetAttribute", argv.size(), argv.data());
   }
+
+  FrameLoadRequest request(current_window->document(),
+                           ResourceRequest(completed_url));
+  request.SetClientRedirectReason(ClientNavigationReason::kFrameNavigation);
   WebFrameLoadType frame_load_type = WebFrameLoadType::kStandard;
   if (set_location_policy == SetLocationPolicy::kReplaceThisFrame)
     frame_load_type = WebFrameLoadType::kReplaceCurrentItem;
 
   current_window->GetFrame()->MaybeLogAdClickNavigation();
-  dom_window_->GetFrame()->ScheduleNavigation(*current_window->document(),
-                                              completed_url, frame_load_type,
-                                              UserGestureStatus::kNone);
+  dom_window_->GetFrame()->Navigate(request, frame_load_type);
 }
 
 Document* Location::GetDocument() const {

third_party/blink/renderer/core/frame/remote_frame.cc

@@ -69,9 +69,6 @@ void RemoteFrame::ScheduleNavigation(Document& origin_document,
   FrameLoadRequest frame_request(&origin_document, ResourceRequest(url));
   frame_request.GetResourceRequest().SetHasUserGesture(
       user_gesture_status == UserGestureStatus::kActive);
-  frame_request.SetFrameType(
-      IsMainFrame() ? network::mojom::RequestContextFrameType::kTopLevel
-                    : network::mojom::RequestContextFrameType::kNested);
   frame_request.SetClientRedirectReason(
       ClientNavigationReason::kFrameNavigation);
   Navigate(frame_request, frame_load_type);
@@ -83,6 +80,9 @@ void RemoteFrame::Navigate(const FrameLoadRequest& passed_request,
     return;
 
   FrameLoadRequest frame_request(passed_request);
+  frame_request.SetFrameType(
+      IsMainFrame() ? network::mojom::RequestContextFrameType::kTopLevel
+                    : network::mojom::RequestContextFrameType::kNested);
 
   const KURL& url = frame_request.GetResourceRequest().Url();
   if (frame_request.OriginDocument() &&

third_party/blink/web_tests/external/wpt/html/browsers/browsing-the-web/unloading-documents/beforeunload-synchronous-expected.txt

-This is a testharness.js-based test.
-FAIL beforeunload event is emitted synchronously assert_equals: invoked synchronously exactly once expected 1 but got 0
-Harness: the test ran to completion.
-

third_party/blink/web_tests/external/wpt/html/semantics/scripting-1/the-script-element/execution-timing/028-expected.txt

-This is a testharness.js-based test.
-FAIL  scheduler: javascript: URL assert_array_equals: lengths differ, expected 4 got 3
-
-assert_array_equals: lengths differ, expected 4 got 3
-Harness: the test ran to completion.
-

third_party/blink/web_tests/fast/dom/location-new-window-no-crash-expected.txt

 CONSOLE ERROR: line 15: Not allowed to navigate top frame to data URL: data:text/plain,a
+CONSOLE ERROR: Not allowed to navigate top frame to data URL: data:text/plain,b
+CONSOLE ERROR: Not allowed to navigate top frame to data URL: data:text/plain,i
+CONSOLE ERROR: Not allowed to navigate top frame to data URL: data:text/plain,j
 Tests that manipulating location properties in a just-created window object does not crash. Note: Turn off pop-up blocking to run this in-browser.
 
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".

third_party/blink/web_tests/fast/layout/subtree-layout-with-javascript-navigate-crash-expected.txt

-Test passes if no crash with ASAN. 
+Test passes if no crash with ASAN.

third_party/blink/web_tests/fast/layout/subtree-layout-with-javascript-navigate-crash.html

 <!DOCTYPE html>
 <body>
-Test passes if no crash with ASAN.
 <input value="boom">
 <script>
 if (window.testRunner) {
@@ -15,7 +14,7 @@ function explode() {
     input.value = value.substr(0, value.length - 1);
     window.setTimeout(explode, 0);
   } else {
-    window.location.href="javascript:''";
+    window.location.href="javascript:'Test passes if no crash with ASAN.'";
     if (window.testRunner)
       testRunner.notifyDone();
   }

third_party/blink/web_tests/http/tests/loading/stop-load-at-commit-expected.txt

@@ -3,10 +3,10 @@ main frame - ReadyToCommitNavigation
 main frame - didCommitLoadForFrame
 main frame - didReceiveTitle: 
 main frame - didFinishDocumentLoadForFrame
-main frame - didHandleOnloadEventsForFrame
-main frame - didFinishLoadForFrame
 main frame - BeginNavigation request to 'http://127.0.0.1:8000/navigation/resources/pass-and-notify-done.html', http method GET
 main frame - DidStartNavigation
+main frame - didFinishLoadForFrame
+main frame - didHandleOnloadEventsForFrame
 main frame - ReadyToCommitNavigation
 main frame - didCommitLoadForFrame
 main frame - didReceiveTitle: 

third_party/blink/web_tests/http/tests/misc/onload-detach-during-csp-frame-src-none-expected.txt

-CONSOLE ERROR: Refused to frame '' because it violates the following Content Security Policy directive: "frame-src 'none'".
+CONSOLE ERROR: line 11: Refused to frame '' because it violates the following Content Security Policy directive: "frame-src 'none'".
 
 PASS

third_party/blink/web_tests/http/tests/security/aboutBlank/security-context-grandchildren-write-lexical-expected.txt

@@ -27,3 +27,4 @@ document.URL = http://127.0.0.1:8000/security/aboutBlank/security-context-grandc
 document.baseURI = http://127.0.0.1:8000/security/aboutBlank/security-context-grandchildren-write-lexical.html
 document.cookie = cookie=parent
 --- Test ends ---
+Helpers loaded!

third_party/blink/web_tests/http/tests/security/aboutBlank/security-context-grandchildren-writeln-lexical-expected.txt

@@ -27,3 +27,4 @@ document.URL = http://127.0.0.1:8000/security/aboutBlank/security-context-grandc
 document.baseURI = http://127.0.0.1:8000/security/aboutBlank/security-context-grandchildren-writeln-lexical.html
 document.cookie = cookie=parent
 --- Test ends ---
+Helpers loaded!

third_party/blink/web_tests/http/tests/security/xss-DENIED-javascript-variations.html

@@ -12,18 +12,18 @@ function runTest() {
 
   a.location.href = " javascript:document.write('FAIL')";
   a.location.href = "javascript\t:document.write('FAIL')";
-  try { a.location.href = "javascript\1:document.write('FAIL')"; } catch(e) {}
+
   a.location.href = "javascript:document.write('FAIL')";
 
 
   a.location.replace(" javascript:document.write('FAIL')");
   a.location.replace("javascript\t:document.write('FAIL')");
-  try { a.location.replace("javascript\1:document.write('FAIL')"); } catch(e) {}
+
   a.location.replace("javascript:document.write('FAIL')");
 
   a.location = " javascript:document.write('FAIL')";
   a.location = "javascript\t:document.write('FAIL')";
-  try { a.location = "javascript\1:document.write('FAIL')"; } catch(e) {}
+
   a.location = "javascript:document.write('FAIL')";
 }
 </script>