Fix a crash in WebUIMessageHandler::AllowJavascript

If WebUI's JS is sending the message when we navigate cross-site,
then we use the new process ID from WebContents in
WebUIImpl::CanCallJavascript and a CHECK crash will happen.

This CL changes to use the old process ID from RFH in
WebUIImpl::CanCallJavascript.

Bug: 1110276
Change-Id: Ie827060b637dbd3aa4c7ad771c0cb4862763ced2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2326050Reviewed-by: Nasko Oskov <nasko@chromium.org>
Commit-Queue: Miyoung Shin <myid.shin@igalia.com>
Cr-Commit-Position: refs/heads/master@{#793816}

content/browser/webui/web_ui_browsertest.cc

@@ -14,9 +14,11 @@
 #include "base/run_loop.h"
 #include "base/strings/strcat.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/test/bind_test_util.h"
 #include "base/test/simple_test_tick_clock.h"
 #include "base/time/time.h"
 #include "content/browser/webui/content_web_ui_controller_factory.h"
+#include "content/browser/webui/web_ui_impl.h"
 #include "content/public/browser/child_process_security_policy.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_process_host.h"
@@ -52,6 +54,10 @@ class TestWebUIMessageHandler : public WebUIMessageHandler {
         "notifyFinish",
         base::BindRepeating(&TestWebUIMessageHandler::OnNotifyFinish,
                             base::Unretained(this)));
+    web_ui()->RegisterMessageCallback(
+        "sendMessage",
+        base::BindRepeating(&TestWebUIMessageHandler::OnSendMessase,
+                            base::Unretained(this)));
   }
 
   void set_finish_closure(base::RepeatingClosure closure) {
@@ -72,6 +78,13 @@ class TestWebUIMessageHandler : public WebUIMessageHandler {
       finish_closure_.Run();
   }
 
+  void OnSendMessase(const base::ListValue* args) {
+    AllowJavascript();
+
+    if (finish_closure_)
+      std::move(finish_closure_).Run();
+  }
+
   int message_requiring_gesture_count_ = 0;
   base::RepeatingClosure finish_closure_;
 };
@@ -338,6 +351,34 @@ IN_PROC_BROWSER_TEST_F(WebUIImplBrowserTest, UntrustedSchemeLoads) {
             web_contents->GetTitle());
 }
 
+// Verify that we can successfully navigate to a chrome-untrusted:// URL
+// without a crash while WebUI::Send is being performed.
+IN_PROC_BROWSER_TEST_F(WebUIImplBrowserTest, NavigateWhileWebUISend) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+
+  auto* web_contents = shell()->web_contents();
+  ASSERT_TRUE(NavigateToURL(web_contents, GetWebUIURL(kChromeUIGpuHost)));
+
+  auto* test_handler = new TestWebUIMessageHandler;
+  web_contents->GetWebUI()->AddMessageHandler(base::WrapUnique(test_handler));
+
+  auto* webui = static_cast<WebUIImpl*>(web_contents->GetWebUI());
+  EXPECT_EQ(web_contents->GetMainFrame(), webui->frame_host_for_test());
+
+  test_handler->set_finish_closure(base::BindLambdaForTesting([&]() {
+    EXPECT_NE(web_contents->GetMainFrame(), webui->frame_host_for_test());
+  }));
+
+  web_contents->GetMainFrame()->ExecuteJavaScriptForTests(
+      base::ASCIIToUTF16("onunload=function() { chrome.send('sendMessage')}"),
+      base::NullCallback());
+
+  RenderFrameDeletedObserver delete_observer(web_contents->GetMainFrame());
+  EXPECT_TRUE(NavigateToURL(
+      web_contents, embedded_test_server()->GetURL("/simple_page.html")));
+  delete_observer.WaitUntilDeleted();
+}
+
 class WebUIRequestSchemesTest : public ContentBrowserTest {
  public:
   WebUIRequestSchemesTest() {

content/browser/webui/web_ui_impl.cc

@@ -203,13 +203,11 @@ void WebUIImpl::SetController(std::unique_ptr<WebUIController> controller) {
 }
 
 bool WebUIImpl::CanCallJavascript() {
-  RenderFrameHost* frame_host = web_contents_->GetMainFrame();
-  return frame_host &&
-         (ChildProcessSecurityPolicyImpl::GetInstance()->HasWebUIBindings(
-              frame_host->GetProcess()->GetID()) ||
+  return (ChildProcessSecurityPolicyImpl::GetInstance()->HasWebUIBindings(
+              frame_host_->GetProcess()->GetID()) ||
           // It's possible to load about:blank in a Web UI renderer.
           // See http://crbug.com/42547
-          frame_host->GetLastCommittedURL().spec() == url::kAboutBlankURL);
+          frame_host_->GetLastCommittedURL().spec() == url::kAboutBlankURL);
 }
 
 void WebUIImpl::CallJavascriptFunctionUnsafe(const std::string& function_name) {

content/browser/webui/web_ui_impl.h

@@ -93,6 +93,8 @@ class CONTENT_EXPORT WebUIImpl : public WebUI,
 
   const mojo::Remote<mojom::WebUI>& GetRemoteForTest() const { return remote_; }
 
+  RenderFrameHost* frame_host_for_test() const { return frame_host_; }
+
  private:
   class MainFrameNavigationObserver;