cc: Adding creation location to debug BeginFrameArgs objects.

cc/output/begin_frame_args.cc

Index: cc/output/begin_frame_args.cc
diff --git a/cc/output/begin_frame_args.cc b/cc/output/begin_frame_args.cc
index 0be93180f8783aa699fa618e19a2ddc1374a7c47..b7b21f73167f90173a0323689ea5f3fab95be1fe 100644
--- a/cc/output/begin_frame_args.cc
+++ b/cc/output/begin_frame_args.cc
@@ -41,12 +41,20 @@ BeginFrameArgs::BeginFrameArgs(base::TimeTicks frame_time,
       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()
@@ -63,6 +71,11 @@ void BeginFrameArgs::AsValueInto(base::debug::TracedValue* state) const {
   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