cc: Adding creation location to debug BeginFrameArgs objects.

cc/output/begin_frame_args.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "cc/output/begin_frame_args.h"
 
 #include "base/debug/trace_event_argument.h"
 #include "ui/gfx/frame_time.h"
 
 namespace cc {
@@ skipping 23 matching lines @@
 BeginFrameArgs::BeginFrameArgs(base::TimeTicks frame_time,
                                base::TimeTicks deadline,
                                base::TimeDelta interval,
                                BeginFrameArgs::BeginFrameArgsType type)
     : frame_time(frame_time),
       deadline(deadline),
       interval(interval),
       type(type) {
 }
 
-BeginFrameArgs BeginFrameArgs::Create(base::TimeTicks frame_time,
+BeginFrameArgs BeginFrameArgs::Create(const tracked_objects::Location* location,
+                                      base::TimeTicks frame_time,
                                       base::TimeTicks deadline,
                                       base::TimeDelta interval,
                                       BeginFrameArgs::BeginFrameArgsType type) {
   DCHECK_NE(type, BeginFrameArgs::INVALID);
+#ifdef NDEBUG
   return BeginFrameArgs(frame_time, deadline, interval, type);
+#else
+  BeginFrameArgs args = BeginFrameArgs(frame_time, deadline, interval, type);
+  args.created_by = *location;
+  delete location;

[brianderson, 2014/11/19 20:43:29]

Would it be better to use a scoped_ptr instead of manually deleting?

[mithro-old, 2014/11/20 04:30:13]

I tried to use a scope pointer but C++ is not smart enough to understand that
the following should work;
 void A(scoped_ptr<blah> a) {
   B(a);
 }

Instead you have to do;
 void A(scoped_ptr<blah> a) {
   B(a.Pass());
 }

Which means that things need to understand that the first argument is a scoped
pointer or not.

[danakj, 2014/11/20 16:02:19]

It's not a matter of smarts. scoped_ptr is move-only.
Why is that a problem here?

#define BEGINFRAME_FROM_HERE make_scoped_ptr(new FROM_HERE)

BeginFrameArgs::Create(scoped_ptr<t_o::Location> location, ..._) {
  ...
  args.created_by = *location;
}

[mithro-old, 2014/11/21 05:57:46]

Yes, scoped_ptr is move-only. The compiler should be able to see that in the
above case B(a) can be transformed into a move operation.

As the B() call is the last thing inside A() the argument "a" will be destroyed
as soon as B() returns. Hence it is perfectly safe to move argument "a" into
B().
As scoped_ptr is move-only, using it means the first argument is no longer an
opaque type that callers don't need to understand.

This means that we can't change the type behind the callers back, IE in release
builds we can't use a void* versus an actual object in Debug builds.

[danakj, 2014/11/21 14:58:07]

Why do you need to change the type to a void*? It can just be an empty
scoped_ptr that you don't deref?

[danakj, 2014/11/21 15:03:43]

Ah, I see what you've done. I don't really see the value of typedeffing it as a
void* in release builds, it's the only place that the typedef is used, and the
code has to know to not deref it anyway. What if you did:

#if debug
#define BEGINFRAME_FROM_HERE nullptr
#else
#define BEGINFRAME_FROM_HERE (&FROM_HERE)
#endif

BeginFrameArgs BeginFrameArgs::Create(const tracked_objects::Location* location,
...) {
  ...
#if debug
  created_from = *location;
#endif
}

?

+  return args;
+#endif
 }
 
 scoped_refptr<base::debug::ConvertableToTraceFormat> BeginFrameArgs::AsValue()
     const {
   scoped_refptr<base::debug::TracedValue> state =
       new base::debug::TracedValue();
   AsValueInto(state.get());
   return state;
 }
 
 void BeginFrameArgs::AsValueInto(base::debug::TracedValue* state) const {
   state->SetString("type", "BeginFrameArgs");
   state->SetString("subtype", TypeToString(type));
   state->SetDouble("frame_time_us", frame_time.ToInternalValue());
   state->SetDouble("deadline_us", deadline.ToInternalValue());
   state->SetDouble("interval_us", interval.InMicroseconds());
+#ifndef NDEBUG
+  std::string created_by_str("");
+  created_by.Write(true, true, &created_by_str);
+  state->SetString("created_by", created_by_str);
+#endif
 }
 
 // This is a hard-coded deadline adjustment that assumes 60Hz, to be used in
 // cases where a good estimated draw time is not known. Using 1/3 of the vsync
 // as the default adjustment gives the Browser the last 1/3 of a frame to
 // produce output, the Renderer Impl thread the middle 1/3 of a frame to produce
 // ouput, and the Renderer Main thread the first 1/3 of a frame to produce
 // output.
 base::TimeDelta BeginFrameArgs::DefaultEstimatedParentDrawTime() {
   return base::TimeDelta::FromMicroseconds(16666 / 3);
 }
 
 base::TimeDelta BeginFrameArgs::DefaultInterval() {
   return base::TimeDelta::FromMicroseconds(16666);
 }
 
 base::TimeDelta BeginFrameArgs::DefaultRetroactiveBeginFramePeriod() {
   return base::TimeDelta::FromMicroseconds(4444);
 }
 
 }  // namespace cc