Separate content shell from web test common and renderer code.

Move kRunWebTests up to shell_switches.h as it is a switch to control
content shell, causing it to inject web test code.

Introduce DEPS rule to disallow content/shell/ to use
  content/shell/common/web_test
  content/shell/renderer/web_test

Allow content/shell/test_runner to still access these, as it is part
of web test code.

Allow content/shell/app/shell_main_delegate.cc to access
  content/shell/renderer/web_test/
  content/shell/browser/web_test/
in order to inject web test behaviour into the application at run-time.

R=nasko@chromium.org

Bug: 866140
Change-Id: I3c0ca417b537506f6d7fad55c4a1bb5e495d94c8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2122462
Commit-Queue: danakj <danakj@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#754956}

content/public/test/browser_test_base.cc

@@ -266,7 +266,7 @@ void BrowserTestBase::SetUp() {
 
   // The layout of windows on screen is unpredictable during tests, so disable
   // occlusion when running browser tests.
-  base::CommandLine::ForCurrentProcess()->AppendSwitch(
+  command_line->AppendSwitch(
       switches::kDisableBackgroundingOccludedWindowsForTesting);
 
 #if defined(USE_AURA)

content/shell/DEPS

@@ -43,6 +43,13 @@ include_rules = [
 
   # Access to the process specific switches.
   "+services/service_manager/embedder",
+
+  # Separating content shell and web test code. Only narrow parts of content
+  # shell may access and inject web test code at runtime.
+  # TODO(danakj): Drop browser/web_test too.
+  "-content/shell/common/web_test",
+  "-content/shell/renderer/web_test",
+  "-content/test_runner",
 ]
 
 specific_include_rules = {

content/shell/app/DEPS

+include_rules = [
+]
+
+specific_include_rules = {
+  "shell_main_delegate\.cc": [
+    # Separating Content Shell and web test code. ShellMainDelegate injects web test
+    # code into Content Shell at runtime.
+    "+content/shell/browser/web_test",
+    "+content/shell/renderer/web_test",
+  ],
+}

content/shell/app/shell_main_delegate.cc

@@ -28,7 +28,6 @@
 #include "content/shell/browser/web_test/web_test_content_browser_client.h"
 #include "content/shell/common/shell_content_client.h"
 #include "content/shell/common/shell_switches.h"
-#include "content/shell/common/web_test/web_test_switches.h"
 #include "content/shell/gpu/shell_content_gpu_client.h"
 #include "content/shell/renderer/shell_content_renderer_client.h"
 #include "content/shell/renderer/web_test/web_test_content_renderer_client.h"

content/shell/browser/DEPS

@@ -7,6 +7,14 @@ include_rules = [
   "+services/service_manager/public/cpp",
   "+services/service_manager/sandbox",
   "+ui/ozone/public",
+
+  # Separating Content Shell and web test code.
+  # TODO(danakj): This directory shouldn't need to access web test code as
+  # the ShellMainDelegate should inject it as appropriate.
+  "+content/shell/browser/web_test",
+  "+content/shell/common/web_test/web_test_switches.h",
+  "+content/shell/renderer/web_test",
+  "+content/shell/test_runner/web_test_delegate.h",
 ]
 
 specific_include_rules = {

content/shell/browser/shell.cc

@@ -95,12 +95,6 @@ Shell::Shell(std::unique_ptr<WebContents> web_contents,
   if (switches::IsRunWebTestsSwitchPresent()) {
     headless_ = !base::CommandLine::ForCurrentProcess()->HasSwitch(
         switches::kDisableHeadlessMode);
-    // Disable occlusion tracking. In a headless shell WebContents would always
-    // behave as if they were occluded, i.e. would not render frames and would
-    // not receive input events. For non-headless mode we do not want tests
-    // running in parallel to trigger occlusion tracking.
-    base::CommandLine::ForCurrentProcess()->AppendSwitch(
-        switches::kDisableBackgroundingOccludedWindowsForTesting);
   }
 
   if (base::CommandLine::ForCurrentProcess()->HasSwitch(
@@ -523,9 +517,10 @@ void Shell::NavigationStateChanged(WebContents* source,
 JavaScriptDialogManager* Shell::GetJavaScriptDialogManager(
     WebContents* source) {
   if (!dialog_manager_) {
-    dialog_manager_.reset(switches::IsRunWebTestsSwitchPresent()
-                              ? new WebTestJavaScriptDialogManager
-                              : new ShellJavaScriptDialogManager);
+    if (switches::IsRunWebTestsSwitchPresent())
+      dialog_manager_ = std::make_unique<WebTestJavaScriptDialogManager>();
+    else
+      dialog_manager_ = std::make_unique<ShellJavaScriptDialogManager>();
   }
   return dialog_manager_.get();
 }

content/shell/browser/shell_content_browser_client.cc

@@ -185,6 +185,7 @@ void ShellContentBrowserClient::AppendExtraCommandLineSwitches(
     switches::kCrashDumpsDir,
     switches::kEnableCrashReporter,
     switches::kExposeInternalsForTesting,
+    switches::kRunWebTests,
   };
 
   command_line->CopySwitchesFrom(*base::CommandLine::ForCurrentProcess(),

content/shell/browser/web_test/DEPS

+include_rules = [
+  "+content/shell/common/web_test",
+]

content/shell/browser/web_test/web_test_browser_main_runner.cc

@@ -111,6 +111,13 @@ void WebTestBrowserMainRunner::Initialize() {
 
   command_line.AppendSwitch(switches::kIgnoreCertificateErrors);
 
+  // Disable occlusion tracking. In a headless shell WebContents would always
+  // behave as if they were occluded, i.e. would not render frames and would
+  // not receive input events. For non-headless mode we do not want tests
+  // running in parallel to trigger occlusion tracking.
+  command_line.AppendSwitch(
+      switches::kDisableBackgroundingOccludedWindowsForTesting);
+
   // Always disable the unsandbox GPU process for DX12 and Vulkan Info
   // collection to avoid interference. This GPU process is launched 120
   // seconds after chrome starts.

content/shell/browser/web_test/web_test_content_browser_client.cc

@@ -259,9 +259,6 @@ void WebTestContentBrowserClient::AppendExtraCommandLineSwitches(
                                                             child_process_id);
 
   static const char* kForwardSwitches[] = {
-    // Indicates we're running web tests. Would be present in order to get
-    // here.
-    switches::kRunWebTests,
     // Switches from web_test_switches.h that are used in the renderer.
     switches::kEnableAccelerated2DCanvas,
     switches::kEnableFontAntialiasing,
@@ -388,8 +385,7 @@ void WebTestContentBrowserClient::RegisterBrowserInterfaceBindersForFrame(
 }
 
 bool WebTestContentBrowserClient::CanAcceptUntrustedExchangesIfNeeded() {
-  return base::CommandLine::ForCurrentProcess()->HasSwitch(
-      switches::kRunWebTests);
+  return true;
 }
 
 BluetoothDelegate* WebTestContentBrowserClient::GetBluetoothDelegate() {

content/shell/common/shell_switches.cc

@@ -5,7 +5,6 @@
 #include "content/shell/common/shell_switches.h"
 
 #include "base/command_line.h"
-#include "content/shell/common/web_test/web_test_switches.h"
 
 namespace switches {
 
@@ -25,6 +24,10 @@ const char kContentShellHostWindowSize[] = "content-shell-host-window-size";
 // Hides toolbar from content_shell's host window.
 const char kContentShellHideToolbar[] = "content-shell-hide-toolbar";
 
+// Runs Content Shell in web test mode, injecting test-only behaviour for
+// blink web tests.
+const char kRunWebTests[] = "run-web-tests";
+
 bool IsRunWebTestsSwitchPresent() {
   return base::CommandLine::ForCurrentProcess()->HasSwitch(
       switches::kRunWebTests);

content/shell/common/shell_switches.h

@@ -7,9 +7,6 @@
 #ifndef CONTENT_SHELL_COMMON_SHELL_SWITCHES_H_
 #define CONTENT_SHELL_COMMON_SHELL_SWITCHES_H_
 
-#include <string>
-#include <vector>
-
 namespace switches {
 
 extern const char kContentShellDataPath[];
@@ -17,11 +14,10 @@ extern const char kCrashDumpsDir[];
 extern const char kExposeInternalsForTesting[];
 extern const char kContentShellHostWindowSize[];
 extern const char kContentShellHideToolbar[];
+extern const char kRunWebTests[];
 
-// Tells if content shell is running web_tests.
-// TODO(lukasza): The function below somewhat violates the layering (by
-// enabling shell -> layout_tests dependency) but at least narrows the extent of
-// the dependency to a single switch...
+// Helper that returns true if kRunWebTests is present in the command line,
+// meaning Content Shell is running in web test mode.
 bool IsRunWebTestsSwitchPresent();
 
 }  // namespace switches

content/shell/common/web_test/web_test_switches.cc

@@ -35,10 +35,6 @@ const char kEnableLeakDetection[] = "enable-leak-detection";
 // Encode binary web test results (images, audio) using base64.
 const char kEncodeBinary[] = "encode-binary";
 
-// Request the render trees of pages to be dumped as text once they have
-// finished loading.
-const char kRunWebTests[] = "run-web-tests";
-
 // This makes us disable some web-platform runtime features so that we test
 // content_shell as if it was a stable release. It is only followed when
 // kRunWebTest is set. For the features' level, see

content/shell/common/web_test/web_test_switches.h

@@ -2,8 +2,6 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// Defines all the "web_test" command-line switches.
-
 #ifndef CONTENT_SHELL_COMMON_WEB_TEST_WEB_TEST_SWITCHES_H_
 #define CONTENT_SHELL_COMMON_WEB_TEST_WEB_TEST_SWITCHES_H_
 
@@ -22,7 +20,6 @@ extern const char kEnableFontAntialiasing[];
 extern const char kAlwaysUseComplexText[];
 extern const char kEnableLeakDetection[];
 extern const char kEncodeBinary[];
-extern const char kRunWebTests[];
 extern const char kStableReleaseMode[];
 extern const char kDisableHeadlessMode[];
 

content/shell/renderer/web_test/DEPS

 include_rules = [
+  "+content/shell/common/web_test",
+
   # Web test code is interconnected with test_runner. It would be nice to merge
   # these two layers and drop extra abstractions between them.
   "+content/shell/test_runner",

content/shell/test_runner/DEPS

@@ -20,6 +20,13 @@ include_rules = [
   "+ui/events",
   "+ui/gfx",
   "+v8/include",
+
+  # Separating Content Shell and web test code. This directory is part of web
+  # test, though in a separate gn component at the moment for historical
+  # reasons. https://crbug.com/866140
+  "+content/shell/common/web_test",
+  "+content/shell/renderer/web_test",
+  "+content/shell/test_runner",
 ]
 
 # //content/renderer is allowed for web tests helpers, but should not leak out