[WebLayer] Allow executeScript() to run in main world

This adds a bool to BrowserController.executeScript() which allows
running the script in the main world instead of an isolate.

Change-Id: I0574de6493c99b187963217341c7e003fcfedb3e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1895320
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#712718}

content/browser/frame_host/render_frame_host_impl.cc

@@ -245,11 +245,9 @@ const void* const kRenderFrameHostAndroidKey = &kRenderFrameHostAndroidKey;
 // The next value to use for the accessibility reset token.
 int g_next_accessibility_reset_token = 1;
 
-#if defined(OS_ANDROID) || defined(OS_FUCHSIA) || defined(IS_CHROMECAST)
 // Whether to allow injecting javascript into any kind of frame, for Android
-// WebView, Fuchsia web.ContextProvider and CastOS content shell.
+// WebView, WebLayer, Fuchsia web.ContextProvider and CastOS content shell.
 bool g_allow_injecting_javascript = false;
-#endif
 
 typedef std::unordered_map<GlobalFrameRoutingId,
                            RenderFrameHostImpl*,
@@ -818,12 +816,10 @@ RenderFrameHost* RenderFrameHost::FromID(int render_process_id,
       GlobalFrameRoutingId(render_process_id, render_frame_id));
 }
 
-#if defined(OS_ANDROID) || defined(OS_FUCHSIA) || defined(IS_CHROMECAST)
 // static
 void RenderFrameHost::AllowInjectingJavaScript() {
   g_allow_injecting_javascript = true;
 }
