New issue
Advanced search Search tips

Issue 914579 link

Starred by 1 user

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug



Sign in to add a comment

Perfetto/tracing: Run tracing as an on-demand sandboxed service

Project Member Reported by oysteine@chromium.org, Dec 12

Issue description

The tracing service can easily run sandboxed, which is great for security purposes. This bug tracks the work needed to make it run on-demand, otherwise there will always be a process running (hosting the service) even when tracing is not in use.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Dec 19

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7655e3cfd19c89d80653379df75c15f14c8c4c9e

commit 7655e3cfd19c89d80653379df75c15f14c8c4c9e
Author: Oystein Eftevaag <oysteine@chromium.org>
Date: Wed Dec 19 18:20:19 2018

Tracing: Make TraceEventAgent a singleton

This class could always ever be have one instance alive (enforced
by DCHECK on a global) anyway.

With this, we:
* Make it an actual singleton, simplifying the usage.
* Separate out the Mojo connection steps from construction.

These are prerequisites to be able to only run the tracing service on-demand
when tracing is enabled, which in turn is necessary for running it
sandboxed. Specifically TraceEventAgent is now a singleton as the
tracing service (when it starts up) will tell each other service that it
should connect back to it, and as which services share which processes
is hidden from the services as an implementation detail, there's no one
clear ownership of the TraceEventAgent for the whole process (i.e. each
service will tell the TraceEventAgent that it's time to connect, but
internally this will only happen once for the process).

R=chiniforooshan@chromium.org

Bug: 914579
Change-Id: I207af20bc5cbf7306b836ffc82632907ac8fa1b4
Reviewed-on: https://chromium-review.googlesource.com/c/1374836
Commit-Queue: oysteine <oysteine@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>
Reviewed-by: Ehsan Chiniforooshan <chiniforooshan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#617881}
[modify] https://crrev.com/7655e3cfd19c89d80653379df75c15f14c8c4c9e/chrome/browser/chromeos/arc/tracing/arc_tracing_bridge.cc
[modify] https://crrev.com/7655e3cfd19c89d80653379df75c15f14c8c4c9e/content/browser/tracing/background_tracing_manager_impl.cc
[modify] https://crrev.com/7655e3cfd19c89d80653379df75c15f14c8c4c9e/content/browser/tracing/cast_tracing_agent.cc
[modify] https://crrev.com/7655e3cfd19c89d80653379df75c15f14c8c4c9e/content/browser/tracing/cast_tracing_agent.h
[modify] https://crrev.com/7655e3cfd19c89d80653379df75c15f14c8c4c9e/content/browser/tracing/cros_tracing_agent.cc
[modify] https://crrev.com/7655e3cfd19c89d80653379df75c15f14c8c4c9e/content/browser/tracing/cros_tracing_agent.h
[modify] https://crrev.com/7655e3cfd19c89d80653379df75c15f14c8c4c9e/content/browser/tracing/tracing_controller_browsertest.cc
[modify] https://crrev.com/7655e3cfd19c89d80653379df75c15f14c8c4c9e/content/browser/tracing/tracing_controller_impl.cc
[modify] https://crrev.com/7655e3cfd19c89d80653379df75c15f14c8c4c9e/content/browser/tracing/tracing_controller_impl.h
[modify] https://crrev.com/7655e3cfd19c89d80653379df75c15f14c8c4c9e/content/child/child_thread_impl.cc
[modify] https://crrev.com/7655e3cfd19c89d80653379df75c15f14c8c4c9e/content/child/child_thread_impl.h
[modify] https://crrev.com/7655e3cfd19c89d80653379df75c15f14c8c4c9e/services/tracing/public/cpp/base_agent.cc
[modify] https://crrev.com/7655e3cfd19c89d80653379df75c15f14c8c4c9e/services/tracing/public/cpp/base_agent.h
[modify] https://crrev.com/7655e3cfd19c89d80653379df75c15f14c8c4c9e/services/tracing/public/cpp/perfetto/producer_client.cc
[modify] https://crrev.com/7655e3cfd19c89d80653379df75c15f14c8c4c9e/services/tracing/public/cpp/perfetto/producer_client.h
[modify] https://crrev.com/7655e3cfd19c89d80653379df75c15f14c8c4c9e/services/tracing/public/cpp/trace_event_agent.cc
[modify] https://crrev.com/7655e3cfd19c89d80653379df75c15f14c8c4c9e/services/tracing/public/cpp/trace_event_agent.h
[modify] https://crrev.com/7655e3cfd19c89d80653379df75c15f14c8c4c9e/services/tracing/public/cpp/trace_event_agent_unittest.cc

Project Member

Comment 2 by bugdroid1@chromium.org, Jan 9

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a4d570d27d1999a5e6071ad8cee3bd2b8f5f50ed

commit a4d570d27d1999a5e6071ad8cee3bd2b8f5f50ed
Author: Oystein Eftevaag <oysteine@chromium.org>
Date: Wed Jan 09 23:57:40 2019

