Skip to content
This repository has been archived by the owner on Sep 2, 2021. It is now read-only.

Disabled remote-debugging by default and appshell.app.getRemoteDebuggingPort() needs a callback argument to work #668

Merged
merged 6 commits into from
Nov 12, 2019
Merged

Disabled remote-debugging by default and appshell.app.getRemoteDebuggingPort() needs a callback argument to work #668

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
2a172ef
Disabled remote-debugging by default and appshell.app.getRemoteDebugg…
g-217 Nov 6, 2019
76fc591
Addressing review comments
g-217 Nov 11, 2019
533c965
Addressing review comments
g-217 Nov 11, 2019
436bdb2
Updating error meesage for port validation
g-217 Nov 11, 2019
9b903fa
Addressing review comments from vickramdhawal, also it is not possibl…
g-217 Nov 11, 2019
de74633
More error message correction
g-217 Nov 12, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions appshell/appshell_extension_handler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand All @@ -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<CefBrowser> browser = CefV8Context::GetCurrentContext()->GetBrowser();
Expand Down
3 changes: 3 additions & 0 deletions appshell/appshell_extensions.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
#include "update.h"

extern std::vector<CefString> gDroppedFiles;
extern int g_remote_debugging_port;

namespace appshell_extensions {

Expand Down Expand Up @@ -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") {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're changing the normal function to one that takes callback? Does this have any impact on existing usages? This is a breaking change, though I guess no extensions will ideally use this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it does impact one use case, and fixed that with commit: adobe/brackets@8677756

responseArgs->SetInt(2, g_remote_debugging_port);
}

else {
Expand Down
4 changes: 2 additions & 2 deletions appshell/appshell_extensions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
};


Expand Down
25 changes: 24 additions & 1 deletion appshell/cefclient.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include <cstdlib>
#include <sstream>
#include <string>
#include <errno.h>
#include "include/cef_app.h"
#include "include/cef_browser.h"
#include "include/cef_command_line.h"
Expand All @@ -19,6 +20,7 @@
#include "config.h"

CefRefPtr<ClientHandler> g_handler;
int g_remote_debugging_port = 0;
Copy link
Collaborator

@vickramdhawal vickramdhawal Nov 11, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please use unsigned int.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since, we are going to use atom or strtol further, as suggested by you using int is better for comparisons.


#ifdef OS_WIN
bool g_force_enable_acc = false;
Expand Down Expand Up @@ -95,7 +97,28 @@ void AppGetSettings(CefSettings& settings, CefRefPtr<CefCommandLine> 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<int>(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();

Expand Down
2 changes: 0 additions & 2 deletions appshell/config.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down