-#endif  // defined(OS_ANDROID) || defined(OS_FUCHSIA) || defined(IS_CHROMECAST)
 
 // static
 RenderFrameHostImpl* RenderFrameHostImpl::FromID(int render_process_id,
@@ -6253,10 +6249,8 @@ bool RenderFrameHostImpl::CreateNetworkServiceDefaultFactoryInternal(
 }
 
 bool RenderFrameHostImpl::CanExecuteJavaScript() {
-#if defined(OS_ANDROID) || defined(OS_FUCHSIA) || defined(IS_CHROMECAST)
   if (g_allow_injecting_javascript)
     return true;
-#endif
   return !frame_tree_node_->current_url().is_valid() ||
          frame_tree_node_->current_url().SchemeIs(kChromeDevToolsScheme) ||
          ChildProcessSecurityPolicyImpl::GetInstance()->HasWebUIBindings(

content/public/browser/render_frame_host.h

@@ -79,12 +79,10 @@ class CONTENT_EXPORT RenderFrameHost : public IPC::Listener,
   // Returns nullptr if the IDs do not correspond to a live RenderFrameHost.
   static RenderFrameHost* FromID(int render_process_id, int render_frame_id);
 
-#if defined(OS_ANDROID) || defined(OS_FUCHSIA) || defined(IS_CHROMECAST)
   // Globally allows for injecting JavaScript into the main world. This feature
-  // is present only to support Android WebView, Fuchsia web.Contexts, and
-  // CastOS content shell. It must not be used in other configurations.
+  // is present only to support Android WebView, WebLayer, Fuchsia web.Contexts,
+  // and CastOS content shell. It must not be used in other configurations.
   static void AllowInjectingJavaScript();
-#endif
 
   // Returns a RenderFrameHost given its accessibility tree ID.
   static RenderFrameHost* FromAXTreeID(ui::AXTreeID ax_tree_id);

weblayer/browser/browser_controller_impl.cc

@@ -9,6 +9,7 @@
 #include "content/public/browser/file_select_listener.h"
 #include "content/public/browser/interstitial_page.h"
 #include "content/public/browser/navigation_controller.h"
+#include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/browser_controls_state.h"
@@ -178,9 +179,16 @@ NavigationController* BrowserControllerImpl::GetNavigationController() {
 }
 
 void BrowserControllerImpl::ExecuteScript(const base::string16& script,
+                                          bool use_separate_isolate,
                                           JavaScriptResultCallback callback) {
-  web_contents_->GetMainFrame()->ExecuteJavaScriptInIsolatedWorld(
-      script, std::move(callback), ISOLATED_WORLD_ID_WEBLAYER);
+  if (use_separate_isolate) {
+    web_contents_->GetMainFrame()->ExecuteJavaScriptInIsolatedWorld(
+        script, std::move(callback), ISOLATED_WORLD_ID_WEBLAYER);
+  } else {
+    content::RenderFrameHost::AllowInjectingJavaScript();
+    web_contents_->GetMainFrame()->ExecuteJavaScript(script,
+                                                     std::move(callback));
+  }
 }
 
 #if !defined(OS_ANDROID)
@@ -223,9 +231,11 @@ void BrowserControllerImpl::SetTopControlsContainerView(
 void BrowserControllerImpl::ExecuteScript(
     JNIEnv* env,
     const base::android::JavaParamRef<jstring>& script,
+    bool use_separate_isolate,
     const base::android::JavaParamRef<jobject>& callback) {
   base::android::ScopedJavaGlobalRef<jobject> jcallback(env, callback);
   ExecuteScript(base::android::ConvertJavaStringToUTF16(script),
+                use_separate_isolate,
                 base::BindOnce(&HandleJavaScriptResult, jcallback));
 }
 

weblayer/browser/browser_controller_impl.h

@@ -68,6 +68,7 @@ class BrowserControllerImpl : public BrowserController,
       jlong native_top_controls_container_view);
   void ExecuteScript(JNIEnv* env,
                      const base::android::JavaParamRef<jstring>& script,
+                     bool use_separate_isolate,
                      const base::android::JavaParamRef<jobject>& callback);
 #endif
 
@@ -83,6 +84,7 @@ class BrowserControllerImpl : public BrowserController,
   void RemoveObserver(BrowserObserver* observer) override;
   NavigationController* GetNavigationController() override;
   void ExecuteScript(const base::string16& script,
+                     bool use_separate_isolate,
                      JavaScriptResultCallback callback) override;
 #if !defined(OS_ANDROID)
   void AttachToView(views::WebView* web_view) override;

weblayer/browser/java/org/chromium/weblayer_private/BrowserControllerImpl.java

@@ -167,7 +167,7 @@ public void setFullscreenCallbackClient(IFullscreenCallbackClient client) {
     }
 
     @Override
-    public void executeScript(String script, IObjectWrapper callback) {
+    public void executeScript(String script, boolean useSeparateIsolate, IObjectWrapper callback) {
         Callback<String> nativeCallback = new Callback<String>() {
             @Override
             public void onResult(String result) {
@@ -179,7 +179,7 @@ public void onResult(String result) {
             }
         };
         BrowserControllerImplJni.get().executeScript(
-                mNativeBrowserController, script, nativeCallback);
+                mNativeBrowserController, script, useSeparateIsolate, nativeCallback);
     }
 
     public void destroy() {
@@ -222,7 +222,7 @@ void setTopControlsContainerView(long nativeBrowserControllerImpl,
                 BrowserControllerImpl caller, long nativeTopControlsContainerView);
         void deleteBrowserController(long browserController);
         WebContents getWebContents(long nativeBrowserControllerImpl, BrowserControllerImpl caller);
-        void executeScript(
-                long nativeBrowserControllerImpl, String script, Callback<String> callback);
+        void executeScript(long nativeBrowserControllerImpl, String script,
+                boolean useSeparateIsolate, Callback<String> callback);
     }
 }

weblayer/browser/java/org/chromium/weblayer_private/aidl/IBrowserController.aidl

@@ -18,5 +18,5 @@ interface IBrowserController {
 
   void setFullscreenCallbackClient(in IFullscreenCallbackClient client) = 3;
 
-  void executeScript(in String script, in IObjectWrapper callback) = 4;
+  void executeScript(in String script, boolean useSeparateIsolate, in IObjectWrapper callback) = 4;
 }

weblayer/browser/webui/webui_browsertest.cc

@@ -16,7 +16,8 @@ IN_PROC_BROWSER_TEST_F(WebLayerWebUIBrowserTest, WebUI) {
   base::RunLoop run_loop;
   bool result =
       ExecuteScript(shell(),
-                    "document.getElementById('remote-debug-label').hidden")
+                    "document.getElementById('remote-debug-label').hidden",
+                    true /* use_separate_isolate */)
           .GetBool();
   // The remote debug checkbox should only be visible on Android.
 #if defined(OS_ANDROID)

weblayer/public/browser_controller.h

@@ -58,7 +58,16 @@ class BrowserController {
   virtual NavigationController* GetNavigationController() = 0;
 
   using JavaScriptResultCallback = base::OnceCallback<void(base::Value)>;
+
+  // Executes the script, and returns the result to the callback if provided. If
+  // |use_separate_isolate| is true, runs the script in a separate v8 Isolate.
+  // This uses more memory, but separates the injected scrips from scripts in
+  // the page. This prevents any potentially malicious interaction between
+  // first-party scripts in the page, and injected scripts. Use with caution,
+  // only pass false for this argument if you know this isn't an issue or you
+  // need to interact with first-party scripts.
   virtual void ExecuteScript(const base::string16& script,
+                             bool use_separate_isolate,
                              JavaScriptResultCallback callback) = 0;
 
 #if !defined(OS_ANDROID)

weblayer/public/java/org/chromium/weblayer/BrowserController.java

@@ -84,12 +84,17 @@ public DownloadCallback getDownloadCallback() {
     }
 
     /**
-     * Executes the script in an isolated world, and returns the result as a JSON object to the
-     * callback if provided. The object passed to the callback will have a single key
-     * SCRIPT_RESULT_KEY which will hold the result of running the script.
+     * Executes the script, and returns the result as a JSON object to the callback if provided. The
+     * object passed to the callback will have a single key SCRIPT_RESULT_KEY which will hold the
+     * result of running the script.
+     * @param useSeparateIsolate If true, runs the script in a separate v8 Isolate. This uses more
+     * memory, but separates the injected scrips from scripts in the page. This prevents any
+     * potentially malicious interaction between first-party scripts in the page, and injected
+     * scripts. Use with caution, only pass false for this argument if you know this isn't an issue
+     * or you need to interact with first-party scripts.
      */
-    public void executeScript(
-            @NonNull String script, @Nullable ValueCallback<JSONObject> callback) {
+    public void executeScript(@NonNull String script, boolean useSeparateIsolate,
+            @Nullable ValueCallback<JSONObject> callback) {
         ThreadCheck.ensureOnUiThread();
         try {
             ValueCallback<String> stringCallback = (String result) -> {
@@ -105,7 +110,7 @@ public void executeScript(
                     throw new RuntimeException(e);
                 }
             };
-            mImpl.executeScript(script, ObjectWrapper.wrap(stringCallback));
+            mImpl.executeScript(script, useSeparateIsolate, ObjectWrapper.wrap(stringCallback));
         } catch (RemoteException e) {
             throw new APICallException(e);
         }

weblayer/shell/android/javatests/src/org/chromium/weblayer/test/ExecuteScriptTest.java

@@ -34,36 +34,49 @@ public class ExecuteScriptTest {
     @SmallTest
     public void testBasicScript() throws Exception {
         InstrumentationActivity activity = mActivityTestRule.launchShellWithUrl(DATA_URL);
-        JSONObject result = mActivityTestRule.executeScriptSync("document.body.innerHTML");
+        JSONObject result = mActivityTestRule.executeScriptSync(
+                "document.body.innerHTML", true /* useSeparateIsolate */);
         Assert.assertEquals(result.getString(BrowserController.SCRIPT_RESULT_KEY), "foo");
     }
 
     @Test
     @SmallTest
     public void testScriptIsolatedFromPage() throws Exception {
         InstrumentationActivity activity = mActivityTestRule.launchShellWithUrl(DATA_URL);
-        JSONObject result = mActivityTestRule.executeScriptSync("bar");
+        JSONObject result =
+                mActivityTestRule.executeScriptSync("bar", true /* useSeparateIsolate */);
         Assert.assertTrue(result.isNull(BrowserController.SCRIPT_RESULT_KEY));
     }
 
+    @Test
+    @SmallTest
+    public void testMainWorldScriptNotIsolatedFromPage() throws Exception {
+        InstrumentationActivity activity = mActivityTestRule.launchShellWithUrl(DATA_URL);
+        JSONObject result =
+                mActivityTestRule.executeScriptSync("bar", false /* useSeparateIsolate */);
+        Assert.assertEquals(result.getInt(BrowserController.SCRIPT_RESULT_KEY), 10);
+    }
+
     @Test
     @SmallTest
     public void testScriptNotIsolatedFromOtherScript() throws Exception {
         InstrumentationActivity activity = mActivityTestRule.launchShellWithUrl(DATA_URL);
-        mActivityTestRule.executeScriptSync("var foo = 20;");
-        JSONObject result = mActivityTestRule.executeScriptSync("foo");
+        mActivityTestRule.executeScriptSync("var foo = 20;", true /* useSeparateIsolate */);
+        JSONObject result =
+                mActivityTestRule.executeScriptSync("foo", true /* useSeparateIsolate */);
         Assert.assertEquals(result.getInt(BrowserController.SCRIPT_RESULT_KEY), 20);
     }
 
     @Test
     @SmallTest
     public void testClearedOnNavigate() throws Exception {
         InstrumentationActivity activity = mActivityTestRule.launchShellWithUrl(DATA_URL);
-        mActivityTestRule.executeScriptSync("var foo = 20;");
+        mActivityTestRule.executeScriptSync("var foo = 20;", true /* useSeparateIsolate */);
 
         String newUrl = UrlUtils.encodeHtmlDataUri("<html></html>");
         mActivityTestRule.navigateAndWait(newUrl);
-        JSONObject result = mActivityTestRule.executeScriptSync("foo");
+        JSONObject result =
+                mActivityTestRule.executeScriptSync("foo", true /* useSeparateIsolate */);
         Assert.assertTrue(result.isNull(BrowserController.SCRIPT_RESULT_KEY));
     }
 
@@ -73,9 +86,10 @@ public void testNullCallback() throws Exception {
         InstrumentationActivity activity = mActivityTestRule.launchShellWithUrl(DATA_URL);
         TestThreadUtils.runOnUiThreadBlocking(() -> {
             // Null callback should not crash.
-            activity.getBrowserController().executeScript("null", null);
+            activity.getBrowserController().executeScript(
+                    "null", true /* useSeparateIsolate */, null);
         });
         // Execute a sync script to make sure the other script finishes.
-        mActivityTestRule.executeScriptSync("null");
+        mActivityTestRule.executeScriptSync("null", true /* useSeparateIsolate */);
     }
 }

weblayer/shell/android/javatests/src/org/chromium/weblayer/test/InputTypesTest.java

@@ -307,7 +307,8 @@ public void testFileInputAudio() {
     public void testColorInput() {
         // Just make sure we don't crash when opening the color picker.
         mActivityTestRule.executeScriptSync("var done = false; document.onclick = function() {"
-                + "document.getElementById('input_color').click(); done = true;}");
+                        + "document.getElementById('input_color').click(); done = true;}",
+                true /* useSeparateIsolate */);
         EventUtils.simulateTouchCenterOfView(
                 mActivityTestRule.getActivity().getWindow().getDecorView());
         CriteriaHelper.pollInstrumentationThread(
@@ -318,7 +319,8 @@ private void openFileInputWithId(String id) {
         // We need to click the input after user input, otherwise it won't open due to security
         // policy.
         mActivityTestRule.executeScriptSync(
-                "document.onclick = function() {document.getElementById('" + id + "').click()}");
+                "document.onclick = function() {document.getElementById('" + id + "').click()}",
+                true /* useSeparateIsolate */);
         EventUtils.simulateTouchCenterOfView(
                 mActivityTestRule.getActivity().getWindow().getDecorView());
         mIntentInterceptor.waitForIntent();

weblayer/shell/android/javatests/src/org/chromium/weblayer/test/InstrumentationActivityTestRule.java

@@ -203,12 +203,12 @@ public void recreateActivity() {
     /**
      * Executes the script passed in and waits for the result.
      */
-    public JSONObject executeScriptSync(String script) {
+    public JSONObject executeScriptSync(String script, boolean useSeparateIsolate) {
         JSONCallbackHelper callbackHelper = new JSONCallbackHelper();
         int count = callbackHelper.getCallCount();
         TestThreadUtils.runOnUiThreadBlocking(() -> {
-            getActivity().getBrowserController().executeScript(
-                    script, (JSONObject result) -> { callbackHelper.notifyCalled(result); });
+            getActivity().getBrowserController().executeScript(script, useSeparateIsolate,
+                    (JSONObject result) -> { callbackHelper.notifyCalled(result); });
         });
         try {
             callbackHelper.waitForCallback(count);
@@ -220,23 +220,26 @@ public JSONObject executeScriptSync(String script) {
 
     public int executeScriptAndExtractInt(String script) {
         try {
-            return executeScriptSync(script).getInt(BrowserController.SCRIPT_RESULT_KEY);
+            return executeScriptSync(script, true /* useSeparateIsolate */)
+                    .getInt(BrowserController.SCRIPT_RESULT_KEY);
         } catch (JSONException e) {
             throw new RuntimeException(e);
         }
     }
 
     public String executeScriptAndExtractString(String script) {
         try {
-            return executeScriptSync(script).getString(BrowserController.SCRIPT_RESULT_KEY);
+            return executeScriptSync(script, true /* useSeparateIsolate */)
+                    .getString(BrowserController.SCRIPT_RESULT_KEY);
         } catch (JSONException e) {
             throw new RuntimeException(e);
         }
     }
 
     public boolean executeScriptAndExtractBoolean(String script) {
         try {
-            return executeScriptSync(script).getBoolean(BrowserController.SCRIPT_RESULT_KEY);
+            return executeScriptSync(script, true /* useSeparateIsolate */)
+                    .getBoolean(BrowserController.SCRIPT_RESULT_KEY);
         } catch (JSONException e) {
             throw new RuntimeException(e);
         }

weblayer/shell/android/javatests/src/org/chromium/weblayer/test/NavigationTest.java

@@ -241,7 +241,8 @@ public void testSameDocument() throws Exception {
 
         int curCompletedCount = mCallback.onCompletedCallback.getCallCount();
 
-        mActivityTestRule.executeScriptSync("history.pushState(null, '', '#bar');");
+        mActivityTestRule.executeScriptSync(
+                "history.pushState(null, '', '#bar');", true /* useSeparateIsolate */);
 
         mCallback.onCompletedCallback.assertCalledWith(
                 curCompletedCount, "data:text,foo#bar", true);

weblayer/test/weblayer_browser_test_utils.cc

@@ -98,11 +98,13 @@ void NavigateAndWaitForFailure(const GURL& url, Shell* shell) {
       url, shell, TestNavigationObserver::NavigationEventToObserve::Failure);
 }
 
-base::Value ExecuteScript(Shell* shell, const std::string& script) {
+base::Value ExecuteScript(Shell* shell,
+                          const std::string& script,
+                          bool use_separate_isolate) {
   base::Value final_result;
   base::RunLoop run_loop;
   shell->browser_controller()->ExecuteScript(
-      base::ASCIIToUTF16(script),
+      base::ASCIIToUTF16(script), use_separate_isolate,
       base::BindLambdaForTesting(
           [&run_loop, &final_result](base::Value result) {
             final_result = std::move(result);

weblayer/test/weblayer_browser_test_utils.h

@@ -19,7 +19,9 @@ void NavigateAndWaitForCompletion(const GURL& url, Shell* shell);
 void NavigateAndWaitForFailure(const GURL& url, Shell* shell);
 
 // Executes |script| in |shell| and returns the result.
-base::Value ExecuteScript(Shell* shell, const std::string& script);
+base::Value ExecuteScript(Shell* shell,
+                          const std::string& script,
+                          bool use_separate_isolate);
 
 }  // namespace weblayer