Mitigate a crash when a navigation to a media document fails.

Blink has some logic for rendering a custom error page if a navigation
fails due to an HTTP error and the response body is empty, so the user
does not simply seen a blank page on error.

https://crrev.com/796508 added a number of `CHECK`s to assert that
navigations always commit. However, it incorrectly trips in the case of
a navigation to a media document that results in an HTTP error since:

1. Media documents synchronously complete loading while handling the
   `CommitNavigation` IPC.
2. When loading is complete, //content checks if the navigation
   resulted in an HTTP error—and if so, whether the response body was
   empty.
3. If both of those conditions are true, //content synthesises a custom
   error page and calls `blink::WebNavigationControl::CommitNavigation`.

Step 3 currently asserts that no commit is in progress, which is not
true in the case of media documents. This CL is intended to be a small
and mergeable patch and simply updates the error page handling in
`RunScriptsAtDocumentReady()` to better handle reentrant commits.

Bug: 1127837
Change-Id: Ied6ce00e10ff8e3b75306e97dba3d28fcd2e4c6f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2430769
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#811593}

content/browser/navigation_browsertest.cc

@@ -69,6 +69,7 @@
 #include "net/dns/mock_host_resolver.h"
 #include "net/test/embedded_test_server/controllable_http_response.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
+#include "net/test/embedded_test_server/http_response.h"
 #include "net/test/url_request/url_request_failed_job.h"
 #include "services/network/public/cpp/features.h"
 #include "services/network/public/mojom/url_loader.mojom.h"
@@ -3552,6 +3553,32 @@ IN_PROC_BROWSER_TEST_F(NavigationBrowserTest, FormSubmissionThenDeleteFrame) {
   loop.Run();
 }
 
+using MediaNavigationBrowserTest = NavigationBaseBrowserTest;
+
+// Media navigations synchronously complete the time of the `CommitNavigation`
+// IPC call. Ensure that the renderer does not crash if the media navigation
+// results in an HTTP error with no body, since the renderer will reentrantly
+// commit an error page while handling the `CommitNavigation` IPC.
+IN_PROC_BROWSER_TEST_F(MediaNavigationBrowserTest, FailedNavigation) {
+  embedded_test_server()->RegisterRequestHandler(base::BindRepeating(
+      [](const net::test_server::HttpRequest& request)
+          -> std::unique_ptr<net::test_server::HttpResponse> {
+        auto response = std::make_unique<net::test_server::BasicHttpResponse>();
+        response->set_code(net::HTTP_NOT_FOUND);
+        response->set_content_type("video/mp4");
+        return response;
+      }));
+  ASSERT_TRUE(embedded_test_server()->Start());
+
+  const GURL error_url(embedded_test_server()->GetURL("/moo.mp4"));
+  EXPECT_FALSE(NavigateToURL(shell(), error_url));
+  EXPECT_EQ(error_url,
+            shell()->web_contents()->GetMainFrame()->GetLastCommittedURL());
+  NavigationEntry* entry =
+      shell()->web_contents()->GetController().GetLastCommittedEntry();
+  EXPECT_EQ(PAGE_TYPE_ERROR, entry->GetPageType());
+}
+
 class DocumentPolicyBrowserTest : public NavigationBaseBrowserTest {
  public:
   DocumentPolicyBrowserTest() {

content/renderer/render_frame_impl.cc

@@ -4550,11 +4550,27 @@ void RenderFrameImpl::RunScriptsAtDocumentReady(bool document_is_empty) {
   navigation_params->service_worker_network_provider =
       ServiceWorkerNetworkProviderForFrame::CreateInvalidInstance();
 
-  CHECK_EQ(NavigationCommitState::kNone, navigation_commit_state_);
-  AssertNavigationCommits assert_navigation_commits(this);
-  frame_->CommitNavigation(std::move(navigation_params), BuildDocumentState());
+  // TODO(dcheng): Remove this strange case. Typically, loading finishes
+  // asynchronously, so this will not be called while `CommitNavigation()`
+  // is on the stack. However, completion for media files is synchronously
+  // signalled in `blink::DocumentLoader::StartLoadingResponse()`. To prevent
+  // the CHECK in `AssertNavigationCommits` from tripping, temporarily reset the
+  // state and restore it after the reentrant `CommitNavigation()` completes.
+  bool reentrantly_committing =
+      (NavigationCommitState::kNone != navigation_commit_state_);
+  if (reentrantly_committing) {
+    CHECK_EQ(NavigationCommitState::kDidCommit, navigation_commit_state_);
+    navigation_commit_state_ = NavigationCommitState::kNone;
+  }
+  {
+    AssertNavigationCommits assert_navigation_commits(this);
+    frame_->CommitNavigation(std::move(navigation_params),
+                             BuildDocumentState());
+  }
   // WARNING: The previous call may have have deleted |this|.
   // Do not use |this| or |frame_| here without checking |weak_self|.
+  if (weak_self && reentrantly_committing)
+    navigation_commit_state_ = NavigationCommitState::kDidCommit;
 }
 
 void RenderFrameImpl::RunScriptsAtDocumentIdle() {