Tracing: Use the compile-time builtin list of categories for about://tracing

This saves a roundtrip to all child-processes to request the list of
categories that each TraceLog has seen, and enables later work
to change the tracing service to be started on-demand.

TBR=nasko@chromium.org

Bug: 914579
Change-Id: I9ddaf9ad2d8cbc633e58bc9560a6ae405a76d5f5
Reviewed-on: https://chromium-review.googlesource.com/c/1401273
Commit-Queue: oysteine <oysteine@chromium.org>
Reviewed-by: Sami Kyöstilä <skyostil@chromium.org>
Reviewed-by: Eric Seckler <eseckler@chromium.org>
Reviewed-by: Yusuke Sato <yusukes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#621381}
[modify] https://crrev.com/a4d570d27d1999a5e6071ad8cee3bd2b8f5f50ed/base/trace_event/builtin_categories.h
[modify] https://crrev.com/a4d570d27d1999a5e6071ad8cee3bd2b8f5f50ed/base/trace_event/trace_event_unittest.cc
[modify] https://crrev.com/a4d570d27d1999a5e6071ad8cee3bd2b8f5f50ed/base/trace_event/trace_log.cc
[modify] https://crrev.com/a4d570d27d1999a5e6071ad8cee3bd2b8f5f50ed/base/trace_event/trace_log.h
[modify] https://crrev.com/a4d570d27d1999a5e6071ad8cee3bd2b8f5f50ed/chrome/browser/chromeos/arc/tracing/arc_tracing_bridge.cc
[modify] https://crrev.com/a4d570d27d1999a5e6071ad8cee3bd2b8f5f50ed/chrome/browser/chromeos/arc/tracing/arc_tracing_bridge.h
[modify] https://crrev.com/a4d570d27d1999a5e6071ad8cee3bd2b8f5f50ed/content/browser/tracing/cast_tracing_agent.cc
[modify] https://crrev.com/a4d570d27d1999a5e6071ad8cee3bd2b8f5f50ed/content/browser/tracing/cast_tracing_agent.h
[modify] https://crrev.com/a4d570d27d1999a5e6071ad8cee3bd2b8f5f50ed/content/browser/tracing/tracing_controller_impl.cc
[modify] https://crrev.com/a4d570d27d1999a5e6071ad8cee3bd2b8f5f50ed/services/tracing/coordinator.cc
[modify] https://crrev.com/a4d570d27d1999a5e6071ad8cee3bd2b8f5f50ed/services/tracing/coordinator.h
[modify] https://crrev.com/a4d570d27d1999a5e6071ad8cee3bd2b8f5f50ed/services/tracing/coordinator_unittest.cc
[modify] https://crrev.com/a4d570d27d1999a5e6071ad8cee3bd2b8f5f50ed/services/tracing/public/cpp/base_agent.cc
[modify] https://crrev.com/a4d570d27d1999a5e6071ad8cee3bd2b8f5f50ed/services/tracing/public/cpp/base_agent.h
[modify] https://crrev.com/a4d570d27d1999a5e6071ad8cee3bd2b8f5f50ed/services/tracing/public/cpp/trace_event_agent.cc
[modify] https://crrev.com/a4d570d27d1999a5e6071ad8cee3bd2b8f5f50ed/services/tracing/public/cpp/trace_event_agent.h
[modify] https://crrev.com/a4d570d27d1999a5e6071ad8cee3bd2b8f5f50ed/services/tracing/public/cpp/trace_event_agent_unittest.cc
[modify] https://crrev.com/a4d570d27d1999a5e6071ad8cee3bd2b8f5f50ed/services/tracing/public/mojom/tracing.mojom
[modify] https://crrev.com/a4d570d27d1999a5e6071ad8cee3bd2b8f5f50ed/services/tracing/test_util.cc
[modify] https://crrev.com/a4d570d27d1999a5e6071ad8cee3bd2b8f5f50ed/services/tracing/test_util.h

Project Member

Comment 3 by bugdroid1@chromium.org, Jan 10

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/298fb68368b7e60c73ce3154e8f46584702ced37

commit 298fb68368b7e60c73ce3154e8f46584702ced37
Author: Sami Kyostila <skyostil@chromium.org>
Date: Thu Jan 10 12:59:04 2019

v8: Remove trace event category warming

Since all categories are now statically defined, there's no need to warm
any of them up explicitly in order for the categories to show up in the
tracing UI.

Depends on https://chromium-review.googlesource.com/c/chromium/src/+/1401273.

Bug: chromium:914579
Change-Id: I8ae8977130ae89d6ee3351194ad258d13f3c14f4
Reviewed-on: https://chromium-review.googlesource.com/c/1402779
Reviewed-by: Alexei Filippov <alph@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Commit-Queue: Sami Kyöstilä <skyostil@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58699}
[modify] https://crrev.com/298fb68368b7e60c73ce3154e8f46584702ced37/src/profiler/tracing-cpu-profiler.cc
[modify] https://crrev.com/298fb68368b7e60c73ce3154e8f46584702ced37/src/tracing/tracing-category-observer.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Jan 11

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/eb0e029a36e458f83480607531122dd514f4ded7

