[iOS] Move base, common, and message JS out of js_compile.gni

The base, common, and message JS files were previously always included
for every js_compile_checked gn target even though all compiled scripts
do not rely on functions defined in these files.

In this CL, the script dependencies are split apart in order to more
explicitly list JavaScript dependencies. This is done in preparation
of moving these shared scripts to a new architecture in order to
support isolated worlds on iOS 14 and later.

Additionally, improve script error logging messages which were useful
while debugging script execution.

Bug: 1042335
Change-Id: Iee9adef123c32bb69ebb8b3c9bb1fc58878f04f5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2554900Reviewed-by: John Wu <jzw@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Commit-Queue: Mike Dougherty <michaeldo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#835692}

components/translate/ios/browser/BUILD.gn

@@ -48,6 +48,12 @@ js_compile_checked("injected_js") {
     "resources/language_detection.js",
     "resources/translate_ios.js",
   ]
+
+  js_modules = [
+    "//ios/web/web_state/js/resources/base.js",
+    "//ios/web/web_state/js/resources/common.js",
+    "//ios/web/js_messaging/resources/message.js",
+  ]
 }
 
 source_set("unit_tests") {

components/translate/ios/browser/resources/translate_ios.js

@@ -10,7 +10,7 @@
  * @suppress {checkTypes, checkVars}
  */
 
-goog.require('__crWeb.base');
+// Requires __crWeb.base
 
 /**
  * Namespace for this module.

ios/chrome/browser/search_engines/BUILD.gn

@@ -101,4 +101,10 @@ source_set("unit_tests") {
 
 js_compile_checked("search_engine_js") {
   sources = [ "resources/search_engine.js" ]
+
+  js_modules = [
+    "//ios/web/web_state/js/resources/base.js",
+    "//ios/web/web_state/js/resources/common.js",
+    "//ios/web/js_messaging/resources/message.js",
+  ]
 }

ios/chrome/browser/web/BUILD.gn

@@ -203,10 +203,18 @@ js_compile_bundle("chrome_bundle_main_frame") {
 
 js_compile_checked("accessibility") {
   sources = [ "resources/accessibility.js" ]
+
+  js_modules = [ "//ios/web/web_state/js/resources/base.js" ]
 }
 
 js_compile_checked("image_fetch") {
   sources = [ "resources/image_fetch.js" ]
+
+  js_modules = [
+    "//ios/web/web_state/js/resources/base.js",
+    "//ios/web/web_state/js/resources/common.js",
+    "//ios/web/js_messaging/resources/message.js",
+  ]
 }
 
 source_set("java_script_console") {

ios/web/BUILD.gn

@@ -32,8 +32,11 @@ source_set("web") {
   ]
 
   deps = [
+    ":base_js",
+    ":common_js",
     ":core",
     ":js_resources",
+    ":message_js",
     ":navigation_resources",
     ":resources",
     ":threads",
@@ -608,11 +611,8 @@ js_compile_bundle("all_frames_web_bundle") {
   closure_entry_point = "__crWeb.allFramesWebBundle"
 
   sources = [
-    "js_messaging/resources/message.js",
     "web_state/js/resources/all_frames_context_menu.js",
     "web_state/js/resources/all_frames_web_bundle.js",
-    "web_state/js/resources/base.js",
-    "web_state/js/resources/common.js",
     "web_state/js/resources/cookie.js",
     "web_state/js/resources/find_in_page.js",
     "web_state/js/resources/share_workaround.js",
@@ -640,6 +640,27 @@ js_compile_bundle("main_frame_document_end_web_bundle") {
   ]
 }
 
+js_compile_bundle("base_js") {
+  visibility = [ ":web" ]
+  closure_entry_point = "__crWeb.base"
+
+  sources = [ "web_state/js/resources/base.js" ]
+}
+
+js_compile_bundle("common_js") {
+  visibility = [ ":web" ]
+  closure_entry_point = "__crWeb.common"
+
+  sources = [ "web_state/js/resources/common.js" ]
+}
+
+js_compile_bundle("message_js") {
+  visibility = [ ":web" ]
+  closure_entry_point = "__crWeb.message"
+
+  sources = [ "js_messaging/resources/message.js" ]
+}
+
 js_compile_checked("js_resources") {
   public_deps = [
     ":all_frames_document_end_web_bundle",
@@ -648,6 +669,12 @@ js_compile_checked("js_resources") {
     ":main_frame_web_bundle",
   ]
 
+  js_modules = [
+    "//ios/web/web_state/js/resources/base.js",
+    "//ios/web/web_state/js/resources/common.js",
+    "//ios/web/js_messaging/resources/message.js",
+  ]
+
   sources = [ "js_messaging/resources/window_id.js" ]
 }
 

ios/web/js_compile.gni

@@ -125,11 +125,7 @@ template("js_compile_checked") {
       forward_variables_from(invoker, [ "testonly" ])
       script = closure_compiler_path
       sources = invoker.sources
-      _js_modules = [
-        "//ios/web/web_state/js/resources/base.js",
-        "//ios/web/web_state/js/resources/common.js",
-        "//ios/web/js_messaging/resources/message.js",
-      ]
+      _js_modules = []
       if (defined(invoker.js_modules)) {
         _js_modules += invoker.js_modules
       }

ios/web/js_messaging/crw_js_window_id_manager.mm

@@ -63,6 +63,7 @@ const size_t kUniqueKeyLength = 16;
                if (error) {
                  DCHECK(error.code == WKErrorWebViewInvalidated ||
                         error.code == WKErrorWebContentProcessTerminated)
+                     << scriptWithResult << " failed with error "
                      << base::SysNSStringToUTF8(error.description);
                  return;
                }

ios/web/js_messaging/crw_js_window_id_manager_unittest.mm

@@ -16,11 +16,33 @@
 #error "This file requires ARC support."
 #endif
 
+namespace {
+
+void AddSharedScriptsToWebView(WKWebView* web_view) {
+  // Scripts must be all injected at once because as soon as __gCrWeb exists,
+  // injection is assumed to be done and __gCrWeb.message is used.
+  NSString* scripts = [NSString
+      stringWithFormat:@"%@; %@; %@", web::test::GetPageScript(@"base_js"),
+                       web::test::GetPageScript(@"common_js"),
+                       web::test::GetPageScript(@"message_js")];
+  web::test::ExecuteJavaScript(web_view, scripts);
+}
+
+}  // namespace
+
 namespace web {
 
 // Test fixture for testing CRWJSWindowIDManager class.
 class JSWindowIDManagerTest : public PlatformTest {
  protected:
+  // Creates and returns a WKWebView instance configured with the necessary
+  // shared scripts.
+  WKWebView* CreateWebView() {
+    WKWebView* web_view = [[WKWebView alloc] init];
+    AddSharedScriptsToWebView(web_view);
+    return web_view;
+  }
+
   TestBrowserState browser_state_;
 };
 
@@ -28,9 +50,7 @@ class JSWindowIDManagerTest : public PlatformTest {
 // window ID.
 TEST_F(JSWindowIDManagerTest, WindowIDDifferentManager) {
   // Inject the first manager.
-  WKWebView* web_view = [[WKWebView alloc] init];
-  test::ExecuteJavaScript(web_view,
-                          GetDocumentStartScriptForAllFrames(&browser_state_));
+  WKWebView* web_view = CreateWebView();
 
   CRWJSWindowIDManager* manager =
       [[CRWJSWindowIDManager alloc] initWithWebView:web_view];
@@ -39,9 +59,7 @@ TEST_F(JSWindowIDManagerTest, WindowIDDifferentManager) {
               test::ExecuteJavaScript(web_view, @"window.__gCrWeb.windowId"));
 
   // Inject the second manager.
-  WKWebView* web_view2 = [[WKWebView alloc] init];
-  test::ExecuteJavaScript(web_view2,
-                          GetDocumentStartScriptForAllFrames(&browser_state_));
+  WKWebView* web_view2 = CreateWebView();
 
   CRWJSWindowIDManager* manager2 =
       [[CRWJSWindowIDManager alloc] initWithWebView:web_view2];
@@ -55,9 +73,7 @@ TEST_F(JSWindowIDManagerTest, WindowIDDifferentManager) {
 
 // Tests that injecting multiple times creates a new window ID.
 TEST_F(JSWindowIDManagerTest, MultipleInjections) {
-  WKWebView* web_view = [[WKWebView alloc] init];
-  test::ExecuteJavaScript(web_view,
-                          GetDocumentStartScriptForAllFrames(&browser_state_));
+  WKWebView* web_view = CreateWebView();
 
   // First injection.
   CRWJSWindowIDManager* manager =
@@ -86,8 +102,7 @@ TEST_F(JSWindowIDManagerTest, InjectionRetry) {
   EXPECT_FALSE(test::ExecuteJavaScript(web_view, @"window.__gCrWeb"));
 
   // Now inject window.__gCrWeb and check if window ID injection retried.
-  test::ExecuteJavaScript(web_view,
-                          GetDocumentStartScriptForAllFrames(&browser_state_));
+  AddSharedScriptsToWebView(web_view);
   EXPECT_NSEQ([manager windowID],
               test::ExecuteJavaScript(web_view, @"window.__gCrWeb.windowId"));
 }

ios/web/js_messaging/page_script_util.mm

@@ -112,8 +112,11 @@ NSString* GetDocumentStartScriptForAllFrames(BrowserState* browser_state) {
   web_bundle =
       [web_bundle stringByReplacingOccurrencesOfString:@"$(COOKIE_STATE)"
                                             withString:injectedCookieState];
-  NSString* script =
-      [NSString stringWithFormat:@"%@; %@", web_bundle, embedder_page_script];
+  NSString* script = [NSString
+      stringWithFormat:@"%@; %@; %@; %@; %@", GetPageScript(@"base_js"),
+                       GetPageScript(@"common_js"),
+                       GetPageScript(@"message_js"), web_bundle,
+                       embedder_page_script];
   return MakeScriptInjectableOnce(@"start_all_frames", script);
 }
 

ios/web/js_messaging/resources/message.js

@@ -9,8 +9,7 @@
 
 goog.provide('__crWeb.message');
 
-goog.require('__crWeb.base');
-goog.require('__crWeb.common');
+// Requires __crWeb.base and __crWeb.common.
 
 /**
  * Namespace for this module.

ios/web/public/test/web_test_with_web_state.mm

@@ -229,14 +229,18 @@ id WebTestWithWebState::ExecuteJavaScript(NSString* script) {
         // Most of executed JS does not return the result, and there is no need
         // to log WKErrorJavaScriptResultTypeIsUnsupported error code.
         if (error && error.code != WKErrorJavaScriptResultTypeIsUnsupported) {
-          DLOG(WARNING) << base::SysNSStringToUTF8(error.localizedDescription);
+          DLOG(WARNING) << "Script execution of:" << script
+                        << "\nfailed with error: "
+                        << base::SysNSStringToUTF8(error.description);
         }
         execution_result = [result copy];
         execution_completed = true;
       }];
-  EXPECT_TRUE(WaitUntilConditionOrTimeout(kWaitForJSCompletionTimeout, ^{
-    return execution_completed;
-  }));
+  EXPECT_TRUE(WaitUntilConditionOrTimeout(kWaitForJSCompletionTimeout,
+                                          ^{
+                                            return execution_completed;
+                                          }))
+      << "Timed out trying to execute: " << script;
 
   return execution_result;
 }

ios/web/web_state/js/resources/all_frames_context_menu.js

@@ -8,8 +8,7 @@
 
 goog.provide('__crWeb.allFramesContextMenu');
 
-goog.require('__crWeb.base');
-goog.require('__crWeb.common');
+// Requires __crWeb.base and __crWeb.common
 
 /** Beginning of anonymous object */
 (function() {

ios/web/web_state/js/resources/all_frames_web_bundle.js

@@ -5,10 +5,8 @@
 // Set of scripts required by web layer backed up by WKWebView.
 goog.provide('__crWeb.allFramesWebBundle');
 
+// Requires __crWeb.base, __crWeb.common, and __crWeb.message
 goog.require('__crWeb.allFramesContextMenu');
-goog.require('__crWeb.base');
-goog.require('__crWeb.common');
 goog.require('__crWeb.cookie');
 goog.require('__crWeb.findInPage');
-goog.require('__crWeb.message');
 goog.require('__crWeb.shareWorkaround');

ios/web/web_state/js/resources/common.js

@@ -6,7 +6,7 @@
 
 goog.provide('__crWeb.common');
 
-goog.require('__crWeb.base');
+// Requires __crWeb.base.
 
 /** @typedef {HTMLInputElement|HTMLTextAreaElement|HTMLSelectElement} */
 var FormControlElement;

ios/web/web_state/js/resources/cookie.js

@@ -6,7 +6,7 @@
  */
 goog.provide('__crWeb.cookie');
 
-goog.require('__crWeb.message');
+//Requires __crWeb.message
 
 /* Beginning of anonymous namespace. */
 (function() {

ios/web/web_state/js/resources/find_in_page.js

@@ -4,7 +4,7 @@
 
 goog.provide('__crWeb.findInPage');
 
-goog.require('__crWeb.base');
+// Requires __crWeb.base
 
 /**
  * Based on code from the Google iOS app.

ios/web/web_state/ui/wk_web_view_configuration_provider_unittest.mm

@@ -22,6 +22,22 @@
 #error "This file requires ARC support."
 #endif
 
+namespace {
+
+// Returns the WKUserScript from |user_scripts| which contains |script_string|
+// or null if no such script is found.
+WKUserScript* FindWKUserScriptContaining(NSArray<WKUserScript*>* user_scripts,
+                                         NSString* script_string) {
+  for (WKUserScript* user_script in user_scripts) {
+    if ([user_script.source containsString:script_string]) {
+      return user_script;
+    }
+  }
+  return nil;
+}
+
+}  // namespace
+
 namespace web {
 namespace {
 
@@ -145,30 +161,32 @@ TEST_F(WKWebViewConfigurationProviderTest, Purge) {
 // Tests that configuration's userContentController has only one script with the
 // same content as web::GetDocumentStartScriptForMainFrame() returns.
 TEST_F(WKWebViewConfigurationProviderTest, UserScript) {
-  WKWebViewConfiguration* config = GetProvider().GetWebViewConfiguration();
-  NSArray* scripts = config.userContentController.userScripts;
-  ASSERT_EQ(4U, scripts.count);
-  EXPECT_FALSE(((WKUserScript*)[scripts objectAtIndex:0]).isForMainFrameOnly);
-  EXPECT_TRUE(((WKUserScript*)[scripts objectAtIndex:1]).isForMainFrameOnly);
-  EXPECT_FALSE(((WKUserScript*)[scripts objectAtIndex:2]).isForMainFrameOnly);
-  EXPECT_TRUE(((WKUserScript*)[scripts objectAtIndex:3]).isForMainFrameOnly);
-  NSString* early_all_frames_script =
-      GetDocumentStartScriptForAllFrames(&browser_state_);
-  NSString* main_frame_script =
-      GetDocumentStartScriptForMainFrame(&browser_state_);
-  NSString* late_all_frames_script =
-      GetDocumentEndScriptForAllFrames(&browser_state_);
-  NSString* late_main_frame_script =
-      GetDocumentEndScriptForMainFrame(&browser_state_);
-  // The scripts in |userScrips| are wrapped with a "if (!injected)" check to
-  // avoid double injections, so a substring check is necessary.
-  EXPECT_LT(0U,
-            [[scripts[0] source] rangeOfString:early_all_frames_script].length);
-  EXPECT_LT(0U, [[scripts[1] source] rangeOfString:main_frame_script].length);
-  EXPECT_LT(0U,
-            [[scripts[2] source] rangeOfString:late_all_frames_script].length);
-  EXPECT_LT(0U,
-            [[scripts[3] source] rangeOfString:late_main_frame_script].length);
+  WKUserContentController* user_content_controller =
+      GetProvider().GetWebViewConfiguration().userContentController;
+
+  WKUserScript* early_all_user_script = FindWKUserScriptContaining(
+      user_content_controller.userScripts,
+      GetDocumentStartScriptForAllFrames(&browser_state_));
+  ASSERT_TRUE(early_all_user_script);
+  EXPECT_FALSE(early_all_user_script.isForMainFrameOnly);
+
+  WKUserScript* main_frame_script = FindWKUserScriptContaining(
+      user_content_controller.userScripts,
+      GetDocumentStartScriptForMainFrame(&browser_state_));
+  ASSERT_TRUE(main_frame_script);
+  EXPECT_TRUE(main_frame_script.isForMainFrameOnly);
+
+  WKUserScript* late_all_frames_script = FindWKUserScriptContaining(
+      user_content_controller.userScripts,
+      GetDocumentEndScriptForAllFrames(&browser_state_));
+  ASSERT_TRUE(late_all_frames_script);
+  EXPECT_FALSE(late_all_frames_script.isForMainFrameOnly);
+
+  WKUserScript* late_main_frame_script = FindWKUserScriptContaining(
+      user_content_controller.userScripts,
+      GetDocumentEndScriptForMainFrame(&browser_state_));
+  ASSERT_TRUE(late_main_frame_script);
+  EXPECT_TRUE(late_main_frame_script.isForMainFrameOnly);
 }
 
 // Tests that configuration's userContentController has different scripts after
@@ -177,33 +195,32 @@ TEST_F(WKWebViewConfigurationProviderTest, UpdateScripts) {
   TestWebClient* client = GetWebClient();
   client->SetEarlyPageScript(@"var test = 4;");
 
-  WKWebViewConfiguration* config = GetProvider().GetWebViewConfiguration();
-  NSArray* scripts = config.userContentController.userScripts;
-  ASSERT_EQ(4U, scripts.count);
+  WKUserContentController* user_content_controller =
+      GetProvider().GetWebViewConfiguration().userContentController;
 
-  WKUserScript* initial_main_frame_wkscript = scripts[1];
   NSString* initial_main_frame_script =
       GetDocumentStartScriptForMainFrame(&browser_state_);
-  EXPECT_LT(0U, [[initial_main_frame_wkscript source]
-                    rangeOfString:initial_main_frame_script]
-                    .length);
+  WKUserScript* initial_script = FindWKUserScriptContaining(
+      user_content_controller.userScripts, initial_main_frame_script);
+  EXPECT_TRUE(initial_script);
 
   client->SetEarlyPageScript(@"var test = 3;");
   GetProvider().UpdateScripts();
 
-  WKUserScript* updated_main_frame_wkscript = scripts[1];
   NSString* updated_main_frame_script =
       GetDocumentStartScriptForMainFrame(&browser_state_);
+  WKUserScript* updated_script = FindWKUserScriptContaining(
+      user_content_controller.userScripts, updated_main_frame_script);
+  EXPECT_TRUE(updated_script);
 
   EXPECT_NE(updated_main_frame_script, initial_main_frame_script);
-  EXPECT_NE([initial_main_frame_wkscript source],
-            [updated_main_frame_wkscript source]);
-  EXPECT_LT(0U, [[updated_main_frame_wkscript source]
-                    rangeOfString:updated_main_frame_script]
-                    .length);
-  EXPECT_EQ(0U, [[initial_main_frame_wkscript source]
-                    rangeOfString:updated_main_frame_script]
-                    .length);
+  EXPECT_NE(initial_script.source, updated_script.source);
+  EXPECT_LT(
+      0U,
+      [updated_script.source rangeOfString:updated_main_frame_script].length);
+  EXPECT_EQ(
+      0U,
+      [initial_script.source rangeOfString:updated_main_frame_script].length);
 }
 
 // Tests that observers methods are correctly triggered when observing the

ios/web/web_state/web_state_impl.mm

@@ -695,7 +695,8 @@ void WebStateImpl::ExecuteJavaScript(const base::string16& javascript,
       executeJavaScript:base::SysUTF16ToNSString(javascript)
       completionHandler:^(id value, NSError* error) {
         if (error) {
-          DLOG(WARNING) << "Script execution has failed: "
+          DLOG(WARNING) << "Script execution of:" << javascript
+                        << "\nfailed with error: "
                         << base::SysNSStringToUTF16(
                                error.userInfo[NSLocalizedDescriptionKey]);
         }