Add tracing AutoOpenCloseEvent.

media/blink/video_frame_compositor.cc

Index: media/blink/video_frame_compositor.cc
diff --git a/media/blink/video_frame_compositor.cc b/media/blink/video_frame_compositor.cc
index 544f7098701d481c2358c2493d2f59aae52d5256..b8d055ef227243cc4ea1f13ad7bcc9cb2afc62f9 100644
--- a/media/blink/video_frame_compositor.cc
+++ b/media/blink/video_frame_compositor.cc
@@ -8,6 +8,7 @@
 #include "base/message_loop/message_loop.h"
 #include "base/time/default_tick_clock.h"
 #include "base/trace_event/trace_event.h"
+#include "base/trace_event/video_playback_pae.h"
 #include "media/base/video_frame.h"
 
 namespace media {
@@ -35,7 +36,9 @@ VideoFrameCompositor::VideoFrameCompositor(
       new_background_frame_(false),
       // Assume 60Hz before the first UpdateCurrentFrame() call.
       last_interval_(base::TimeDelta::FromSecondsD(1.0 / 60)),
-      callback_(nullptr) {
+      callback_(nullptr),
+      persistent_async_(
+          new base::trace_event::VideoPlaybackPersistentAsyncEvent()) {
   background_rendering_timer_.SetTaskRunner(compositor_task_runner_);
 }
 
@@ -105,8 +108,7 @@ bool VideoFrameCompositor::HasCurrentFrame() {
 }
 
 void VideoFrameCompositor::Start(RenderCallback* callback) {
-  TRACE_EVENT_ASYNC_BEGIN0("media,rail", "VideoPlayback",

[DaleCurtis, 2016/08/30 19:09:49]

Do we really need a Start()/Stop() API or should this just construct an ASYNC
event during VFC::Start() and destruct it during VFC::Stop()? The recording can
be handled by the constructor and destructor respectively.

[alexandermont, 2016/08/30 19:34:36]

I don't really see an advantage of doing it the way you're suggesting. It's
possible to start and stop an event multiple times - and sometimes this will
happen, for instance if you're starting and stopping the same video multiple
times. Also, destructors can get called when you didn't do anything that makes
it obvious that the destructor was called (like calling delete) - if an object
goes out of scope that could trigger a destructor call. (Similar things can
happen with constructors as well - e.g. if the PersistentAsyncEvent is a member
of the class, then the constructor will be called when the class constructor is
called.) Having the recording happening in the constructor and destructor seems
like it might make it less clear to the reader of the code exactly when
recording starts and stops.

[DaleCurtis, 2016/08/30 19:42:31]

Generally I find it preferable to avoid unnecessary APIs for simplicity. We're
all well versed with smart pointers, and even have things like ElapsedTimer,
ScopedHistogramTimer, etc which perform in a similar vein. It's not media/ code
so I don't feel strongly, but I encourage you to avoid unnecessary function
calls for long term cleanliness. See a recent example of a similar cleanup I
did: https://codereview.chromium.org/2076793002/

[alexandermont, 2016/08/31 18:31:59]

See discussion below in comments. Given that future subclasses may add arguments
to the Start() and Stop() calls, we think it's best to leave them as specific
calls.

-                           static_cast<const void*>(this));
+  persistent_async_->Start();
 
   // Called from the media thread, so acquire the callback under lock before
   // returning in case a Stop() call comes in before the PostTask is processed.
@@ -119,8 +121,7 @@ void VideoFrameCompositor::Start(RenderCallback* callback) {
 }
 
 void VideoFrameCompositor::Stop() {
-  TRACE_EVENT_ASYNC_END0("media,rail", "VideoPlayback",
-                         static_cast<const void*>(this));
+  persistent_async_->Stop();
 
   // Called from the media thread, so release the callback under lock before
   // returning to avoid a pending UpdateCurrentFrame() call occurring before