DevTools: use --custom-devtools-frontend when debug_devtools=false

Until now, when debug_devtools=true, --custom-devtools-frontend
can be used to specify a file path for requests to devtools://bundled/,
or a URL for requests to devtools://custom/. When debug_devtools=false,
only the latter works.

This change allows --custom-devtools-frontend to specify a file path
for requests to devtools://bundled/ while debug_devtools=false.

Change-Id: I4672ce22b53ae6fa91852bb0463c11db50f5af3f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1791150Reviewed-by: Dan Beam <dbeam@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#697912}

chrome/browser/ui/webui/devtools_ui_data_source.cc

@@ -84,11 +84,21 @@ std::string DevToolsDataSource::GetSource() {
   return chrome::kChromeUIDevToolsHost;
 }
 
+// static
+GURL GetCustomDevToolsFrontendURL() {
+  base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
+  if (command_line->HasSwitch(switches::kCustomDevtoolsFrontend)) {
+    return GURL(base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
+        switches::kCustomDevtoolsFrontend));
+  }
+  return GURL();
+}
+
 void DevToolsDataSource::StartDataRequest(
     const std::string& path,
     const content::WebContents::Getter& wc_getter,
     const GotDataCallback& callback) {
-  // Serve request from local bundle.
+  // Serve request to devtools://bundled/ from local bundle.
   std::string bundled_path_prefix(chrome::kChromeUIDevToolsBundledPath);
   bundled_path_prefix += "/";
   if (base::StartsWith(path, bundled_path_prefix,
@@ -99,15 +109,19 @@ void DevToolsDataSource::StartDataRequest(
                             base::CompareCase::INSENSITIVE_ASCII));
     std::string path_under_bundled =
         path_without_params.substr(bundled_path_prefix.length());
-#if BUILDFLAG(DEBUG_DEVTOOLS)
-    StartFileRequestForDebugDevtools(path_under_bundled, callback);
-#else
-    StartBundledDataRequest(path_under_bundled, callback);
+#if !BUILDFLAG(DEBUG_DEVTOOLS)
+    if (!GetCustomDevToolsFrontendURL().SchemeIsFile()) {
+      // Fetch from packaged resources.
+      StartBundledDataRequest(path_under_bundled, callback);
+      return;
+    }
 #endif
+    // Fetch from file system.
+    StartFileRequest(path_under_bundled, callback);
     return;
   }
 
-  // Serve empty page.
+  // Serve request to devtools://blank as empty page.
   std::string empty_path_prefix(chrome::kChromeUIDevToolsBlankPath);
   if (base::StartsWith(path, empty_path_prefix,
                        base::CompareCase::INSENSITIVE_ASCII)) {
@@ -115,7 +129,7 @@ void DevToolsDataSource::StartDataRequest(
     return;
   }
 
-  // Serve request from remote location.
+  // Serve request to devtools://remote from remote location.
   std::string remote_path_prefix(chrome::kChromeUIDevToolsRemotePath);
   remote_path_prefix += "/";
   if (base::StartsWith(path, remote_path_prefix,
@@ -132,29 +146,22 @@ void DevToolsDataSource::StartDataRequest(
     return;
   }
 
-  std::string custom_frontend_url =
-      base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
-          switches::kCustomDevtoolsFrontend);
-
-  if (custom_frontend_url.empty()) {
-    callback.Run(nullptr);
-    return;
-  }
-
-  // Serve request from custom location.
+  // Serve request to devtools://custom from custom URL.
   std::string custom_path_prefix(chrome::kChromeUIDevToolsCustomPath);
   custom_path_prefix += "/";
-
   if (base::StartsWith(path, custom_path_prefix,
                        base::CompareCase::INSENSITIVE_ASCII)) {
-    GURL url =
-        GURL(custom_frontend_url + path.substr(custom_path_prefix.length()));
-    DCHECK(url.is_valid());
-    StartCustomDataRequest(url, callback);
-    return;
+    GURL custom_devtools_frontend = GetCustomDevToolsFrontendURL();
+    if (!custom_devtools_frontend.is_empty()) {
+      GURL url = GURL(custom_devtools_frontend.spec() +
+                      path.substr(custom_path_prefix.length()));
+      DCHECK(url.is_valid());
+      StartCustomDataRequest(url, callback);
+      return;
+    }
   }
 
-  callback.Run(nullptr);
+  callback.Run(CreateNotFoundResponse());
 }
 
 std::string DevToolsDataSource::GetMimeType(const std::string& path) {
@@ -279,8 +286,7 @@ void DevToolsDataSource::StartNetworkRequest(
                      base::Unretained(this), request_iter));
 }
 
-#if BUILDFLAG(DEBUG_DEVTOOLS)
-scoped_refptr<base::RefCountedMemory> ReadFileForDebugDevTools(
+scoped_refptr<base::RefCountedMemory> ReadFileForDevTools(
     const base::FilePath& path) {
   std::string buffer;
   if (!base::ReadFileToString(path, &buffer)) {
@@ -290,29 +296,26 @@ scoped_refptr<base::RefCountedMemory> ReadFileForDebugDevTools(
   return base::RefCountedString::TakeString(&buffer);
 }
 
-void DevToolsDataSource::StartFileRequestForDebugDevtools(
-    const std::string& path,
-    const GotDataCallback& callback) {
-  base::FilePath inspector_debug_dir;
-  base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
-  if (command_line->HasSwitch(switches::kCustomDevtoolsFrontend)) {
-    inspector_debug_dir =
-        command_line->GetSwitchValuePath(switches::kCustomDevtoolsFrontend);
-    // --custom-devtools-frontend may already be used to specify an URL.
-    // In that case, fall back to the default debug-devtools bundle.
-    if (!base::PathExists(inspector_debug_dir))
-      inspector_debug_dir.clear();
-  }
-  if (inspector_debug_dir.empty() &&
-      !base::PathService::Get(chrome::DIR_INSPECTOR_DEBUG,
-                              &inspector_debug_dir)) {
-    callback.Run(CreateNotFoundResponse());
-    return;
+void DevToolsDataSource::StartFileRequest(const std::string& path,
+                                          const GotDataCallback& callback) {
+  base::FilePath base_path;
+  GURL custom_devtools_frontend = GetCustomDevToolsFrontendURL();
+  if (custom_devtools_frontend.SchemeIsFile()) {
+    base_path = base_path.AppendASCII(custom_devtools_frontend.GetContent());
+  } else {
+#if BUILDFLAG(DEBUG_DEVTOOLS)
+    // Use default path for unbundled files when debug_devtools=true
+    if (!base::PathService::Get(chrome::DIR_INSPECTOR_DEBUG, &base_path)) {
+      callback.Run(CreateNotFoundResponse());
+      return;
+    }
+#else
+    NOTREACHED();
+#endif
   }
 
-  DCHECK(!inspector_debug_dir.empty());
-
-  base::FilePath full_path = inspector_debug_dir.AppendASCII(path);
+  base::FilePath full_path = base_path.AppendASCII(path);
+  CHECK(base_path.IsParent(full_path));
 
   base::PostTaskAndReplyWithResult(
       FROM_HERE,
@@ -322,10 +325,8 @@ void DevToolsDataSource::StartFileRequestForDebugDevtools(
       // The usage of BindRepeating below is only because the type of
       // task callback needs to match that of response callback, which
       // is currently a repeating callback.
-      base::BindRepeating(ReadFileForDebugDevTools, std::move(full_path)),
-      callback);
+      base::BindRepeating(ReadFileForDevTools, std::move(full_path)), callback);
 }
-#endif  // BUILDFLAG(DEBUG_DEVTOOLS)
 
 void DevToolsDataSource::OnLoadComplete(
     std::list<PendingRequest>::iterator request_iter,

chrome/browser/ui/webui/devtools_ui_data_source.h

@@ -23,12 +23,11 @@ struct NetworkTrafficAnnotationTag;
 
 // An URLDataSource implementation that handles devtools://devtools/
 // requests. Three types of requests could be handled based on the URL path:
-// 1. /bundled/: bundled DevTools frontend is served.
-//    when built with debug_devtools=true, the path can be provided via
-//    --custom-devtools-frontend.
+// 1. /bundled/: bundled DevTools frontend is served. The path can be provided
+//    via --custom-devtools-frontend as file:// URL.
 // 2. /remote/: remote DevTools frontend is served from App Engine.
 // 3. /custom/: custom DevTools frontend is served from the server as specified
-//    by the --custom-devtools-frontend flag.
+//    via --custom-devtools-frontend as http:// URL.
 class DevToolsDataSource : public content::URLDataSource {
  public:
   using GotDataCallback = content::URLDataSource::GotDataCallback;
@@ -75,10 +74,8 @@ class DevToolsDataSource : public content::URLDataSource {
       int load_flags,
       const GotDataCallback& callback);
 
-#if BUILDFLAG(DEBUG_DEVTOOLS)
-  void StartFileRequestForDebugDevtools(const std::string& path,
-                                        const GotDataCallback& callback);
-#endif
+  virtual void StartFileRequest(const std::string& path,
+                                const GotDataCallback& callback);
 
   struct PendingRequest {
     PendingRequest();

chrome/browser/ui/webui/devtools_ui_data_source_unittest.cc

@@ -50,8 +50,14 @@ class TestDevToolsDataSource : public DevToolsDataSource {
       const net::NetworkTrafficAnnotationTag& traffic_annotation,
       int load_flags,
       const GotDataCallback& callback) override {
-    std::string copy_of_url = url.spec();
-    callback.Run(base::RefCountedString::TakeString(&copy_of_url));
+    std::string result = "url: " + url.spec();
+    callback.Run(base::RefCountedString::TakeString(&result));
+  }
+
+  void StartFileRequest(const std::string& path,
+                        const GotDataCallback& callback) override {
+    std::string result = "file: " + path;
+    callback.Run(base::RefCountedString::TakeString(&result));
   }
 };
 
@@ -120,12 +126,23 @@ TEST_F(DevToolsUIDataSourceTest, TestDevToolsBundledURLWithQueryParam) {
   EXPECT_FALSE(data().empty());
 }
 
+TEST_F(DevToolsUIDataSourceTest, TestDevToolsBundledURLWithSwitch) {
+  base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
+      switches::kCustomDevtoolsFrontend, "file://tmp/");
+  const GURL path =
+      DevToolsUrl().Resolve(DevToolsBundledPath(kDevToolsUITestFrontEndUrl));
+  StartRequest(path.path());
+  EXPECT_TRUE(data_received());
+  EXPECT_EQ(data(), "file: devtools_app.html");
+}
+
 TEST_F(DevToolsUIDataSourceTest, TestDevToolsInvalidBundledURL) {
   const GURL path =
       DevToolsUrl().Resolve(DevToolsBundledPath("invalid_devtools_app.html"));
   StartRequest(path.path());
   EXPECT_TRUE(data_received());
-  EXPECT_TRUE(data().empty());
+  ASSERT_TRUE(base::StartsWith(data(), kDevToolsUITest404Response,
+                               base::CompareCase::SENSITIVE));
 }
 
 TEST_F(DevToolsUIDataSourceTest, TestDevToolsInvalidBundledURLWithQueryParam) {
@@ -133,7 +150,8 @@ TEST_F(DevToolsUIDataSourceTest, TestDevToolsInvalidBundledURLWithQueryParam) {
       DevToolsUrl().Resolve(DevToolsBundledPath("invalid_devtools_app.html"));
   StartRequest(path.path() + "?foo");
   EXPECT_TRUE(data_received());
-  EXPECT_TRUE(data().empty());
+  ASSERT_TRUE(base::StartsWith(data(), kDevToolsUITest404Response,
+                               base::CompareCase::SENSITIVE));
 }
 
 // devtools/blank path
@@ -159,8 +177,9 @@ TEST_F(DevToolsUIDataSourceTest, TestDevToolsRemoteURL) {
       DevToolsUrl().Resolve(DevToolsRemotePath(kDevToolsUITestFrontEndUrl));
   StartRequest(path.path());
   EXPECT_TRUE(data_received());
-  EXPECT_EQ(data(),
-            "https://chrome-devtools-frontend.appspot.com/devtools_app.html");
+  EXPECT_EQ(
+      data(),
+      "url: https://chrome-devtools-frontend.appspot.com/devtools_app.html");
 }
 
 TEST_F(DevToolsUIDataSourceTest, TestDevToolsRemoteURLWithQueryParam) {
@@ -179,7 +198,8 @@ TEST_F(DevToolsUIDataSourceTest, TestDevToolsCustomURLWithNoSwitch) {
       DevToolsUrl().Resolve(DevToolsCustomPath(kDevToolsUITestFrontEndUrl));
   StartRequest(path.path());
   EXPECT_TRUE(data_received());
-  EXPECT_TRUE(data().empty());
+  ASSERT_TRUE(base::StartsWith(data(), kDevToolsUITest404Response,
+                               base::CompareCase::SENSITIVE));
 }
 
 TEST_F(DevToolsUIDataSourceTest, TestDevToolsCustomURLWithSwitch) {
@@ -189,7 +209,7 @@ TEST_F(DevToolsUIDataSourceTest, TestDevToolsCustomURLWithSwitch) {
       DevToolsUrl().Resolve(DevToolsCustomPath(kDevToolsUITestFrontEndUrl));
   StartRequest(path.path());
   EXPECT_TRUE(data_received());
-  EXPECT_EQ(data(), "http://localhost:8090/front_end/devtools_app.html");
+  EXPECT_EQ(data(), "url: http://localhost:8090/front_end/devtools_app.html");
 }
 
 TEST_F(DevToolsUIDataSourceTest, TestDevToolsCustomURLWithSwitchAndQueryParam) {
@@ -199,7 +219,8 @@ TEST_F(DevToolsUIDataSourceTest, TestDevToolsCustomURLWithSwitchAndQueryParam) {
       DevToolsUrl().Resolve(DevToolsCustomPath(kDevToolsUITestFrontEndUrl));
   StartRequest(path.path() + "?foo");
   EXPECT_TRUE(data_received());
-  EXPECT_EQ(data(), "http://localhost:8090/front_end/devtools_app.html?foo");
+  EXPECT_EQ(data(),
+            "url: http://localhost:8090/front_end/devtools_app.html?foo");
 }
 
 #if !DCHECK_IS_ON()
@@ -222,7 +243,8 @@ TEST_F(DevToolsUIDataSourceTest, TestDevToolsNoRoute) {
   const GURL path = DevToolsUrl().Resolve(kDevToolsUITestFrontEndUrl);
   StartRequest(path.path());
   EXPECT_TRUE(data_received());
-  EXPECT_TRUE(data().empty());
+  ASSERT_TRUE(base::StartsWith(data(), kDevToolsUITest404Response,
+                               base::CompareCase::SENSITIVE));
 }
 
 TEST_F(DevToolsUIDataSourceTest, TestDevToolsNoRouteWithSwitch) {
@@ -231,5 +253,6 @@ TEST_F(DevToolsUIDataSourceTest, TestDevToolsNoRouteWithSwitch) {
   const GURL path = DevToolsUrl().Resolve(kDevToolsUITestFrontEndUrl);
   StartRequest(path.path());
   EXPECT_TRUE(data_received());
-  EXPECT_TRUE(data().empty());
+  ASSERT_TRUE(base::StartsWith(data(), kDevToolsUITest404Response,
+                               base::CompareCase::SENSITIVE));
 }

chrome/common/chrome_switches.cc

@@ -137,8 +137,10 @@ const char kCrashOnHangThreads[]            = "crash-on-hang-threads";
 const char kCreateBrowserOnStartupForTests[] =
     "create-browser-on-startup-for-tests";
 
-// Specifies the HTTP endpoint which will be used to serve
+// Specifies the http:// endpoint which will be used to serve
 // devtools://devtools/custom/<path>
+// Or a file:// URL to specify a custom file path to load from for
+// devtools://devtools/bundled/<path>
 const char kCustomDevtoolsFrontend[] = "custom-devtools-frontend";
 
 // Enables a frame context menu item that toggles the frame in and out of glass