[WebUI] Use WebUIController::RenderFrameCreated() to request MojoJS.

ConversionInternalsUI and ProcessInternalsUI don't need to be
WebContentsObservers at all, since WebUIController already provides a
RenderFrameCreated() override that is scoped WebUI-specific
RenderFrameHosts.

Bug: 1148682, 1149125
Change-Id: Iebef544d7fb7c8938d273c84279ce7e8b19f90a5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2566889Reviewed-by: Nasko Oskov <nasko@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#832563}

content/browser/conversions/conversion_internals_browsertest.cc

@@ -41,7 +41,7 @@ class ConversionInternalsWebUiBrowserTest : public ContentBrowserTest {
   // Executing javascript in the WebUI requires using an isolated world in which
   // to execute the script because WebUI has a default CSP policy denying
   // "eval()", which is what EvalJs uses under the hood.
-  bool ExecJsInWebUI(std::string script) {
+  bool ExecJsInWebUI(const std::string& script) {
     return ExecJs(shell()->web_contents()->GetMainFrame(), script,
                   EXECUTE_SCRIPT_DEFAULT_OPTIONS, 1 /* world_id */);
   }
@@ -314,4 +314,26 @@ IN_PROC_BROWSER_TEST_F(ConversionInternalsWebUiBrowserTest,
   EXPECT_EQ(kSentTitle, sent_title_watcher.WaitAndGetTitle());
 }
 
+IN_PROC_BROWSER_TEST_F(ConversionInternalsWebUiBrowserTest,
+                       MojoJsBindingsCorrectlyScoped) {
+  EXPECT_TRUE(NavigateToURL(shell(), GURL(kConversionInternalsUrl)));
+
+  const base::string16 passed_title = base::ASCIIToUTF16("passed");
+
+  {
+    TitleWatcher sent_title_watcher(shell()->web_contents(), passed_title);
+    EXPECT_TRUE(
+        ExecJsInWebUI("document.title = window.Mojo? 'passed' : 'failed';"));
+    EXPECT_EQ(passed_title, sent_title_watcher.WaitAndGetTitle());
+  }
+
+  EXPECT_TRUE(NavigateToURL(shell(), GURL("about:blank")));
+  {
+    TitleWatcher sent_title_watcher(shell()->web_contents(), passed_title);
+    EXPECT_TRUE(
+        ExecJsInWebUI("document.title = window.Mojo? 'failed' : 'passed';"));
+    EXPECT_EQ(passed_title, sent_title_watcher.WaitAndGetTitle());
+  }
+}
+
 }  // namespace content

content/browser/conversions/conversion_internals_ui.h

@@ -19,15 +19,14 @@ namespace content {
 class ConversionInternalsHandlerImpl;
 
 // WebUI which handles serving the chrome://conversion-internals page.
-class CONTENT_EXPORT ConversionInternalsUI : public WebUIController,
-                                             public WebContentsObserver {
+class CONTENT_EXPORT ConversionInternalsUI : public WebUIController {
  public:
   explicit ConversionInternalsUI(WebUI* web_ui);
   ConversionInternalsUI(const ConversionInternalsUI& other) = delete;
   ConversionInternalsUI& operator=(const ConversionInternalsUI& other) = delete;
   ~ConversionInternalsUI() override;
 
-  // WebContentsObserver:
+  // WebUIController overrides:
   void RenderFrameCreated(RenderFrameHost* render_frame_host) override;
 
   void BindInterface(

content/browser/process_internals/process_internals_browsertest.cc

@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/strings/utf_string_conversions.h"
 #include "content/browser/child_process_security_policy_impl.h"
 #include "content/browser/renderer_host/render_process_host_impl.h"
 #include "content/public/test/browser_test.h"
@@ -13,7 +14,16 @@
 
 namespace content {
 
-class ProcessInternalsWebUiBrowserTest : public ContentBrowserTest {};
+class ProcessInternalsWebUiBrowserTest : public ContentBrowserTest {
+ protected:
+  // Executing javascript in the WebUI requires using an isolated world in which
+  // to execute the script because WebUI has a default CSP policy denying
+  // "eval()", which is what EvalJs uses under the hood.
+  bool ExecJsInWebUI(const std::string& script) {
+    return ExecJs(shell()->web_contents()->GetMainFrame(), script,
+                  EXECUTE_SCRIPT_DEFAULT_OPTIONS, 1 /* world_id */);
+  }
+};
 
 // This test verifies that loading of the process-internals WebUI works
 // correctly and the process rendering it has no WebUI bindings.
@@ -38,4 +48,26 @@ IN_PROC_BROWSER_TEST_F(ProcessInternalsWebUiBrowserTest, NoProcessBindings) {
   EXPECT_THAT(page_contents, ::testing::HasSubstr("Process Internals"));
 }
 
+IN_PROC_BROWSER_TEST_F(ProcessInternalsWebUiBrowserTest,
+                       MojoJsBindingsCorrectlyScoped) {
+  const base::string16 passed_title = base::ASCIIToUTF16("passed");
+
+  GURL url("chrome://process-internals/#web-contents");
+  EXPECT_TRUE(NavigateToURL(shell(), url));
+  {
+    TitleWatcher sent_title_watcher(shell()->web_contents(), passed_title);
+    EXPECT_TRUE(
+        ExecJsInWebUI("document.title = window.Mojo? 'passed' : 'failed';"));
+    EXPECT_EQ(passed_title, sent_title_watcher.WaitAndGetTitle());
+  }
+
+  EXPECT_TRUE(NavigateToURL(shell(), GURL("about:blank")));
+  {
+    TitleWatcher sent_title_watcher(shell()->web_contents(), passed_title);
+    EXPECT_TRUE(
+        ExecJsInWebUI("document.title = window.Mojo? 'failed' : 'passed';"));
+    EXPECT_EQ(passed_title, sent_title_watcher.WaitAndGetTitle());
+  }
+}
+
 }  // namespace content

content/browser/process_internals/process_internals_ui.cc

@@ -26,7 +26,7 @@
 namespace content {
 
 ProcessInternalsUI::ProcessInternalsUI(WebUI* web_ui)
-    : WebUIController(web_ui), WebContentsObserver(web_ui->GetWebContents()) {
+    : WebUIController(web_ui) {
   // This WebUI does not require any process bindings, so disable it early in
   // initialization time.
   web_ui->SetBindings(0);
@@ -44,7 +44,7 @@ ProcessInternalsUI::ProcessInternalsUI(WebUI* web_ui)
       network::mojom::CSPDirectiveName::TrustedTypes,
       "trusted-types cr-ui-tree-js-static;");
 
-  WebUIDataSource::Add(web_contents()->GetBrowserContext(), source);
+  WebUIDataSource::Add(web_ui->GetWebContents()->GetBrowserContext(), source);
 }
 
 WEB_UI_CONTROLLER_TYPE_IMPL(ProcessInternalsUI)

content/browser/process_internals/process_internals_ui.h

@@ -11,7 +11,6 @@
 
 #include "content/browser/process_internals/process_internals.mojom.h"
 #include "content/common/frame.mojom.h"
-#include "content/public/browser/web_contents_observer.h"
 #include "content/public/browser/web_ui_controller.h"
 #include "mojo/public/cpp/bindings/pending_receiver.h"
 
@@ -21,12 +20,12 @@ namespace content {
 // TODO(nasko): Change the inheritance of this class to be from
 // MojoWebUIController, so the registry_ can be removed and properly
 // inherited from common base class for Mojo WebUIs.
-class ProcessInternalsUI : public WebUIController, public WebContentsObserver {
+class ProcessInternalsUI : public WebUIController {
  public:
   explicit ProcessInternalsUI(WebUI* web_ui);
   ~ProcessInternalsUI() override;
 
-  // content::WebContentsObserver implementation.
+  // WebUIController overrides:
   void RenderFrameCreated(RenderFrameHost* render_frame_host) override;
 
   void BindProcessInternalsHandler(