cc: Adding creation location to debug BeginFrameArgs objects.

cc/output/begin_frame_args.h

 // 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.
 
 #ifndef CC_OUTPUT_BEGIN_FRAME_ARGS_H_
 #define CC_OUTPUT_BEGIN_FRAME_ARGS_H_
 
+#include "base/location.h"
 #include "base/memory/ref_counted.h"
 #include "base/time/time.h"
 #include "base/values.h"
 #include "cc/base/cc_export.h"
 
 namespace base {
 namespace debug {
 class ConvertableToTraceFormat;
 class TracedValue;
 }
 }
 
+/**

[danakj, 2014/11/19 15:46:00]

Use c++ style comments

+ * In debug builds we trace the creation origin of BeginFrameArgs objects. We
+ * reuse the tracked_objects::Location system to do that.
+ *
+ * However, in release builds we don't want this as it doubles the size of the
+ * BeginFrameArgs object. As well it adds a number of largish strings to the
+ * binary. Despite the argument being unused, most compilers are unable to
+ * optimise it away even when unused. Instead we use the BEGINFRAME_FROM_HERE
+ * macro to prevent the data even getting referenced.
+ */
+#ifdef NDEBUG
+#define BEGINFRAME_FROM_HERE nullptr
+#else
+#define BEGINFRAME_FROM_HERE (new FROM_HERE)
+#endif
+
 namespace cc {
 
 struct CC_EXPORT BeginFrameArgs {
   enum BeginFrameArgsType {
     INVALID,
     NORMAL,
     SYNCHRONOUS,
     MISSED,
   };
   static const char* TypeToString(BeginFrameArgsType type);
 
   // Creates an invalid set of values.
   BeginFrameArgs();
 
   // You should be able to find all instances where a BeginFrame has been
   // created by searching for "BeginFrameArgs::Create".
-  static BeginFrameArgs Create(base::TimeTicks frame_time,
+  // The location argument should **always** be BEGINFRAME_FROM_HERE macro.

[danakj, 2014/11/19 15:46:00]

The **'s seem excessive?

+  static BeginFrameArgs Create(const tracked_objects::Location* location,
+                               base::TimeTicks frame_time,
                                base::TimeTicks deadline,
                                base::TimeDelta interval,
                                BeginFrameArgsType type);
 
   // This is the default delta that will be used to adjust the deadline when
   // proper draw-time estimations are not yet available.
   static base::TimeDelta DefaultEstimatedParentDrawTime();
 
   // This is the default interval to use to avoid sprinkling the code with
   // magic numbers.
   static base::TimeDelta DefaultInterval();
 
   // This is the default amount of time after the frame_time to retroactively
   // send a BeginFrame that had been skipped. This only has an effect if the
   // deadline has passed, since the deadline is also used to trigger BeginFrame
   // retroactively.
   static base::TimeDelta DefaultRetroactiveBeginFramePeriod();
 
   bool IsValid() const { return interval >= base::TimeDelta(); }
 
   scoped_refptr<base::debug::ConvertableToTraceFormat> AsValue() const;
   void AsValueInto(base::debug::TracedValue* dict) const;
 
   base::TimeTicks frame_time;
   base::TimeTicks deadline;
   base::TimeDelta interval;
   BeginFrameArgsType type;
+#ifndef NDEBUG

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

Do we have any kind of policy against differing struct layouts between Release
and Debug builds? I tried to find it in the style guides, but I couldn't find
anything about the topic.

I know some projects have that policy to avoid hard to debug errors in case devs
mix and match Release/Debug libraries. I don't know of any chrome devs that do
that though.

[danakj, 2014/11/19 20:45:45]

We've done this before, though we tend to do it by DCHECK_IS_ON now. This is
fine to do.

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

I would normally 100% agree that it is a bad idea to have these types be
different in Debug/Release builds. However it does *double* the space used by a
BeginFrameArgs object and that would means the object is starting to get to the
size were pass-by-value isn't the best idea anymore .

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

What is the difference between DCHECK_IS_ON and NDEBUG?

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

You can build a release build with dchecks on via a gyp define. Our bots do
this.

+  tracked_objects::Location created_by;
+#endif
 
  private:
   BeginFrameArgs(base::TimeTicks frame_time,
                  base::TimeTicks deadline,
                  base::TimeDelta interval,
                  BeginFrameArgsType type);
 };
 
 }  // namespace cc
 
 #endif  // CC_OUTPUT_BEGIN_FRAME_ARGS_H_