Skip to content

Commit

Permalink
mandoline/mojo: Make ApplicaitonImpl connect to tracing during startup.
Browse files Browse the repository at this point in the history
This removes explicit initialization of the tracing system from
individual mojo apps and moves it to ApplicationImpl, which always
runs.

BUG=534895

Review URL: https://codereview.chromium.org/1440403002

Cr-Commit-Position: refs/heads/master@{#360174}
  • Loading branch information
eglaysher authored and Commit bot committed Nov 17, 2015
1 parent 9a55fac commit 64370e5
Show file tree
Hide file tree
Showing 33 changed files with 109 additions and 80 deletions.
1 change: 0 additions & 1 deletion components/html_viewer/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,6 @@ source_set("lib") {
"//mojo/public/cpp/environment:environment",
"//mojo/services/network/public/cpp",
"//mojo/services/network/public/interfaces",
"//mojo/services/tracing/public/cpp",
"//mojo/services/tracing/public/interfaces",
"//net",
"//skia",
Expand Down
2 changes: 0 additions & 2 deletions components/html_viewer/global_state.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
#include "gin/v8_initializer.h"
#include "mojo/application/public/cpp/application_impl.h"
#include "mojo/logging/init_logging.h"
#include "mojo/services/tracing/public/cpp/tracing_impl.h"
#include "third_party/WebKit/public/web/WebKit.h"
#include "third_party/WebKit/public/web/WebRuntimeFeatures.h"
#include "ui/base/resource/resource_bundle.h"
Expand Down Expand Up @@ -79,7 +78,6 @@ GlobalState::GlobalState(mojo::ApplicationImpl* app)
discardable_memory_allocator_(kDesiredMaxMemory),
compositor_thread_("compositor thread"),
blink_settings_(new BlinkSettingsImpl()) {
tracing_.Initialize(app);
}

GlobalState::~GlobalState() {
Expand Down
3 changes: 0 additions & 3 deletions components/html_viewer/global_state.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
#include "components/mus/gles2/raster_thread_helper.h"
#include "components/mus/public/interfaces/gpu.mojom.h"
#include "components/resource_provider/public/cpp/resource_loader.h"
#include "mojo/services/tracing/public/cpp/tracing_impl.h"
#include "skia/ext/refptr.h"
#include "ui/gfx/geometry/size.h"

Expand Down Expand Up @@ -118,8 +117,6 @@ class GlobalState {
// memory based purging allocator working here.
DiscardableMemoryAllocator discardable_memory_allocator_;

mojo::TracingImpl tracing_;

scoped_ptr<scheduler::RendererScheduler> renderer_scheduler_;
scoped_ptr<BlinkPlatformImpl> blink_platform_;
base::Thread compositor_thread_;
Expand Down
8 changes: 4 additions & 4 deletions components/html_viewer/stats_collection_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
#include "gin/handle.h"
#include "gin/object_template_builder.h"
#include "mojo/application/public/cpp/application_impl.h"
#include "mojo/services/tracing/public/cpp/switches.h"
#include "mojo/application/public/cpp/switches.h"
#include "third_party/WebKit/public/web/WebKit.h"
#include "third_party/WebKit/public/web/WebLocalFrame.h"

Expand Down Expand Up @@ -67,7 +67,7 @@ tracing::StartupPerformanceDataCollectorPtr StatsCollectionController::Install(
// Only make startup tracing available when running in the context of a test.
if (!app ||
!base::CommandLine::ForCurrentProcess()->HasSwitch(
tracing::kEnableStatsCollectionBindings)) {
mojo::kEnableStatsCollectionBindings)) {
return nullptr;
}

Expand Down Expand Up @@ -105,7 +105,7 @@ StatsCollectionController::ConnectToDataCollector(mojo::ApplicationImpl* app) {
// Only make startup tracing available when running in the context of a test.
if (!app ||
!base::CommandLine::ForCurrentProcess()->HasSwitch(
tracing::kEnableStatsCollectionBindings)) {
mojo::kEnableStatsCollectionBindings)) {
return nullptr;
}

Expand Down Expand Up @@ -134,7 +134,7 @@ gin::ObjectTemplateBuilder StatsCollectionController::GetObjectTemplateBuilder(
std::string StatsCollectionController::GetHistogram(
const std::string& histogram_name) {
DCHECK(base::CommandLine::ForCurrentProcess()->HasSwitch(
tracing::kEnableStatsCollectionBindings));
mojo::kEnableStatsCollectionBindings));

static bool startup_histogram_initialized = false;
if (!startup_histogram_initialized) {
Expand Down
1 change: 0 additions & 1 deletion components/mus/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@ source_set("lib") {
"//components/mus/ws:lib",
"//mojo/application/public/cpp",
"//mojo/common:common_base",
"//mojo/services/tracing/public/cpp",
"//ui/events",
"//ui/gl:gl",
"//ui/platform_window:platform_impls",
Expand Down
1 change: 0 additions & 1 deletion components/mus/mus_app.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
#include "mojo/application/public/cpp/application_impl.h"
#include "mojo/application/public/cpp/application_runner.h"
#include "mojo/public/c/system/main.h"
#include "mojo/services/tracing/public/cpp/tracing_impl.h"
#include "ui/events/event_switches.h"
#include "ui/events/platform/platform_event_source.h"
#include "ui/gl/gl_surface.h"
Expand Down
1 change: 0 additions & 1 deletion components/mus/mus_app.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
#include "mojo/application/public/cpp/application_delegate.h"
#include "mojo/application/public/cpp/interface_factory.h"
#include "mojo/common/weak_binding_set.h"
#include "mojo/services/tracing/public/cpp/tracing_impl.h"

namespace mojo {
class ApplicationImpl;
Expand Down
1 change: 0 additions & 1 deletion components/mus/surfaces/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ source_set("surfaces") {
"//mojo/application/public/cpp",
"//mojo/converters/geometry",
"//mojo/converters/surfaces",
"//mojo/services/tracing/public/cpp",
"//ui/gfx",
"//ui/gl",
"//ui/mojo/geometry:interfaces",
Expand Down
1 change: 0 additions & 1 deletion components/mus/ws/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ source_set("lib") {
"//mojo/converters/input_events",
"//mojo/converters/surfaces",
"//mojo/public/cpp/bindings:callback",
"//mojo/services/tracing/public/cpp",
"//ui/events",
"//ui/events/platform",
"//ui/gfx",
Expand Down
1 change: 0 additions & 1 deletion components/pdf_viewer/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ mojo_native_application("pdf_viewer") {
"//mojo/public/cpp/bindings",
"//mojo/services/network/public/cpp",
"//mojo/services/network/public/interfaces",
"//mojo/services/tracing/public/cpp",
"//mojo/services/tracing/public/interfaces",
"//third_party/pdfium",
"//ui/gfx/geometry",
Expand Down
7 changes: 0 additions & 7 deletions components/pdf_viewer/pdf_viewer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
#include "mojo/common/data_pipe_utils.h"
#include "mojo/public/c/system/main.h"
#include "mojo/public/cpp/bindings/binding.h"
#include "mojo/services/tracing/public/cpp/tracing_impl.h"
#include "third_party/pdfium/public/fpdf_ext.h"
#include "third_party/pdfium/public/fpdfview.h"
#include "ui/gfx/geometry/rect.h"
Expand Down Expand Up @@ -341,10 +340,6 @@ class PDFViewer : public mojo::ApplicationDelegate,

private:
// ApplicationDelegate:
void Initialize(mojo::ApplicationImpl* app) override {
tracing_.Initialize(app);
}

bool ConfigureIncomingConnection(
mojo::ApplicationConnection* connection) override {
connection->AddService(this);
Expand All @@ -357,8 +352,6 @@ class PDFViewer : public mojo::ApplicationDelegate,
new ContentHandlerImpl(request.Pass());
}

mojo::TracingImpl tracing_;

DISALLOW_COPY_AND_ASSIGN(PDFViewer);
};
} // namespace
Expand Down
1 change: 0 additions & 1 deletion mandoline/services/core_services/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ source_set("sources") {
"//mojo/message_pump",
"//mojo/public/cpp/bindings",
"//mojo/services/tracing:lib",
"//mojo/services/tracing/public/cpp",
"//third_party/icu",
"//url",
]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
#include "mojo/application/public/cpp/application_runner.h"
#include "mojo/logging/init_logging.h"
#include "mojo/message_pump/message_pump_mojo.h"
#include "mojo/services/tracing/public/cpp/tracing_impl.h"
#include "mojo/services/tracing/tracing_app.h"
#include "url/gurl.h"

Expand Down Expand Up @@ -99,7 +98,6 @@ void CoreServicesApplicationDelegate::ApplicationThreadDestroyed(
void CoreServicesApplicationDelegate::Initialize(mojo::ApplicationImpl* app) {
base::PlatformThread::SetName("CoreServicesDispatcher");
mojo::logging::InitLogging();
tracing_.Initialize(app);
}

bool CoreServicesApplicationDelegate::ConfigureIncomingConnection(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
#include "mojo/application/public/cpp/interface_factory_impl.h"
#include "mojo/application/public/interfaces/content_handler.mojom.h"
#include "mojo/common/weak_binding_set.h"
#include "mojo/services/tracing/public/cpp/tracing_impl.h"

namespace core_services {

Expand Down Expand Up @@ -52,7 +51,6 @@ class CoreServicesApplicationDelegate
mojo::WeakBindingSet<ContentHandler> handler_bindings_;

ScopedVector<ApplicationThread> application_threads_;
mojo::TracingImpl tracing_;

base::WeakPtrFactory<CoreServicesApplicationDelegate> weak_factory_;

Expand Down
1 change: 0 additions & 1 deletion mandoline/ui/desktop_ui/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ source_set("lib") {
"//mojo/common:common_base",
"//mojo/converters/geometry",
"//mojo/public/cpp/bindings",
"//mojo/services/tracing/public/cpp",
"//mojo/services/tracing/public/interfaces",
"//skia",
"//ui/gfx",
Expand Down
4 changes: 2 additions & 2 deletions mandoline/ui/desktop_ui/browser_window.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@
#include "mandoline/ui/desktop_ui/find_bar_view.h"
#include "mandoline/ui/desktop_ui/public/interfaces/omnibox.mojom.h"
#include "mandoline/ui/desktop_ui/toolbar_view.h"
#include "mojo/application/public/cpp/switches.h"
#include "mojo/common/common_type_converters.h"
#include "mojo/converters/geometry/geometry_type_converters.h"
#include "mojo/services/tracing/public/cpp/switches.h"
#include "mojo/services/tracing/public/interfaces/tracing.mojom.h"
#include "ui/gfx/canvas.h"
#include "ui/mojo/init/ui_init.h"
Expand Down Expand Up @@ -241,7 +241,7 @@ void BrowserWindow::OnEmbed(mus::Window* root) {
static bool recorded_browser_startup_metrics = false;
if (!recorded_browser_startup_metrics &&
base::CommandLine::ForCurrentProcess()->HasSwitch(
tracing::kEnableStatsCollectionBindings)) {
mojo::kEnableStatsCollectionBindings)) {
mojo::URLRequestPtr request(mojo::URLRequest::New());
request->url = mojo::String::From("mojo:tracing");
tracing::StartupPerformanceDataCollectorPtr collector;
Expand Down
7 changes: 7 additions & 0 deletions mojo/application/public/cpp/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,14 @@ source_set("sources") {
"lib/service_provider_impl.cc",
"lib/service_registry.cc",
"lib/service_registry.h",
"lib/trace_provider_impl.cc",
"lib/trace_provider_impl.h",
"lib/tracing_impl.cc",
"lib/tracing_impl.h",
"service_connector.h",
"service_provider_impl.h",
"switches.cc",
"switches.h",
]

deps = [
Expand All @@ -52,6 +58,7 @@ source_set("sources") {
"//mojo/message_pump",
"//mojo/public/cpp/bindings",
"//mojo/public/cpp/system",
"//mojo/services/tracing/public/interfaces",
]
}

Expand Down
3 changes: 3 additions & 0 deletions mojo/application/public/cpp/application_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@

namespace mojo {

class TracingImpl;

// TODO(beng): This comment is hilariously out of date.
// Utility class for communicating with the Shell, and providing Services
// to clients.
Expand Down Expand Up @@ -150,6 +152,7 @@ class ApplicationImpl : public Application {
ApplicationDelegate* delegate_;
Binding<Application> binding_;
ShellPtr shell_;
scoped_ptr<TracingImpl> tracing_impl_;
std::string url_;
Closure termination_closure_;
AppLifetimeHelper app_lifetime_helper_;
Expand Down
20 changes: 20 additions & 0 deletions mojo/application/public/cpp/lib/application_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,16 @@
#include "base/message_loop/message_loop.h"
#include "mojo/application/public/cpp/application_delegate.h"
#include "mojo/application/public/cpp/lib/service_registry.h"
#include "mojo/application/public/cpp/lib/tracing_impl.h"
#include "mojo/public/cpp/bindings/interface_ptr.h"
#include "mojo/public/cpp/environment/logging.h"

namespace mojo {

namespace {

bool g_has_tracing_service = false;

void DefaultTerminationClosure() {
if (base::MessageLoop::current() &&
base::MessageLoop::current()->is_running())
Expand Down Expand Up @@ -106,6 +109,23 @@ void ApplicationImpl::Initialize(ShellPtr shell, const mojo::String& url) {
shell_ = shell.Pass();
shell_.set_connection_error_handler([this]() { OnConnectionError(); });
url_ = url;

if (!g_has_tracing_service) {
// Each copy of base in our process must have one tracing service,
// otherwise data will be double counted or not counted. When we load a
// mojo application, either in process or creating a child process, the
// copy of base loaded needs to be connected to the tracing service. It's
// the responsibility of the first application to connect to the tracing
// service to establish this connection.
//
// This is safe because if this is a ContentHandler, it will outlive all
// its served Applications. If this is a raw mojo application, it is the
// only Application served.
tracing_impl_.reset(new TracingImpl);
tracing_impl_->Initialize(this);
g_has_tracing_service = true;
}

delegate_->Initialize(this);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "mojo/services/tracing/public/cpp/trace_provider_impl.h"
#include "mojo/application/public/cpp/lib/trace_provider_impl.h"

#include "base/callback.h"
#include "base/logging.h"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef MOJO_SERVICES_TRACING_PUBLIC_CPP_TRACE_PROVIDER_IMPL_H_
#define MOJO_SERVICES_TRACING_PUBLIC_CPP_TRACE_PROVIDER_IMPL_H_
#ifndef MOJO_APPLICATION_PUBLIC_CPP_LIB_TRACE_PROVIDER_IMPL_H_
#define MOJO_APPLICATION_PUBLIC_CPP_LIB_TRACE_PROVIDER_IMPL_H_

#include "base/macros.h"
#include "base/memory/ref_counted_memory.h"
Expand Down Expand Up @@ -48,4 +48,4 @@ class TraceProviderImpl : public tracing::TraceProvider {

} // namespace mojo

#endif // MOJO_SERVICES_TRACING_PUBLIC_CPP_TRACE_PROVIDER_IMPL_H_
#endif // MOJO_APPLICATION_PUBLIC_CPP_LIB_TRACE_PROVIDER_IMPL_H_
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "mojo/services/tracing/public/cpp/tracing_impl.h"
#include "mojo/application/public/cpp/lib/tracing_impl.h"

#include "base/trace_event/trace_event_impl.h"
#include "mojo/application/public/cpp/application_impl.h"

#ifdef NDEBUG
#include "base/command_line.h"
#include "mojo/services/tracing/public/cpp/switches.h"
#include "mojo/application/public/cpp/switches.h"
#endif

namespace mojo {
Expand All @@ -27,8 +27,7 @@ void TracingImpl::Initialize(ApplicationImpl* app) {
connection_->AddService(this);

#ifdef NDEBUG
if (base::CommandLine::ForCurrentProcess()->HasSwitch(
tracing::kEarlyTracing)) {
if (base::CommandLine::ForCurrentProcess()->HasSwitch(kEarlyTracing)) {
provider_impl_.ForceEnableTracing();
}
#else
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef MOJO_SERVICES_TRACING_PUBLIC_CPP_TRACING_IMPL_H_
#define MOJO_SERVICES_TRACING_PUBLIC_CPP_TRACING_IMPL_H_
#ifndef MOJO_APPLICATION_PUBLIC_CPP_LIB_TRACING_IMPL_H_
#define MOJO_APPLICATION_PUBLIC_CPP_LIB_TRACING_IMPL_H_

#include "base/macros.h"
#include "mojo/application/public/cpp/interface_factory.h"
#include "mojo/services/tracing/public/cpp/trace_provider_impl.h"
#include "mojo/application/public/cpp/lib/trace_provider_impl.h"
#include "mojo/services/tracing/public/interfaces/tracing.mojom.h"

namespace mojo {
Expand Down Expand Up @@ -37,4 +37,4 @@ class TracingImpl : public InterfaceFactory<tracing::TraceProvider> {

} // namespace mojo

#endif // MOJO_SERVICES_TRACING_PUBLIC_CPP_TRACING_IMPL_H_
#endif // MOJO_APPLICATION_PUBLIC_CPP_LIB_TRACING_IMPL_H_
Loading

0 comments on commit 64370e5

Please sign in to comment.