Tidy up the debugger. Remove dependence on WindowManager. Remove unused profiling option. Fix crash in trace collector when it receives multiple connections. R=abarth@google.com, ppi@chromium.org Review URL: https://codereview.chromium.org/1532893003 .
diff --git a/mojo/devtools/common/mojo_debug b/mojo/devtools/common/mojo_debug index b3b786d..dc1000b 100755 --- a/mojo/devtools/common/mojo_debug +++ b/mojo/devtools/common/mojo_debug
@@ -109,27 +109,6 @@ stop_tracing_parser.set_defaults(func=_tracing_stop) -def _wm_load(args): - """Loads (embeds) the given url in the window manager.""" - if not _send_request('load', args.url): - return 1 - return 0 - - -def _add_wm_command(subparsers): - """Sets up the parser for the 'wm' command.""" - wm_parser = subparsers.add_parser('wm', help='window manager (requires ' - 'debugger.mojo)') - wm_subparser = wm_parser.add_subparsers( - help='the command to run') - - wm_load_parser = wm_subparser.add_parser('load', - help='load (embed) the given url') - wm_load_parser.add_argument('url', type=str, - help='the url to load') - wm_load_parser.set_defaults(func=_wm_load) - - def _device_stack(args): """Runs the device logcat through android_stack_parser.""" adb_path = args.adb_path if args.adb_path else 'adb' @@ -325,7 +304,6 @@ subparsers = parser.add_subparsers(help='the tool to run') _add_device_command(subparsers) _add_tracing_command(subparsers) - _add_wm_command(subparsers) _add_gdb_command(subparsers) args = parser.parse_args()
diff --git a/services/debugger/BUILD.gn b/services/debugger/BUILD.gn index ebf0d42..e574561 100644 --- a/services/debugger/BUILD.gn +++ b/services/debugger/BUILD.gn
@@ -25,6 +25,5 @@ "//mojo/services/http_server/interfaces", "//mojo/services/network/interfaces", "//mojo/services/tracing/interfaces", - "//mojo/services/window_manager/interfaces", ] }
diff --git a/services/debugger/debugger.cc b/services/debugger/debugger.cc index 059c411..4150ade 100644 --- a/services/debugger/debugger.cc +++ b/services/debugger/debugger.cc
@@ -19,7 +19,6 @@ #include "mojo/services/http_server/interfaces/http_server_factory.mojom.h" #include "mojo/services/network/interfaces/net_address.mojom.h" #include "mojo/services/tracing/interfaces/tracing.mojom.h" -#include "mojo/services/window_manager/interfaces/window_manager.mojom.h" #include "services/debugger/trace_collector.h" // Debugger is a Mojo application that exposes an http server and talks to other @@ -30,7 +29,7 @@ namespace debugger { class Debugger : public mojo::ApplicationDelegate, - public http_server::HttpHandler { + public http_server::HttpHandler { public: Debugger() : is_tracing_(false), app_(nullptr), handler_binding_(this) {} ~Debugger() override {} @@ -46,11 +45,6 @@ mojo::ApplicationImpl::Terminate(); return; } - if (app->args().size() == 3 && app->args()[2] == "--wm") { - // Connect to window manager only if requested, as the user might want to - // run the debugger without spawning one. - app_->ConnectToService("mojo:window_manager", &window_manager_); - } base::StringToUint(app->args()[1], &command_port_); http_server::HttpServerFactoryPtr http_server_factory; app->ConnectToService("mojo:http_server", &http_server_factory); @@ -83,18 +77,8 @@ const HandleRequestCallback& callback) override { // FIXME: We should use use a fancier lookup system more like what // services/http_server/http_server.cc does with AddHandler. - if (request->relative_url == "/reload") { - Load(callback, url_); - } else if (request->relative_url == "/quit") { + if (request->relative_url == "/quit") { Exit(); - } else if (request->relative_url == "/load") { - std::string url; - mojo::common::BlockingCopyToString(request->body.Pass(), &url); - Load(callback, url); - } else if (request->relative_url == "/start_profiling") { - StartProfiling(callback); - } else if (request->relative_url == "/stop_profiling") { - StopProfiling(callback); } else if (request->relative_url == "/start_tracing") { StartTracing(callback); } else if (request->relative_url == "/stop_tracing") { @@ -114,35 +98,17 @@ void Help(const HandleRequestCallback& callback, std::string path) { std::string help = base::StringPrintf( - "Sky Debugger running on port %d\n" + "Debugger running on port %d\n" "Supported URLs:\n" - "/reload -- Reload the current page\n" "/quit -- Quit\n" - "/load -- Load a new URL, url in POST body.\n", + "/start_tracing -- Start Tracing\n" + "/stop_tracing -- Stop Tracing\n", command_port_); if (path != "/") help = "Unknown path: " + path + "\n\n" + help; Respond(callback, help); } - void Load(const HandleRequestCallback& callback, std::string url) { - url_ = url; - Reload(); - std::string response = std::string("Loaded ") + url + "\n"; - Respond(callback, response); - } - - void Reload() { - if (!window_manager_) { - // If window_manager_ was not connected to eagerly on startup, we do that - // on the first demand. - app_->ConnectToService("mojo:window_manager", &window_manager_); - } - - // SimpleWindowManager will wire up necessary services on our behalf. - window_manager_->Embed(url_, nullptr, nullptr); - } - void Exit() { // TODO(eseidel): We should orderly shutdown once mojo can. exit(0); @@ -180,31 +146,9 @@ Respond(callback, trace); } - void StartProfiling(const HandleRequestCallback& callback) { -#if !defined(NDEBUG) || !defined(ENABLE_PROFILING) - Error(callback, - "Profiling requires is_debug=false and enable_profiling=true"); - return; -#else - base::debug::StartProfiling("sky_viewer.pprof"); - Respond(callback, "Starting profiling (stop with 'stop_profiling')"); -#endif - } - - void StopProfiling(const HandleRequestCallback& callback) { - if (!base::debug::BeingProfiled()) { - Error(callback, "Profiling not started"); - return; - } - base::debug::StopProfiling(); - Respond(callback, "Stopped profiling"); - } - bool is_tracing_; mojo::ApplicationImpl* app_; - mojo::WindowManagerPtr window_manager_; tracing::TraceCollectorPtr tracing_; - std::string url_; uint32_t command_port_; http_server::HttpServerPtr http_server_;
diff --git a/services/tracing/tracing_app.cc b/services/tracing/tracing_app.cc index 8689d3a..62cec70 100644 --- a/services/tracing/tracing_app.cc +++ b/services/tracing/tracing_app.cc
@@ -10,11 +10,9 @@ namespace tracing { -TracingApp::TracingApp() : collector_binding_(this), tracing_active_(false) { -} +TracingApp::TracingApp() : collector_binding_(this), tracing_active_(false) {} -TracingApp::~TracingApp() { -} +TracingApp::~TracingApp() {} bool TracingApp::ConfigureIncomingConnection( mojo::ApplicationConnection* connection) { @@ -40,6 +38,11 @@ // mojo::InterfaceFactory<TraceCollector> implementation. void TracingApp::Create(mojo::ApplicationConnection* connection, mojo::InterfaceRequest<TraceCollector> request) { + if (collector_binding_.is_bound()) { + LOG(ERROR) << "Another application is already connected to tracing."; + return; + } + collector_binding_.Bind(request.Pass()); }