New Sequence/Thread checking API.

base/sequence_checker.h

Index: base/sequence_checker.h
diff --git a/base/sequence_checker.h b/base/sequence_checker.h
index 471631844ba7da9a8ceb8a908767b1795eed0721..43e3e2d6300448ac50d5e7b203d906127138be6c 100644
--- a/base/sequence_checker.h
+++ b/base/sequence_checker.h
@@ -5,14 +5,55 @@
 #ifndef BASE_SEQUENCE_CHECKER_H_
 #define BASE_SEQUENCE_CHECKER_H_
 
+#include "base/logging.h"
 #include "base/sequence_checker_impl.h"
 
+// SequenceChecker is a helper class used to help verify that some methods of a
+// class are called sequentially (for thread-safety).
+//
+// Use the macros below instead of the SequenceChecker directly so that the
+// unused member doesn't result in an extra byte (four when padded) per
+// instance in production.
+//
+// This class is much prefered to ThreadChecker for thread-safety checks.
+// ThreadChecker should only be used for classes that are truly thread-affine
+// (use thread-local-storage or a third-party API that does.)

[fdoray, 2017/05/10 13:14:05]

s/.)/)./ ?

[gab, 2017/05/10 15:40:08]

Done.

+//
+// Usage:
+//   class MyClass {
+//    public:
+//     MyClass() {
+//       // It's sometimes useful to detach on construction for objects that are
+//       // constructed in one place and forever after used from another
+//       // sequence.
+//       DETACH_FROM_SEQUENCE(my_sequence_checker_);
+//     }
+//
+//     void MyMethod() {
+//       DCHECK_CALLED_ON_VALID_SEQUENCE(my_sequence_checker_);
+//       ... (do stuff) ...
+//     }

[danakj, 2017/05/09 16:04:28]

I suggest adding the destructor checking to the example because there's been
past confusion that the checker would implicitly check destructor but it does
not.

[gab, 2017/05/10 15:40:08]

Done.

+//
+//    private:
+//     SEQUENCE_CHECKER(my_sequence_checker_);
+//   }
+
+#if DCHECK_IS_ON()
+#define SEQUENCE_CHECKER(name) base::SequenceChecker name
+#define DCHECK_CALLED_ON_VALID_SEQUENCE(name) DCHECK((name).CalledOnValidSequence())
+#define DETACH_FROM_SEQUENCE(name) (name).DetachFromSequence()
+#else  // DCHECK_IS_ON()
+#define SEQUENCE_CHECKER(name)
+#define DCHECK_CALLED_ON_VALID_SEQUENCE(name)
+#define DETACH_FROM_SEQUENCE(name)
+#endif  // DCHECK_IS_ON()
+
 namespace base {
 
 // Do nothing implementation, for use in release mode.
 //
-// Note: You should almost always use the SequenceChecker class to get
-// the right version for your build configuration.
+// Note: You should almost always use the SequenceChecker class (through the
+// above macros) to get the right version for your build configuration.
 class SequenceCheckerDoNothing {
  public:
   bool CalledOnValidSequence() const { return true; }
@@ -20,25 +61,6 @@ class SequenceCheckerDoNothing {
   void DetachFromSequence() {}
 };
 
-// SequenceChecker is a helper class to verify that calls to some methods of a
-// class are sequenced. Calls are sequenced when they are issued:
-// - From tasks posted to SequencedTaskRunners or SingleThreadTaskRunners bound
-//   to the same sequence, or,
-// - From a single thread outside of any task.
-//
-// Example:
-// class MyClass {
-//  public:
-//   void Foo() {
-//     DCHECK(sequence_checker_.CalledOnValidSequence());
-//     ... (do stuff) ...
-//   }
-//
-//  private:
-//   SequenceChecker sequence_checker_;
-// }
-//
-// In Release mode, CalledOnValidSequence() will always return true.
 #if DCHECK_IS_ON()
 class SequenceChecker : public SequenceCheckerImpl {

[fdoray, 2017/05/10 13:14:05]

Suggestion: Add a presubmit warning (or error?) to prevent usage of
SequenceChecker directly.
Alternative suggestion: Put in base::internal:: namespace once all callers have
been migrated to macros.

[fdoray, 2017/05/10 13:14:56]

Oups... I just saw that you added the PRESUBMIT check. The alternative
suggestion remains valid.

[gab, 2017/05/10 15:40:08]

What I'll do is remove ThreadChecker and ThreadCheckerDoNothing, but I'll leave
ThreadCheckerImpl as it's sometimes useful when you want to use it for logic
outside of DCHECKs. I won't put it in base::internal:: as that would indicate
you should *never* use it which is not the intent.

 };