From b7b9efa875f5e63dd182247909629c37c68d8788 Mon Sep 17 00:00:00 2001 From: Samuel Attard Date: Tue, 16 Apr 2019 14:22:51 -0400 Subject: [PATCH] fix: handle remote-debugging-port=0 correctly (#17800) By default the Chromedriver will send remote-debugging-port=0 to let the browser choose a free port to listen on. The chosen port is written to a known file in the user data dir that is passed to the app through the CLI. This PR does two things. 1. Correctly passes the USER_DATA_DIR to the remote debugging server so it knows where to write the file 2. Adds support for --user-data-dir as we did not support that CLI argument and Chromedriver relies on being able to tell the "browser" where to write this file. Fixes #17354 --- atom/browser/ui/devtools_manager_delegate.cc | 8 ++++++-- default_app/main.ts | 4 +--- lib/browser/api/app.ts | 15 +++++++++++++++ lib/browser/init.ts | 5 +---- typings/internal-electron.d.ts | 1 + 5 files changed, 24 insertions(+), 9 deletions(-) diff --git a/atom/browser/ui/devtools_manager_delegate.cc b/atom/browser/ui/devtools_manager_delegate.cc index 95e45cf243b5..f461f6e67d1b 100644 --- a/atom/browser/ui/devtools_manager_delegate.cc +++ b/atom/browser/ui/devtools_manager_delegate.cc @@ -8,9 +8,11 @@ #include #include +#include "atom/browser/atom_paths.h" #include "base/bind.h" #include "base/command_line.h" #include "base/files/file_path.h" +#include "base/path_service.h" #include "base/strings/string_number_conversions.h" #include "base/strings/stringprintf.h" #include "base/strings/utf_string_conversions.h" @@ -67,7 +69,7 @@ std::unique_ptr CreateSocketFactory() { int temp_port; std::string port_str = command_line.GetSwitchValueASCII(switches::kRemoteDebuggingPort); - if (base::StringToInt(port_str, &temp_port) && temp_port > 0 && + if (base::StringToInt(port_str, &temp_port) && temp_port >= 0 && temp_port < 65535) { port = temp_port; } else { @@ -84,8 +86,10 @@ std::unique_ptr CreateSocketFactory() { // static void DevToolsManagerDelegate::StartHttpHandler() { + base::FilePath user_dir; + base::PathService::Get(DIR_USER_DATA, &user_dir); content::DevToolsAgentHost::StartRemoteDebuggingServer( - CreateSocketFactory(), base::FilePath(), base::FilePath()); + CreateSocketFactory(), user_dir, base::FilePath()); } DevToolsManagerDelegate::DevToolsManagerDelegate() {} diff --git a/default_app/main.ts b/default_app/main.ts index a25ba6d5579c..597f47ddd8b4 100644 --- a/default_app/main.ts +++ b/default_app/main.ts @@ -102,9 +102,7 @@ function loadApplicationPackage (packagePath: string) { } else if (packageJson.name) { app.setName(packageJson.name) } - app.setPath('userData', path.join(app.getPath('appData'), app.getName())) - app.setPath('userCache', path.join(app.getPath('cache'), app.getName())) - app.setAppPath(packagePath) + app._setDefaultAppPaths(packagePath) } try { diff --git a/lib/browser/api/app.ts b/lib/browser/api/app.ts index cc02b5b03120..0d6110e4ceb1 100644 --- a/lib/browser/api/app.ts +++ b/lib/browser/api/app.ts @@ -57,6 +57,21 @@ app.isPackaged = (() => { return execFile !== 'electron' })() +app._setDefaultAppPaths = (packagePath) => { + // Set the user path according to application's name. + app.setPath('userData', path.join(app.getPath('appData'), app.getName())) + app.setPath('userCache', path.join(app.getPath('cache'), app.getName())) + app.setAppPath(packagePath) + + // Add support for --user-data-dir= + const userDataDirFlag = '--user-data-dir=' + const userDataArg = process.argv.find(arg => arg.startsWith(userDataDirFlag)) + if (userDataArg) { + const userDataDir = userDataArg.substr(userDataDirFlag.length) + if (path.isAbsolute(userDataDir)) app.setPath('userData', userDataDir) + } +} + if (process.platform === 'darwin') { const setDockMenu = app.dock.setMenu app.dock.setMenu = (menu) => { diff --git a/lib/browser/init.ts b/lib/browser/init.ts index 1cb9d785de59..473433a51196 100644 --- a/lib/browser/init.ts +++ b/lib/browser/init.ts @@ -151,10 +151,7 @@ if (packageJson.v8Flags != null) { v8.setFlagsFromString(packageJson.v8Flags) } -// Set the user path according to application's name. -app.setPath('userData', path.join(app.getPath('appData'), app.getName())) -app.setPath('userCache', path.join(app.getPath('cache'), app.getName())) -app.setAppPath(packagePath) +app._setDefaultAppPaths(packagePath) // Load the chrome devtools support. require('@electron/internal/browser/chrome-devtools') diff --git a/typings/internal-electron.d.ts b/typings/internal-electron.d.ts index 8c15bc758a4d..5760bfb9b4d7 100644 --- a/typings/internal-electron.d.ts +++ b/typings/internal-electron.d.ts @@ -13,6 +13,7 @@ declare namespace Electron { } interface App { + _setDefaultAppPaths(packagePath: string | null): void; setVersion(version: string): void; setDesktopName(name: string): void; setAppPath(path: string | null): void;