New issue
Advanced search Search tips

Issue 808839 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 797809



Sign in to add a comment

Clean up tracing agents

Project Member Reported by chiniforooshan@chromium.org, Feb 4 2018

Issue description

1- There is a considerable boilerplate code that can be factored out to a common base class.

2- The ARC tracing agent is in //content which is a layering violation.

3- The ARC tracing agent depends on //cc, which looks weird. The reason is that it uses cc::RingBuffer. So, cc:RingBuffer should be moved to //base.
 
Blocking: 797809
Project Member

Comment 2 by bugdroid1@chromium.org, Feb 6 2018

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

commit 65c25b89d9077b9976998da7e9ff3b8d40bcf99d
Author: Ehsan Chiniforooshan <chiniforooshan@chromium.org>
Date: Tue Feb 06 21:26:28 2018

Move cc::RingBuffer to //base

It is used by content/browser/tracing/arc_tracing_agent_impl.{cc,h}, which,
logically, should not depend on chrome compositor.

Also, we want to move arc_tracing_agent_impl to //chrome/browser
(crrev/c/868575) but we do not want to add a dependency from //chrome/browser to
//cc/base ( https://cs.chromium.org/chromium/src/chrome/browser/DEPS?l=89).

TBR=oysteine@chromium.org

Bug:  808839 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
Change-Id: I30adee8ee4f3c7e8c4f478f15e6841937b4be96c
Reviewed-on: https://chromium-review.googlesource.com/899858
Reviewed-by: danakj <danakj@chromium.org>
Reviewed-by: Ehsan Chiniforooshan <chiniforooshan@chromium.org>
Commit-Queue: Ehsan Chiniforooshan <chiniforooshan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534796}
[rename] https://crrev.com/65c25b89d9077b9976998da7e9ff3b8d40bcf99d/base/containers/ring_buffer.h
[modify] https://crrev.com/65c25b89d9077b9976998da7e9ff3b8d40bcf99d/cc/resources/memory_history.h
[modify] https://crrev.com/65c25b89d9077b9976998da7e9ff3b8d40bcf99d/cc/trees/frame_rate_counter.h
[modify] https://crrev.com/65c25b89d9077b9976998da7e9ff3b8d40bcf99d/content/browser/tracing/arc_tracing_agent_impl.h

Project Member

Comment 3 by bugdroid1@chromium.org, Feb 6 2018

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

commit 5b1c577174868e1e0a13f23b8698fa6a206f7df4
Author: Ehsan Chiniforooshan <chiniforooshan@chromium.org>
Date: Tue Feb 06 23:10:16 2018

Tracing: Reduce boilerplate code in tracing agents

Since most tracing agents only care about receiving start/stop
tracing signals, a default implementation is provided for other
methods to reduce boilerplate code.

Bug:  808839 
Change-Id: I4645add6e7e81a25d0e923c8afcdf8f5dc3a7a13
Reviewed-on: https://chromium-review.googlesource.com/868575
Commit-Queue: Ehsan Chiniforooshan <chiniforooshan@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Luis Hector Chavez <lhchavez@chromium.org>
Reviewed-by: oysteine <oysteine@chromium.org>
Reviewed-by: Charlie Andrews <charliea@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534825}
[modify] https://crrev.com/5b1c577174868e1e0a13f23b8698fa6a206f7df4/chrome/browser/chromeos/BUILD.gn
[modify] https://crrev.com/5b1c577174868e1e0a13f23b8698fa6a206f7df4/chrome/browser/chromeos/DEPS
[modify] https://crrev.com/5b1c577174868e1e0a13f23b8698fa6a206f7df4/chrome/browser/chromeos/arc/tracing/arc_tracing_bridge.cc
[modify] https://crrev.com/5b1c577174868e1e0a13f23b8698fa6a206f7df4/chrome/browser/chromeos/arc/tracing/arc_tracing_bridge.h
[modify] https://crrev.com/5b1c577174868e1e0a13f23b8698fa6a206f7df4/content/browser/BUILD.gn
[delete] https://crrev.com/4c69778aabd2cdc75008d2dcefa5e35b8ba7783e/content/browser/tracing/arc_tracing_agent_impl.cc
[delete] https://crrev.com/4c69778aabd2cdc75008d2dcefa5e35b8ba7783e/content/browser/tracing/arc_tracing_agent_impl.h
[modify] https://crrev.com/5b1c577174868e1e0a13f23b8698fa6a206f7df4/content/browser/tracing/cast_tracing_agent.cc
[modify] https://crrev.com/5b1c577174868e1e0a13f23b8698fa6a206f7df4/content/browser/tracing/cast_tracing_agent.h
[modify] https://crrev.com/5b1c577174868e1e0a13f23b8698fa6a206f7df4/content/browser/tracing/cros_tracing_agent.cc
[modify] https://crrev.com/5b1c577174868e1e0a13f23b8698fa6a206f7df4/content/browser/tracing/cros_tracing_agent.h
[modify] https://crrev.com/5b1c577174868e1e0a13f23b8698fa6a206f7df4/content/browser/tracing/etw_tracing_agent_win.cc
[modify] https://crrev.com/5b1c577174868e1e0a13f23b8698fa6a206f7df4/content/browser/tracing/etw_tracing_agent_win.h
[modify] https://crrev.com/5b1c577174868e1e0a13f23b8698fa6a206f7df4/content/browser/tracing/tracing_controller_impl.cc
[modify] https://crrev.com/5b1c577174868e1e0a13f23b8698fa6a206f7df4/content/public/browser/BUILD.gn
[delete] https://crrev.com/4c69778aabd2cdc75008d2dcefa5e35b8ba7783e/content/public/browser/arc_tracing_agent.h
[modify] https://crrev.com/5b1c577174868e1e0a13f23b8698fa6a206f7df4/services/resource_coordinator/public/cpp/BUILD.gn
[add] https://crrev.com/5b1c577174868e1e0a13f23b8698fa6a206f7df4/services/resource_coordinator/public/cpp/tracing/base_agent.cc
[add] https://crrev.com/5b1c577174868e1e0a13f23b8698fa6a206f7df4/services/resource_coordinator/public/cpp/tracing/base_agent.h
[modify] https://crrev.com/5b1c577174868e1e0a13f23b8698fa6a206f7df4/services/resource_coordinator/public/cpp/tracing/chrome_trace_event_agent.cc
[modify] https://crrev.com/5b1c577174868e1e0a13f23b8698fa6a206f7df4/services/resource_coordinator/public/cpp/tracing/chrome_trace_event_agent.h

Status: Fixed (was: Started)

Sign in to add a comment