commit eb0e029a36e458f83480607531122dd514f4ded7
Author: Sami Kyostila <skyostil@chromium.org>
Date: Fri Jan 11 12:36:13 2019

Remove trace event category warming

Since all categories are now statically defined, there's no need to warm
any of them up explicitly in order for the categories to show up in the
tracing UI.

Depends on https://chromium-review.googlesource.com/c/chromium/src/+/1401273

TBR=sandersd@chromium.org

Bug: 914579
Change-Id: Idbabec3fa9e3f35716663d08260691d7bf4ecfa7
Reviewed-on: https://chromium-review.googlesource.com/c/1402760
Reviewed-by: Sami Kyöstilä <skyostil@chromium.org>
Reviewed-by: Eric Seckler <eseckler@chromium.org>
Reviewed-by: oysteine <oysteine@chromium.org>
Commit-Queue: Sami Kyöstilä <skyostil@chromium.org>
Cr-Commit-Position: refs/heads/master@{#621979}
[modify] https://crrev.com/eb0e029a36e458f83480607531122dd514f4ded7/base/task/sequence_manager/sequence_manager_impl.cc
[modify] https://crrev.com/eb0e029a36e458f83480607531122dd514f4ded7/base/trace_event/common/trace_event_common.h
[modify] https://crrev.com/eb0e029a36e458f83480607531122dd514f4ded7/base/trace_event/cpufreq_monitor_android.cc
[modify] https://crrev.com/eb0e029a36e458f83480607531122dd514f4ded7/base/trace_event/memory_dump_manager.cc
[modify] https://crrev.com/eb0e029a36e458f83480607531122dd514f4ded7/components/tracing/common/tracing_sampler_profiler.cc
[modify] https://crrev.com/eb0e029a36e458f83480607531122dd514f4ded7/media/base/media.cc
[modify] https://crrev.com/eb0e029a36e458f83480607531122dd514f4ded7/third_party/blink/renderer/platform/scheduler/common/tracing_helper.cc
[modify] https://crrev.com/eb0e029a36e458f83480607531122dd514f4ded7/third_party/blink/renderer/platform/scheduler/common/tracing_helper.h
[modify] https://crrev.com/eb0e029a36e458f83480607531122dd514f4ded7/third_party/blink/renderer/platform/scheduler/common/web_thread_scheduler.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Jan 16

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/368ff90edd63322c7d67a04fbffe23d136c07956

commit 368ff90edd63322c7d67a04fbffe23d136c07956
Author: Oystein Eftevaag <oysteine@chromium.org>
Date: Wed Jan 16 02:29:00 2019

Service Manager: Allow a service manifest to specify interfaces it should
always be allowed to bind.

This is needed for situations where the ServiceBinding client lib
itself handles a binding for an interface, to avoid adding
the capability to every single manifest file (Specifically, for
the tracing service).

See https://chromium-review.googlesource.com/c/chromium/src/+/1377593)

R=rockot@google.com

Bug: 914579
Change-Id: If9a82b1e2b2b292272b85812ff146a3eb2a726cf
Reviewed-on: https://chromium-review.googlesource.com/c/1410384
Commit-Queue: oysteine <oysteine@chromium.org>
Reviewed-by: Ken Rockot <rockot@google.com>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#623052}
[modify] https://crrev.com/368ff90edd63322c7d67a04fbffe23d136c07956/services/catalog/BUILD.gn
[modify] https://crrev.com/368ff90edd63322c7d67a04fbffe23d136c07956/services/catalog/entry_cache.cc
[add] https://crrev.com/368ff90edd63322c7d67a04fbffe23d136c07956/services/catalog/service_options.cc
[modify] https://crrev.com/368ff90edd63322c7d67a04fbffe23d136c07956/services/catalog/service_options.h
[modify] https://crrev.com/368ff90edd63322c7d67a04fbffe23d136c07956/services/service_manager/public/cpp/manifest.h
[modify] https://crrev.com/368ff90edd63322c7d67a04fbffe23d136c07956/services/service_manager/public/cpp/manifest_builder.h
[modify] https://crrev.com/368ff90edd63322c7d67a04fbffe23d136c07956/services/service_manager/service_manager.cc
[modify] https://crrev.com/368ff90edd63322c7d67a04fbffe23d136c07956/services/service_manager/tests/connect/connect.test-mojom
[modify] https://crrev.com/368ff90edd63322c7d67a04fbffe23d136c07956/services/service_manager/tests/connect/connect_test_package.cc
[modify] https://crrev.com/368ff90edd63322c7d67a04fbffe23d136c07956/services/service_manager/tests/connect/connect_unittest.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Jan 17 (5 days ago)

Sign in to add a comment