Skip to content

Commit

Permalink
Only create DebugModule where necessary. (youtube#654)
Browse files Browse the repository at this point in the history 
Create a DebugModule only for the MainWebModule, so that there is no
unnecessary waste observing messages which are then always discarded.

Add a command-line switch '--disable_web_debugger' that disables web
debugger support completely, including the debug server.

b/287658692
  • Loading branch information
@jellefoks
jellefoks committed Jul 26, 2023
1 parent 810fb26 commit df40dfd
Show file tree
Hide file tree
Showing 11 changed files with 106 additions and 66 deletions.
21 changes: 13 additions & 8 deletions cobalt/browser/application.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -985,15 +985,20 @@ Application::Application(const base::Closure& quit_closure, bool should_preload,
#endif // ENABLE_WEBDRIVER

#if defined(ENABLE_DEBUGGER)
int remote_debugging_port = GetRemoteDebuggingPort();
if (remote_debugging_port == 0) {
DLOG(INFO) << "Remote web debugger disabled because "
<< switches::kRemoteDebuggingPort << " is 0.";
if (base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kDisableWebDebugger)) {
LOG(INFO) << "Remote web debugger disabled.";
} else {
debug_web_server_.reset(new debug::remote::DebugWebServer(
remote_debugging_port, GetDevServersListenIp(),
base::Bind(&BrowserModule::CreateDebugClient,
base::Unretained(browser_module_.get()))));
int remote_debugging_port = GetRemoteDebuggingPort();
if (remote_debugging_port == 0) {
LOG(INFO) << "Remote web debugger disabled because "
<< switches::kRemoteDebuggingPort << " is 0.";
} else {
debug_web_server_.reset(new debug::remote::DebugWebServer(
remote_debugging_port, GetDevServersListenIp(),
base::Bind(&BrowserModule::CreateDebugClient,
base::Unretained(browser_module_.get()))));
}
}
#endif // ENABLE_DEBUGGER

Expand Down
15 changes: 12 additions & 3 deletions cobalt/browser/browser_module.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -620,6 +620,10 @@ void BrowserModule::Navigate(const GURL& url_reference) {
}

options.debugger_state = debugger_state_.get();
if (!base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kDisableWebDebugger)) {
options.enable_debugger = true;
}
#endif // ENABLE_DEBUGGER

// Pass down this callback from to Web module.
Expand Down Expand Up @@ -1418,9 +1422,14 @@ std::unique_ptr<debug::DebugClient> BrowserModule::CreateDebugClient(
FROM_HERE,
base::Bind(&BrowserModule::GetDebugDispatcherInternal,
base::Unretained(this), base::Unretained(&debug_dispatcher)));
DCHECK(debug_dispatcher);
return std::unique_ptr<debug::DebugClient>(
new debug::DebugClient(debug_dispatcher, delegate));
if (debug_dispatcher) {
return std::unique_ptr<debug::DebugClient>(
new debug::DebugClient(debug_dispatcher, delegate));
} else {
LOG(ERROR)
<< "Debugger connected but debugging the main web module is disabled.";
return nullptr;
}
}

