Allow LoadNavigationErrorPage to run scripts

Move the LoadNavigationErrorPage call from DidFinishDocumentLoad to
RunScriptsAtDocumentReady, because the former is in a
ScriptForbiddenScope and caused an assertion to be triggered in
blink::beforeCallEnteredCallback when the beforeunload listeners were
ran as a part of the navigation.

BUG=590634

Review URL: https://codereview.chromium.org/1837483002

Cr-Commit-Position: refs/heads/master@{#383355}

3 files changed, 28 insertions, 21 deletions

diff --git a/content/renderer/render_frame_impl.cc b/content/renderer/render_frame_impl.cc
index 67703b2..8d8d967 100644
--- a/content/renderer/render_frame_impl.cc
+++ b/content/renderer/render_frame_impl.cc
@@ -3355,8 +3355,7 @@ void RenderFrameImpl::didChangeIcon(blink::WebLocalFrame* frame,
   render_view_->didChangeIcon(frame, icon_type);
 }
 
-void RenderFrameImpl::didFinishDocumentLoad(blink::WebLocalFrame* frame,
-                                            bool document_is_empty) {
+void RenderFrameImpl::didFinishDocumentLoad(blink::WebLocalFrame* frame) {
   TRACE_EVENT1("navigation,benchmark", "RenderFrameImpl::didFinishDocumentLoad",
                "id", routing_id_);
   DCHECK_EQ(frame_, frame);
@@ -3372,6 +3371,26 @@ void RenderFrameImpl::didFinishDocumentLoad(blink::WebLocalFrame* frame,
 
   // Check whether we have new encoding name.
   UpdateEncoding(frame, frame->view()->pageEncoding().utf8());
+}
+
+void RenderFrameImpl::runScriptsAtDocumentReady(blink::WebLocalFrame* frame,
+                                                bool document_is_empty) {
+  DCHECK_EQ(frame_, frame);
+  base::WeakPtr<RenderFrameImpl> weak_self = weak_factory_.GetWeakPtr();
+
+  MojoBindingsController* mojo_bindings_controller =
+      MojoBindingsController::Get(this);
+  if (mojo_bindings_controller)
+    mojo_bindings_controller->RunScriptsAtDocumentReady();
+
+  if (!weak_self.get())
+    return;
+
+  GetContentClient()->renderer()->RunScriptsAtDocumentEnd(this);
+
+  // ContentClient might have deleted |frame| and |this| by now!
+  if (!weak_self.get())
+    return;
 
   // If this is an empty document with an http status code indicating an error,
   // we may want to display our own error page, so the user doesn't end up
@@ -3400,23 +3419,10 @@ void RenderFrameImpl::didFinishDocumentLoad(blink::WebLocalFrame* frame,
     error.unreachableURL = frame->document().url();
     error.domain = WebString::fromUTF8(error_domain);
     error.reason = http_status_code;
+    // This call may run scripts, e.g. via the beforeunload event.
     LoadNavigationErrorPage(frame->dataSource()->request(), error, true);
   }
-}
-
-void RenderFrameImpl::runScriptsAtDocumentReady(blink::WebLocalFrame* frame) {
-  base::WeakPtr<RenderFrameImpl> weak_self = weak_factory_.GetWeakPtr();
-
-  MojoBindingsController* mojo_bindings_controller =
-      MojoBindingsController::Get(this);
-  if (mojo_bindings_controller)
-    mojo_bindings_controller->RunScriptsAtDocumentReady();
-
-  if (!weak_self.get())
-    return;
-
-  GetContentClient()->renderer()->RunScriptsAtDocumentEnd(this);
-  // Do not use |this| or |frame|! ContentClient might have deleted them by now!
+  // Do not use |this| or |frame| here without checking |weak_self|.
 }
 
 void RenderFrameImpl::didHandleOnloadEvents(blink::WebLocalFrame* frame) {
diff --git a/content/renderer/render_frame_impl.h b/content/renderer/render_frame_impl.h
index 42f0f02..729c949 100644
--- a/content/renderer/render_frame_impl.h
+++ b/content/renderer/render_frame_impl.h
@@ -495,9 +495,9 @@ class CONTENT_EXPORT RenderFrameImpl
                        blink::WebTextDirection direction) override;
   void didChangeIcon(blink::WebLocalFrame* frame,
                      blink::WebIconURL::Type icon_type) override;
-  void didFinishDocumentLoad(blink::WebLocalFrame* frame,
-                             bool document_is_empty) override;
-  void runScriptsAtDocumentReady(blink::WebLocalFrame* frame) override;
+  void didFinishDocumentLoad(blink::WebLocalFrame* frame) override;
+  void runScriptsAtDocumentReady(blink::WebLocalFrame* frame,
+                                 bool document_is_empty) override;
   void didHandleOnloadEvents(blink::WebLocalFrame* frame) override;
   void didFailLoad(blink::WebLocalFrame* frame,
                    const blink::WebURLError& error,
diff --git a/content/renderer/render_view_browsertest.cc b/content/renderer/render_view_browsertest.cc
index 0695669..765a06e 100644
--- a/content/renderer/render_view_browsertest.cc
+++ b/content/renderer/render_view_browsertest.cc
@@ -1861,7 +1861,8 @@ TEST_F(RendererErrorPageTest, MAYBE_HttpStatusCodeErrorWithEmptyBody) {
 
   // Emulate a 4xx/5xx main resource response with an empty body.
   main_frame->didReceiveResponse(1, response);
-  main_frame->didFinishDocumentLoad(web_frame, true);
+  main_frame->didFinishDocumentLoad(web_frame);
+  main_frame->runScriptsAtDocumentReady(web_frame, true);
 
   // The error page itself is loaded asynchronously.
   FrameLoadWaiter(main_frame).Wait();