From 9a566b8a236c61aca299af34f4730f097007a46b Mon Sep 17 00:00:00 2001 From: Gautam Jha Date: Tue, 12 Nov 2019 20:13:04 +0530 Subject: [PATCH 1/3] Disabled remote-debugging by default and appshell.app.getRemoteDebuggingPort() needs a callback argument to work (#668) * Disabled remote-debugging by default and appshell.app.getRemoteDebuggingPort() needs a callback argument to work * Addressing review comments * Addressing review comments * Updating error meesage for port validation * Addressing review comments from vickramdhawal, also it is not possible to use g_handler to hold remote-debugging-port as g_handler does not get initialized while we are parsing command line * More error message correction --- appshell/appshell_extension_handler.h | 4 +--- appshell/appshell_extensions.cpp | 3 +++ appshell/appshell_extensions.js | 4 ++-- appshell/cefclient.cpp | 25 ++++++++++++++++++++++++- appshell/config.h | 2 -- 5 files changed, 30 insertions(+), 8 deletions(-) diff --git a/appshell/appshell_extension_handler.h b/appshell/appshell_extension_handler.h index 588f42bbf..0df32e42f 100644 --- a/appshell/appshell_extension_handler.h +++ b/appshell/appshell_extension_handler.h @@ -160,7 +160,7 @@ class AppShellExtensionHandler : public CefV8Handler { CefString& exception) { // The only messages that are handled here is getElapsedMilliseconds(), - // GetCurrentLanguage(), GetApplicationSupportDirectory(), and GetRemoteDebuggingPort(). + // GetCurrentLanguage(), and GetApplicationSupportDirectory(). // All other messages are passed to the browser process. if (name == "GetElapsedMilliseconds") { retval = CefV8Value::CreateDouble(GetElapsedMilliseconds()); @@ -170,8 +170,6 @@ class AppShellExtensionHandler : public CefV8Handler { retval = CefV8Value::CreateString(AppGetSupportDirectory()); } else if (name == "GetUserDocumentsDirectory") { retval = CefV8Value::CreateString(AppGetDocumentsDirectory()); - } else if (name == "GetRemoteDebuggingPort") { - retval = CefV8Value::CreateInt(REMOTE_DEBUGGING_PORT); } else { // Pass all messages to the browser process. Look in appshell_extensions.cpp for implementation. CefRefPtr browser = CefV8Context::GetCurrentContext()->GetBrowser(); diff --git a/appshell/appshell_extensions.cpp b/appshell/appshell_extensions.cpp index c5ac80bd8..4723876d9 100644 --- a/appshell/appshell_extensions.cpp +++ b/appshell/appshell_extensions.cpp @@ -37,6 +37,7 @@ #include "update.h" extern std::vector gDroppedFiles; +extern int g_remote_debugging_port; namespace appshell_extensions { @@ -842,6 +843,8 @@ class ProcessMessageDelegate : public ClientHandler::ProcessMessageDelegate { uberDict->SetList(0, dirContents); uberDict->SetList(1, allStats); responseArgs->SetList(2, uberDict); + } else if (message_name == "GetRemoteDebuggingPort") { + responseArgs->SetInt(2, g_remote_debugging_port); } else { diff --git a/appshell/appshell_extensions.js b/appshell/appshell_extensions.js index fc161e6e4..379420d27 100644 --- a/appshell/appshell_extensions.js +++ b/appshell/appshell_extensions.js @@ -640,8 +640,8 @@ if (!brackets) { * @return int. The remote debugging port used by the appshell. */ native function GetRemoteDebuggingPort(); - appshell.app.getRemoteDebuggingPort = function () { - return GetRemoteDebuggingPort(); + appshell.app.getRemoteDebuggingPort = function (callback) { + GetRemoteDebuggingPort(callback || _dummyCallback); }; diff --git a/appshell/cefclient.cpp b/appshell/cefclient.cpp index a6bf5aca8..5d84d930b 100644 --- a/appshell/cefclient.cpp +++ b/appshell/cefclient.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include "include/cef_app.h" #include "include/cef_browser.h" #include "include/cef_command_line.h" @@ -19,6 +20,7 @@ #include "config.h" CefRefPtr g_handler; +int g_remote_debugging_port = 0; #ifdef OS_WIN bool g_force_enable_acc = false; @@ -95,7 +97,28 @@ void AppGetSettings(CefSettings& settings, CefRefPtr command_lin command_line->GetSwitchValue(client::switches::kJavascriptFlags); // Enable dev tools - settings.remote_debugging_port = REMOTE_DEBUGGING_PORT; + CefString debugger_port = command_line->GetSwitchValue("remote-debugging-port"); + if (!debugger_port.empty()) { + const long port = strtol(debugger_port.ToString().c_str(), NULL, 10); + if (errno == ERANGE || port == 0) { + LOG(ERROR) << "Could not enable Remote debugging."; + LOG(ERROR) << "Error while parsing remote-debugging-port arg: "<< debugger_port.ToString(); + errno = 0; + } + else { + static const long max_port_num = 65535; + static const long max_reserved_port_num = 1024; + if (port > max_reserved_port_num && port < max_port_num) { + g_remote_debugging_port = static_cast(port); + settings.remote_debugging_port = g_remote_debugging_port; + } + else { + LOG(ERROR) << "Could not enable Remote debugging on port: "<< port + << ". Port number must be greater than "<< max_reserved_port_num + << " and less than " << max_port_num << "."; + } + } + } std::wstring versionStr = appshell::AppGetProductVersionString(); diff --git a/appshell/config.h b/appshell/config.h index 2e1d5d397..9744e1973 100644 --- a/appshell/config.h +++ b/appshell/config.h @@ -82,8 +82,6 @@ #endif -#define REMOTE_DEBUGGING_PORT 9234 - // Comment out this line to enable OS themed drawing #define DARK_UI #define DARK_AERO_GLASS From 340be0e37ccba982b04d1b03d95b167e82186c33 Mon Sep 17 00:00:00 2001 From: Gautam Jha Date: Mon, 18 Nov 2019 13:49:50 +0530 Subject: [PATCH 2/3] Improved console error messages (#669) --- appshell/cefclient.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/appshell/cefclient.cpp b/appshell/cefclient.cpp index 5d84d930b..f0f875658 100644 --- a/appshell/cefclient.cpp +++ b/appshell/cefclient.cpp @@ -101,21 +101,21 @@ void AppGetSettings(CefSettings& settings, CefRefPtr command_lin if (!debugger_port.empty()) { const long port = strtol(debugger_port.ToString().c_str(), NULL, 10); if (errno == ERANGE || port == 0) { - LOG(ERROR) << "Could not enable Remote debugging."; - LOG(ERROR) << "Error while parsing remote-debugging-port arg: "<< debugger_port.ToString(); + LOG(ERROR) << "Could not enable remote debugging." + << " Error while parsing remote-debugging-port arg: "<< debugger_port.ToString(); errno = 0; } else { - static const long max_port_num = 65535; - static const long max_reserved_port_num = 1024; - if (port > max_reserved_port_num && port < max_port_num) { + static const long max_port_num = 65534; + static const long max_reserved_port_num = 1025; + if (port >= max_reserved_port_num && port <= max_port_num) { g_remote_debugging_port = static_cast(port); settings.remote_debugging_port = g_remote_debugging_port; } else { - LOG(ERROR) << "Could not enable Remote debugging on port: "<< port - << ". Port number must be greater than "<< max_reserved_port_num - << " and less than " << max_port_num << "."; + LOG(ERROR) << "Cannot enable remote debugging on port "<< port + << ". Port numbers should be between "<< max_reserved_port_num + << " and " << max_port_num << "."; } } } From c662e970649f32f650ae95627e0b0a0787e36382 Mon Sep 17 00:00:00 2001 From: Narayani Date: Thu, 21 Nov 2019 17:43:02 +0530 Subject: [PATCH 3/3] Merge pull request #673 from jha-g/jha-g/moving-commandline-error-to-infobar2 moving command line error to infobar and fixing windows port range issues --- appshell/appshell_extensions.cpp | 19 ++++++++++++++++--- appshell/cefclient.cpp | 31 +++++++++++++++---------------- 2 files changed, 31 insertions(+), 19 deletions(-) diff --git a/appshell/appshell_extensions.cpp b/appshell/appshell_extensions.cpp index 4723876d9..773cc68a6 100644 --- a/appshell/appshell_extensions.cpp +++ b/appshell/appshell_extensions.cpp @@ -38,6 +38,7 @@ extern std::vector gDroppedFiles; extern int g_remote_debugging_port; +extern std::string g_get_remote_debugging_port_error; namespace appshell_extensions { @@ -57,6 +58,7 @@ class ProcessMessageDelegate : public ClientHandler::ProcessMessageDelegate { CefRefPtr argList = message->GetArgumentList(); int32 callbackId = -1; int32 error = NO_ERROR; + std::string errInfo; CefRefPtr response = CefProcessMessage::Create("invokeCallback"); CefRefPtr responseArgs = response->GetArgumentList(); @@ -844,7 +846,13 @@ class ProcessMessageDelegate : public ClientHandler::ProcessMessageDelegate { uberDict->SetList(1, allStats); responseArgs->SetList(2, uberDict); } else if (message_name == "GetRemoteDebuggingPort") { - responseArgs->SetInt(2, g_remote_debugging_port); + if (g_get_remote_debugging_port_error.empty() && g_remote_debugging_port > 0) { + responseArgs->SetInt(2, g_remote_debugging_port); + } + else { + responseArgs->SetNull(2); + errInfo = g_get_remote_debugging_port_error; + } } else { @@ -853,8 +861,13 @@ class ProcessMessageDelegate : public ClientHandler::ProcessMessageDelegate { } if (callbackId != -1) { - responseArgs->SetInt(1, error); - + if (errInfo.empty()) { + responseArgs->SetInt(1, error); + } + else { + responseArgs->SetString(1, errInfo); + } + // Send response browser->SendProcessMessage(PID_RENDERER, response); } diff --git a/appshell/cefclient.cpp b/appshell/cefclient.cpp index f0f875658..07b7fe9fe 100644 --- a/appshell/cefclient.cpp +++ b/appshell/cefclient.cpp @@ -21,6 +21,7 @@ CefRefPtr g_handler; int g_remote_debugging_port = 0; +std::string g_get_remote_debugging_port_error; #ifdef OS_WIN bool g_force_enable_acc = false; @@ -99,24 +100,22 @@ void AppGetSettings(CefSettings& settings, CefRefPtr command_lin // Enable dev tools CefString debugger_port = command_line->GetSwitchValue("remote-debugging-port"); if (!debugger_port.empty()) { - const long port = strtol(debugger_port.ToString().c_str(), NULL, 10); - if (errno == ERANGE || port == 0) { - LOG(ERROR) << "Could not enable remote debugging." - << " Error while parsing remote-debugging-port arg: "<< debugger_port.ToString(); - errno = 0; + g_get_remote_debugging_port_error = debugger_port.ToString(); + long port = strtol(g_get_remote_debugging_port_error.c_str(), NULL, 10); + if (errno == ERANGE) { + errno = port = 0; + } + static const long max_port_num = 65535; + static const long max_reserved_port_num = 1023; + if (port > max_reserved_port_num && port < max_port_num) { + g_remote_debugging_port = static_cast(port); + settings.remote_debugging_port = g_remote_debugging_port; + g_get_remote_debugging_port_error.clear(); } else { - static const long max_port_num = 65534; - static const long max_reserved_port_num = 1025; - if (port >= max_reserved_port_num && port <= max_port_num) { - g_remote_debugging_port = static_cast(port); - settings.remote_debugging_port = g_remote_debugging_port; - } - else { - LOG(ERROR) << "Cannot enable remote debugging on port "<< port - << ". Port numbers should be between "<< max_reserved_port_num - << " and " << max_port_num << "."; - } + // Setting debugging port to highest number will disable remote debugging + // As setting.remote_debugging_port has higher priority compared to command line option + settings.remote_debugging_port = max_port_num; } }