Skip to content

Commit

Permalink
Revert of mandoline/mojo: Make ApplicaitonImpl connect to tracing dur…
Browse files Browse the repository at this point in the history
…ing startup. (patchset chromium#9 id:160001 of https://codereview.chromium.org/1440403002/ )

Reason for revert:
Breaks on android due to lifetime issues. This approach isn't going to work.

Original issue's description:
> mandoline/mojo: Make ApplicaitonImpl connect to tracing during startup.
>
> This removes explicit initialization of the tracing system from
> individual mojo apps and moves it to ApplicationImpl, which always
> runs.
>
> BUG=534895
>
> Committed: https://crrev.com/64370e5eaf911e665b991361b0da1f9d5baa95b7
> Cr-Commit-Position: refs/heads/master@{#360174}

TBR=sky@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=534895, 557467

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

Cr-Commit-Position: refs/heads/master@{#360377}
  • Loading branch information
eglaysher authored and Commit bot committed Nov 18, 2015
1 parent 17ec244 commit 4706d07
Show file tree
Hide file tree
Showing 33 changed files with 80 additions and 109 deletions.
1 change: 1 addition & 0 deletions components/html_viewer/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@ 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: 2 additions & 0 deletions components/html_viewer/global_state.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#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 @@ -78,6 +79,7 @@ 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: 3 additions & 0 deletions components/html_viewer/global_state.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#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 @@ -117,6 +118,8 @@ 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/application/public/cpp/switches.h"
#include "mojo/services/tracing/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(
mojo::kEnableStatsCollectionBindings)) {
tracing::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(
mojo::kEnableStatsCollectionBindings)) {
tracing::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(
mojo::kEnableStatsCollectionBindings));
tracing::kEnableStatsCollectionBindings));

static bool startup_histogram_initialized = false;
if (!startup_histogram_initialized) {
Expand Down
1 change: 1 addition & 0 deletions components/mus/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ 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: 1 addition & 0 deletions components/mus/mus_app.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#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: 1 addition & 0 deletions components/mus/mus_app.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#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: 1 addition & 0 deletions components/mus/surfaces/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ 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: 1 addition & 0 deletions components/mus/ws/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ 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: 1 addition & 0 deletions components/pdf_viewer/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ 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: 7 additions & 0 deletions components/pdf_viewer/pdf_viewer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#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 @@ -340,6 +341,10 @@ 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 @@ -352,6 +357,8 @@ class PDFViewer : public mojo::ApplicationDelegate,
new ContentHandlerImpl(request.Pass());
}

mojo::TracingImpl tracing_;

DISALLOW_COPY_AND_ASSIGN(PDFViewer);
};
} // namespace
Expand Down
1 change: 1 addition & 0 deletions mandoline/services/core_services/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ 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,6 +17,7 @@
#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 @@ -98,6 +99,7 @@ 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,6 +13,7 @@
#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 @@ -51,6 +52,7 @@ class CoreServicesApplicationDelegate
mojo::WeakBindingSet<ContentHandler> handler_bindings_;

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

base::WeakPtrFactory<CoreServicesApplicationDelegate> weak_factory_;

Expand Down
1 change: 1 addition & 0 deletions mandoline/ui/desktop_ui/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ 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(
mojo::kEnableStatsCollectionBindings)) {
tracing::kEnableStatsCollectionBindings)) {
mojo::URLRequestPtr request(mojo::URLRequest::New());
request->url = mojo::String::From("mojo:tracing");
tracing::StartupPerformanceDataCollectorPtr collector;
Expand Down
7 changes: 0 additions & 7 deletions mojo/application/public/cpp/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,8 @@ 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 @@ -58,7 +52,6 @@ source_set("sources") {
"//mojo/message_pump",
"//mojo/public/cpp/bindings",
"//mojo/public/cpp/system",
"//mojo/services/tracing/public/interfaces",
]
}

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

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 @@ -152,7 +150,6 @@ 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: 0 additions & 20 deletions mojo/application/public/cpp/lib/application_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,13 @@
#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 @@ -109,23 +106,6 @@ 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
7 changes: 0 additions & 7 deletions mojo/mojo_base.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -241,14 +241,8 @@
'application/public/cpp/lib/service_provider_impl.cc',
'application/public/cpp/lib/service_registry.cc',
'application/public/cpp/lib/service_registry.h',
'application/public/cpp/lib/trace_provider_impl.cc',
'application/public/cpp/lib/trace_provider_impl.h',
'application/public/cpp/lib/tracing_impl.cc',
'application/public/cpp/lib/tracing_impl.h',
'application/public/cpp/service_connector.h',
'application/public/cpp/service_provider_impl.h',
'application/public/cpp/switches.cc',
'application/public/cpp/switches.h',
],
'dependencies': [
'mojo_application_bindings',
Expand All @@ -262,7 +256,6 @@
'dependencies': [
'mojo_application_bindings_mojom',
'mojo_services.gyp:network_service_bindings_lib',
'mojo_services.gyp:tracing_service_bindings_lib',
'../third_party/mojo/mojo_public.gyp:mojo_cpp_bindings',
],
'export_dependent_settings': [
Expand Down
21 changes: 0 additions & 21 deletions mojo/mojo_services.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -49,27 +49,6 @@
'network_service_bindings_mojom',
],
},
{
'target_name': 'tracing_service_bindings_mojom',
'type': 'none',
'variables': {
'mojom_files': [
'services/tracing/public/interfaces/tracing.mojom',
],
'mojom_include_path': '<(DEPTH)/mojo/services',
},
'includes': [
'../third_party/mojo/mojom_bindings_generator_explicit.gypi',
],
},
{
# GN version: //mojo/services/tracing/public/interfaces
'target_name': 'tracing_service_bindings_lib',
'type': 'static_library',
'dependencies': [
'tracing_service_bindings_mojom',
],
},
{
'target_name': 'updater_bindings_mojom',
'type': 'none',
Expand Down
1 change: 1 addition & 0 deletions mojo/runner/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ source_set("lib") {
"//mojo/runner/child:interfaces",
"//mojo/runner/host:lib",
"//mojo/services/network/public/interfaces",
"//mojo/services/tracing/public/cpp",
"//mojo/services/tracing/public/interfaces",
"//mojo/shell",
"//mojo/util:filename_util",
Expand Down
Loading

0 comments on commit 4706d07

Please sign in to comment.