void BrowserModule::GetDebugDispatcherInternal(
Expand Down
9 changes: 8 additions & 1 deletion cobalt/browser/debug_console.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,14 @@ bool DebugConsole::FilterOnScreenKeyboardInputEvent(

void DebugConsole::CycleMode() {
base::AutoLock lock(mode_mutex_);
mode_ = (mode_ + 1) % debug::console::kDebugConsoleModeCount;
int number_of_modes = debug::console::kDebugConsoleModeCount;
#if defined(ENABLE_DEBUGGER)
if (base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kDisableWebDebugger)) {
number_of_modes = (debug::console::kDebugConsoleModeHud + 1);
}
#endif
mode_ = (mode_ + 1) % number_of_modes;
}

debug::console::DebugConsoleMode DebugConsole::GetMode() {
Expand Down
6 changes: 5 additions & 1 deletion cobalt/browser/switches.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ const char kDevServersListenIpHelp[] =
"IPv4), and to listen to LOOPBACK use \"::1\" (\"127.0.0.1\" for IPv4)";

#if defined(ENABLE_DEBUGGER)
const char kDisableWebDebugger[] = "disable_web_debugger";
const char kDisableWebDebuggerHelp[] = "Disable support for the web debugger";

const char kRemoteDebuggingPort[] = "remote_debugging_port";
const char kRemoteDebuggingPortHelp[] =
"Remote web debugger is served from the specified port. If 0, then the "
Expand Down Expand Up @@ -439,8 +442,9 @@ std::string HelpMessage() {
{kDebugConsoleMode, kDebugConsoleModeHelp},
{kDevServersListenIp, kDevServersListenIpHelp},
#if defined(ENABLE_DEBUGGER)
{kWaitForWebDebugger, kWaitForWebDebuggerHelp},
{kDisableWebDebugger, kDisableWebDebuggerHelp},
{kRemoteDebuggingPort, kRemoteDebuggingPortHelp},
{kWaitForWebDebugger, kWaitForWebDebuggerHelp},
#endif // ENABLE_DEBUGGER
{kDisableImageAnimations, kDisableImageAnimationsHelp},
{kForceDeterministicRendering, kForceDeterministicRenderingHelp},
Expand Down
2 changes: 2 additions & 0 deletions cobalt/browser/switches.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ extern const char kDevServersListenIp[];
extern const char kDevServersListenIpHelp[];

#if defined(ENABLE_DEBUGGER)
extern const char kDisableWebDebugger[];
extern const char kDisableWebDebuggerHelp[];
extern const char kRemoteDebuggingPort[];
extern const char kRemoteDebuggingPortHelp[];
extern const char kWaitForWebDebugger[];
Expand Down
50 changes: 31 additions & 19 deletions cobalt/browser/web_module.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,8 +142,7 @@ class WebModule::Impl {

#if defined(ENABLE_DEBUGGER)
debug::backend::DebugDispatcher* debug_dispatcher() {
DCHECK(debug_module_);
return debug_module_->debug_dispatcher();
return debug_module_ ? debug_module_->debug_dispatcher() : nullptr;
}
#endif // ENABLE_DEBUGGER

Expand Down Expand Up @@ -229,7 +228,13 @@ class WebModule::Impl {

void FreezeDebugger(
std::unique_ptr<debug::backend::DebuggerState>* debugger_state) {
if (debugger_state) *debugger_state = debug_module_->Freeze();
if (debugger_state) {
if (debug_module_) {
*debugger_state = debug_module_->Freeze();
} else {
debugger_state->reset();
}
}
}
#endif // defined(ENABLE_DEBUGGER)

Expand Down Expand Up @@ -613,7 +618,7 @@ WebModule::Impl::Impl(web::Context* web_context, const ConstructionData& data)
web_context_->global_environment()->AddRoot(media_source_registry_.get());

#if defined(ENABLE_DEBUGGER)
if (data.options.wait_for_web_debugger) {
if (data.options.enable_debugger && data.options.wait_for_web_debugger) {
// Post a task that blocks the message loop and waits for the web debugger.
// This must be posted before the the window's task to load the document.
waiting_for_web_debugger_->store(true);
Expand Down Expand Up @@ -718,13 +723,15 @@ WebModule::Impl::Impl(web::Context* web_context, const ConstructionData& data)
}

#if defined(ENABLE_DEBUGGER)
debug_overlay_.reset(
new debug::backend::RenderOverlay(render_tree_produced_callback_));

debug_module_.reset(new debug::backend::DebugModule(
&debugger_hooks_, web_context_->global_environment(),
debug_overlay_.get(), resource_provider_, window_,
data.options.debugger_state));
if (data.options.enable_debugger) {
debug_overlay_.reset(
new debug::backend::RenderOverlay(render_tree_produced_callback_));

debug_module_.reset(new debug::backend::DebugModule(
&debugger_hooks_, web_context_->global_environment(),
debug_overlay_.get(), resource_provider_, window_,
data.options.debugger_state));
}
#endif // ENABLE_DEBUGGER

report_unload_timing_info_callback_ =
Expand Down Expand Up @@ -955,7 +962,11 @@ void WebModule::Impl::OnRenderTreeProduced(
last_render_tree_produced_time_));

#if defined(ENABLE_DEBUGGER)
debug_overlay_->OnRenderTreeProduced(layout_results_with_callback);
if (debug_overlay_) {
debug_overlay_->OnRenderTreeProduced(layout_results_with_callback);
} else {
render_tree_produced_callback_.Run(layout_results_with_callback);
}
#else // ENABLE_DEBUGGER
render_tree_produced_callback_.Run(layout_results_with_callback);
#endif // ENABLE_DEBUGGER
Expand Down Expand Up @@ -1036,12 +1047,13 @@ void WebModule::Impl::CreateWindowDriver(
#if defined(ENABLE_DEBUGGER)
void WebModule::Impl::WaitForWebDebugger() {
DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
DCHECK(debug_module_);
LOG(WARNING) << "\n-------------------------------------"
"\n Waiting for web debugger to connect "
"\n-------------------------------------";
// This blocks until the web debugger connects.
debug_module_->debug_dispatcher()->SetPaused(true);
if (debug_module_) {
LOG(WARNING) << "\n-------------------------------------"
"\n Waiting for web debugger to connect "
"\n-------------------------------------";
// This blocks until the web debugger connects.
debug_module_->debug_dispatcher()->SetPaused(true);
}
waiting_for_web_debugger_->store(false);
}
#endif // defined(ENABLE_DEBUGGER)
Expand Down Expand Up @@ -1132,7 +1144,7 @@ void WebModule::Impl::Conceal(render_tree::ResourceProvider* resource_provider,

#if defined(ENABLE_DEBUGGER)
// The debug overlay may be holding onto a render tree, clear that out.
debug_overlay_->ClearInput();
if (debug_overlay_) debug_overlay_->ClearInput();
#endif

// Force garbage collection in |javascript_engine|.
Expand Down
2 changes: 2 additions & 0 deletions cobalt/browser/web_module.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,8 @@ class WebModule : public base::MessageLoop::DestructionObserver,
bool limit_performance_timer_resolution = true;

#if defined(ENABLE_DEBUGGER)
// Whether a debugger should be started for this WebModule.
bool enable_debugger = false;
// Whether the debugger should block until remote devtools connects.
bool wait_for_web_debugger = false;

Expand Down
55 changes: 28 additions & 27 deletions cobalt/debug/console/content/debugger_client.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ function DebuggerClient() {
this.DEBUGGER_DETACHED = 0;
this.DEBUGGER_ATTACHING = 1;
this.DEBUGGER_ATTACHED = 2;
this.DEBUGGER_DISABLED = 3;
this.scripts = [];
this.attachState = this.DEBUGGER_DETACHED;
this.onAttachCallback = this.onAttach.bind(this);
Expand All @@ -25,11 +26,11 @@ function DebuggerClient() {

// Attaches to the debugger and listens for debug events.
// Enables the domains we care about here.
DebuggerClient.prototype.attach = function() {
DebuggerClient.prototype.attach = function () {
if (this.attachState == this.DEBUGGER_DETACHED) {
this.attachState = this.DEBUGGER_ATTACHING;
printToMessageLog(MessageLog.INTERACTIVE,
'Attempting to attach to debugger...');
'Attempting to attach to debugger...');
this.scripts = [];
debugHub.onEvent.addListener(this.onEventCallback);
debugHub.attach(this.onAttachCallback);
Expand All @@ -38,14 +39,14 @@ DebuggerClient.prototype.attach = function() {
this.sendCommand('Runtime.enable');
} else if (this.attachState == this.DEBUGGER_ATTACHING) {
printToMessageLog(MessageLog.INTERACTIVE,
'Still attempting to attach to debugger...');
'Still attempting to attach to debugger...');
}
}

// Local method to list the parsed scripts the client has been notifed of
// Local method to list the parsed scripts the client has been notified of
// via the |Debugger.scriptParsed| event. Maps the possibly very long script id
// from the debug dispatcher to a more human-readable 0-based index.
DebuggerClient.prototype.getScripts = function(scriptId) {
DebuggerClient.prototype.getScripts = function (scriptId) {
for (var i in this.scripts) {
var index = this.pad(i, 3);
var scriptUrl = this.scripts[i].url;
Expand All @@ -57,7 +58,7 @@ DebuggerClient.prototype.getScripts = function(scriptId) {

// Each debugger command has an associated callback to get the result.

DebuggerClient.prototype.getScriptSource = function(scriptId) {
DebuggerClient.prototype.getScriptSource = function (scriptId) {
// If the id looks like an index into the local script array, look it up there.
if (scriptId >= 0 && scriptId < this.scripts.length) {
scriptId = this.scripts[scriptId].scriptId;
Expand All @@ -69,7 +70,7 @@ DebuggerClient.prototype.getScriptSource = function(scriptId) {
this.sendCommand(method, params, callback);
}

DebuggerClient.prototype.getScriptSourceCallback = function(result) {
DebuggerClient.prototype.getScriptSourceCallback = function (result) {
var scriptSource = result.scriptSource;
var lines = scriptSource.split('\n');
for (var i = 0; i < lines.length; i++) {
Expand All @@ -78,7 +79,7 @@ DebuggerClient.prototype.getScriptSourceCallback = function(result) {
}
}

DebuggerClient.prototype.evaluate = function(expression, callback) {
DebuggerClient.prototype.evaluate = function (expression, callback) {
var method = 'Runtime.evaluate';
var params = {};
params.contextId = this.executionContext;
Expand All @@ -92,8 +93,8 @@ DebuggerClient.prototype.evaluate = function(expression, callback) {

// All debugger commands are routed through this method. Converts the command
// parameters into a JSON string to pass to the debug dispatcher.
DebuggerClient.prototype.sendCommand = function(method, commandParams,
callback) {
DebuggerClient.prototype.sendCommand = function (method, commandParams,
callback) {
var jsonParams = JSON.stringify(commandParams);
var responseCallback = this.responseCallback.bind(this, method, callback);
debugHub.sendCommand(method, jsonParams, responseCallback);
Expand All @@ -102,15 +103,15 @@ DebuggerClient.prototype.sendCommand = function(method, commandParams,
// All command responses are routed through this method. Parses the JSON
// response from the debug dispatcher, checks for errors and passes on to the
// command-specific callback to handle the result.
DebuggerClient.prototype.responseCallback = function(method, callback,
responseString) {
DebuggerClient.prototype.responseCallback = function (method, callback,
responseString) {
var response = JSON.parse(responseString);

if (response && response.error) {
printToMessageLog(
MessageLog.ERROR,
'[ERROR(' + response.error.code + '):' + method + '] ' +
response.error.message);
MessageLog.ERROR,
'[ERROR(' + response.error.code + '):' + method + '] ' +
response.error.message);
} else if (callback) {
if (response) {
callback(response.result);
Expand All @@ -122,10 +123,10 @@ DebuggerClient.prototype.responseCallback = function(method, callback,

//-- Events.

DebuggerClient.prototype.onAttach = function() {
DebuggerClient.prototype.onAttach = function () {
if (debugHub.lastError) {
printToMessageLog(MessageLog.WARNING, 'Could not attach to debugger.');
this.attachState = this.DEBUGGER_DETACHED;
this.attachState = this.DEBUGGER_DISABLED;
} else {
printToMessageLog(MessageLog.INTERACTIVE, 'Debugger attached.');
this.attachState = this.DEBUGGER_ATTACHED;
Expand All @@ -135,7 +136,7 @@ DebuggerClient.prototype.onAttach = function() {
// All events generated by the debug dispatcher are routed through this method.
// Parses the JSON string and passes on to the appropriate handler according to
// the method name.
DebuggerClient.prototype.onEvent = function(method, paramString) {
DebuggerClient.prototype.onEvent = function (method, paramString) {
var params = JSON.parse(paramString);
if (method == 'Console.messageAdded') {
this.onConsoleMessageAdded(params)
Expand All @@ -152,32 +153,32 @@ DebuggerClient.prototype.onEvent = function(method, paramString) {
}
}

DebuggerClient.prototype.onDetached = function() {
DebuggerClient.prototype.onDetached = function () {
printToMessageLog(MessageLog.INTERACTIVE, 'Debugger detached.');
this.attachState = this.DEBUGGER_DETACHED;
}

DebuggerClient.prototype.onExecutionContextCreated = function(params) {
DebuggerClient.prototype.onExecutionContextCreated = function (params) {
this.executionContext = params.context.id;
printToMessageLog(MessageLog.INFO,
'Execution context created: ' + this.executionContext);
'Execution context created: ' + this.executionContext);
}

DebuggerClient.prototype.onLogEntryAdded = function(params) {
DebuggerClient.prototype.onLogEntryAdded = function (params) {
printToMessageLog(params.entry.level, params.entry.text);
}


DebuggerClient.prototype.onConsoleMessageAdded = function(params) {
DebuggerClient.prototype.onConsoleMessageAdded = function (params) {
// Translate Console.messageAdded params to Runtime.consoleAPICalled params.
var consoleApiParams = {
type: params.message.level,
args: [ { type: 'string', value: params.message.text } ]
args: [{ type: 'string', value: params.message.text }]
};
this.onConsoleApiCalled(consoleApiParams);
}

DebuggerClient.prototype.onConsoleApiCalled = function(params) {
DebuggerClient.prototype.onConsoleApiCalled = function (params) {
var severity = params.type;
if (severity === "assert") {
severity = MessageLog.ERROR;
Expand All @@ -200,13 +201,13 @@ DebuggerClient.prototype.onConsoleApiCalled = function(params) {
printToMessageLog(MessageLog.CONSOLE + severity, message);
}

DebuggerClient.prototype.onScriptParsed = function(params) {
DebuggerClient.prototype.onScriptParsed = function (params) {
this.scripts.push(params);
}

//--- Utils.

DebuggerClient.prototype.pad = function(number, minLength) {
DebuggerClient.prototype.pad = function (number, minLength) {
var result = number.toString();
while (result.length < minLength) {
result = ' ' + result;
Expand Down
Loading

0 comments on commit df40dfd

Please sign in to comment.