[Chromecast] Type-safe static sequences.

With this, you can make the compiler ensure, statically, that
the methods of your class always run on the same sequence.
This is vastly safer than runtime checks like SequenceChecker
since it will fail to compile if you violate the requirement.

(It wouldn't be C++ if there weren't hacky ways around this, but
any attempt to trick the compiler into allowing a sequence
violation should look pretty obvious to reviewers.)

To use this, declare a struct that inherits from StaticSequence.
This struct will automatically declare a Key class that can only
be constructed inside the StaticSequence's PostTask function. To
force users to go through that StaticSequence's PostTask() to
call your method, simply declare a reference to the Key object
as the last parameter to the method.

This also includes a wrapper template called Sequenced, similar
to base::SequenceBound, but with the TaskRunner known at compile
time rather than runtime. This can be used to add an extra level
of safety over runtime checks like
DCHECK_CALLED_ON_VALID_SEQUENCE or
TaskRunner::RunsTasksInCurrentThread. This wrapper ensures the
wrapped object is destroyed on the correct sequence, essentially
turning any sequence-affine object to a thread-safe object.

This will be made even more useful once base::PostTask() returns
a base::Promise, as it will work with methods that return values
as well as void methods.

Bug: None
Test: cast_base_unittests
Change-Id: Ic408e343bc084c19c4c6f9e983b343b98502daf2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1863549Reviewed-by: Kenneth MacKay <kmackay@chromium.org>
Reviewed-by: Yuchen Liu <yucliu@chromium.org>
Commit-Queue: Simeon Anfinrud <sanfin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#711007}

chromecast/base/BUILD.gn

@@ -185,6 +185,7 @@ test("cast_base_unittests") {
     ":thread_health_checker",
     "//base/test:run_all_unittests",
     "//base/test:test_support",
+    "//chromecast/base/static_sequence:tests",
     "//testing/gmock",
     "//testing/gtest",
   ]

chromecast/base/static_sequence/BUILD.gn

+# Copyright 2019 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.
+
+import("//build/nocompile.gni")
+import("//chromecast/chromecast.gni")
+
+cast_source_set("static_sequence") {
+  sources = [
+    "static_sequence.cc",
+    "static_sequence.h",
+  ]
+  deps = [
+    "//base",
+  ]
+}
+
+cast_source_set("tests") {
+  testonly = true
+  sources = [
+    "static_sequence_unittest.cc",
+  ]
+
+  deps = [
+    ":static_sequence",
+    "//base/test:test_support",
+    "//testing/gtest",
+  ]
+
+  if (enable_nocompile_tests) {
+    deps += [ ":nocompile_tests" ]
+  }
+}
+
+if (enable_nocompile_tests) {
+  nocompile_test("nocompile_tests") {
+    sources = [
+      "static_sequence_unittest.nc",
+    ]
+    deps = [
+      ":static_sequence",
+      "//base/test:run_all_unittests",
+      "//testing/gtest",
+    ]
+  }
+}

chromecast/base/static_sequence/OWNERS

+sanfin@chromium.org
+thoren@chromium.org

chromecast/base/static_sequence/static_sequence.cc

+// Copyright 2019 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 "chromecast/base/static_sequence/static_sequence.h"
+
+namespace util {
+namespace internal {
+
+StaticTaskRunnerHolder::StaticTaskRunnerHolder(base::TaskTraits traits)
+    : traits_(traits), initialized_(false) {}
+
+StaticTaskRunnerHolder::~StaticTaskRunnerHolder() = default;
+
+void StaticTaskRunnerHolder::WillDestroyCurrentMessageLoop() {
+  initialized_ = false;
+  task_runner_ = nullptr;
+}
+
+const scoped_refptr<base::SequencedTaskRunner>& StaticTaskRunnerHolder::Get() {
+  if (!initialized_) {
+    task_runner_ = base::CreateSequencedTaskRunner(traits_);
+    base::MessageLoopCurrent::Get().AddDestructionObserver(this);
+    initialized_ = true;
+  }
+  return task_runner_;
+}
+
+}  // namespace internal
+}  // namespace util

chromecast/base/static_sequence/static_sequence_unittest.cc

+// Copyright 2019 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 "chromecast/base/static_sequence/static_sequence.h"
+
+#include "base/test/task_environment.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace util {
+namespace {
+
+struct TestSequence : StaticSequence<TestSequence> {};
+
+struct CustomTraitsProvider {
+  static constexpr base::TaskTraits GetTraits() {
+    return {base::ThreadPool(), base::TaskPriority::LOWEST,
+            base::ThreadPolicy::PREFER_BACKGROUND, base::MayBlock()};
+  }
+};
+
+struct TestSequenceWithCustomTraits
+    : StaticSequence<TestSequenceWithCustomTraits, CustomTraitsProvider> {};
+
+void DoSomething(bool* activated) {
+  *activated = true;
+}
+
+void DoSomethingWithRequiredSequence(bool* activated, TestSequence::Key&) {
+  *activated = true;
+}
+
+class TestObject {
+ public:
+  void DoSomething(bool* activated) { *activated = true; }
+  void DoSomethingWithRequiredSequence(bool* activated, TestSequence::Key&) {
+    *activated = true;
+  }
+};
+
+class ParameterizedObject {
+ public:
+  explicit ParameterizedObject(int increment_by)
+      : increment_by_(increment_by) {}
+
+  void Increment(int* out, TestSequence::Key&) { *out += increment_by_; }
+
+ private:
+  int increment_by_;
+};
+
+class HasSideEffectsInConstructor {
+ public:
+  HasSideEffectsInConstructor(int x, int y, int* r) { *r = x + y; }
+};
+
+class HasSideEffectsInDestructor {
+ public:
+  HasSideEffectsInDestructor(int x, int y, int* r) : r_(r), sum_(x + y) {}
+  ~HasSideEffectsInDestructor() { *r_ = sum_; }
+
+ private:
+  int* r_;
+  int sum_;
+};
+
+}  // namespace
+
+TEST(StaticSequenceTest, StaticProperties) {
+  static_assert(!std::is_copy_constructible<TestSequence::Key>::value,
+                "Keys must not be copyable.");
+  static_assert(!std::is_move_constructible<TestSequence::Key>::value,
+                "Keys must not be movable.");
+}
+
+TEST(StaticSequenceTest, InvokeUnprotectedCallback) {
+  base::test::TaskEnvironment env;
+  bool activated = false;
+  TestSequence::PostTask(base::BindOnce(&DoSomething, &activated));
+  EXPECT_FALSE(activated);
+  env.RunUntilIdle();
+  EXPECT_TRUE(activated);
+}
+
+TEST(StaticSequenceTest, InvokeProtectedCallback) {
+  base::test::TaskEnvironment env;
+  bool activated = false;
+  TestSequence::PostTask(
+      base::BindOnce(&DoSomethingWithRequiredSequence, &activated));
+  EXPECT_FALSE(activated);
+  env.RunUntilIdle();
+  EXPECT_TRUE(activated);
+}
+
+TEST(StaticSequenceTest, InvokeObjectUnprotectedMethod) {
+  base::test::TaskEnvironment env;
+  bool activated = false;
+  TestObject obj;
+  TestSequence::PostTask(base::BindOnce(&TestObject::DoSomething,
+                                        base::Unretained(&obj), &activated));
+  EXPECT_FALSE(activated);
+  env.RunUntilIdle();
+  EXPECT_TRUE(activated);
+}
+
+TEST(StaticSequenceTest, InvokeSequencedObjectUnprotectedMethod) {
+  base::test::TaskEnvironment env;
+  bool activated = false;
+  Sequenced<TestObject, TestSequence> obj;
+  obj.Post(FROM_HERE, &TestObject::DoSomething, &activated);
+  EXPECT_FALSE(activated);
+  env.RunUntilIdle();
+  EXPECT_TRUE(activated);
+}
+
+TEST(StaticSequenceTest, InvokeSequencedObjectProtectedMethod) {
+  base::test::TaskEnvironment env;
+  bool activated = false;
+  Sequenced<TestObject, TestSequence> obj;
+  obj.Post(FROM_HERE, &TestObject::DoSomethingWithRequiredSequence, &activated);
+  EXPECT_FALSE(activated);
+  env.RunUntilIdle();
+  EXPECT_TRUE(activated);
+}
+
+TEST(StaticSequenceTest, SequencedConstructorIncludesArguments) {
+  base::test::TaskEnvironment env;
+  int r = 0;
+  Sequenced<ParameterizedObject, TestSequence> obj(2);
+  obj.Post(FROM_HERE, &ParameterizedObject::Increment, &r);
+  EXPECT_EQ(r, 0);
+  env.RunUntilIdle();
+  EXPECT_EQ(r, 2);
+}
+
+TEST(StaticSequenceTest, UseCustomTraits) {
+  base::test::TaskEnvironment env;
+  bool r = false;
+  Sequenced<TestObject, TestSequenceWithCustomTraits> obj;
+  obj.Post(FROM_HERE, &TestObject::DoSomething, &r);
+  EXPECT_FALSE(r);
+  env.RunUntilIdle();
+  EXPECT_TRUE(r);
+}
+
+TEST(StaticSequenceTest, ConstructsOnSequence) {
+  base::test::TaskEnvironment env;
+  int r = 0;
+  // The constructor for HasSideEffectsInConstructor will set |r| to the sum of
+  // the first two arguments, but should only run on the sequence.
+  Sequenced<HasSideEffectsInConstructor, TestSequence> obj(1, 2, &r);
+  EXPECT_EQ(r, 0);
+  env.RunUntilIdle();
+  EXPECT_EQ(r, 3);
+}
+
+TEST(StaticSequenceTest, DestructOnSequence) {
+  base::test::TaskEnvironment env;
+  int r = 0;
+  {
+    // The destructor for HasSideEffectsInDestructor will set |r| to the sum of
+    // the first two constructor arguments, but should only run on the sequence.
+    Sequenced<HasSideEffectsInDestructor, TestSequence> obj(2, 3, &r);
+    env.RunUntilIdle();
+    EXPECT_EQ(r, 0);
+  }
+  EXPECT_EQ(r, 0);
+  env.RunUntilIdle();
+  EXPECT_EQ(r, 5);
+}
+
+TEST(StaticSequenceTest, PostUnprotectedMemberFunction) {
+  base::test::TaskEnvironment env;
+  TestObject x;
+  bool r = false;
+  TestSequence::Post(FROM_HERE, &TestObject::DoSomething, base::Unretained(&x),
+                     &r);
+  EXPECT_FALSE(r);
+  env.RunUntilIdle();
+  EXPECT_TRUE(r);
+}
+
+TEST(StaticSequenceTest, PostProtectedMemberFunction) {
+  base::test::TaskEnvironment env;
+  TestObject x;
+  bool r = false;
+  TestSequence::Post(FROM_HERE, &TestObject::DoSomethingWithRequiredSequence,
+                     base::Unretained(&x), &r);
+  EXPECT_FALSE(r);
+  env.RunUntilIdle();
+  EXPECT_TRUE(r);
+}
+
+TEST(StaticSequenceTest, PostUnprotectedFreeFunction) {
+  base::test::TaskEnvironment env;
+  bool r = false;
+  TestSequence::Post(FROM_HERE, &DoSomething, &r);
+  EXPECT_FALSE(r);
+  env.RunUntilIdle();
+  EXPECT_TRUE(r);
+}
+
+TEST(StaticSequenceTest, PostProtectedFreeFunction) {
+  base::test::TaskEnvironment env;
+  bool r = false;
+  TestSequence::Post(FROM_HERE, &DoSomethingWithRequiredSequence, &r);
+  EXPECT_FALSE(r);
+  env.RunUntilIdle();
+  EXPECT_TRUE(r);
+}
+
+}  // namespace util

chromecast/base/static_sequence/static_sequence_unittest.nc

+// Copyright 2019 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.
+
+// This is a no-compile test suite.
+// http://dev.chromium.org/developers/testing/no-compile-tests
+
+#include "chromecast/base/static_sequence/static_sequence.h"
+
+namespace util {
+
+struct SequenceA : StaticSequence<SequenceA> {};
+struct SequenceB : StaticSequence<SequenceB> {};
+
+void Foo(SequenceA::Key&);
+
+class Bar {
+ public:
+  void Baz(SequenceA::Key&) {}
+};
+
+void StaticSequenceNoCompileTests() {
+  Sequenced<Bar, SequenceB> bar;
+#if defined(NCTEST_POST_FUNCTION_TO_WRONG_SEQUENCE) // [r"fatal error: static_assert failed due to requirement 'invalid<util::SequenceB, util::SequenceA>' \"Attempting to post a statically-sequenced task to the wrong static sequence!\""]
+  SequenceB::PostTask(base::BindOnce(&Foo));
+#elif defined(NCTEST_POST_METHOD_TO_WRONG_SEQUENCE) // [r"fatal error: static_assert failed due to requirement 'invalid<util::SequenceB, util::SequenceA>' \"Attempting to post a statically-sequenced task to the wrong static sequence!\""]
+  bar.Post(FROM_HERE, &Bar::Baz);
+#endif
+}
+
+}  // namespace util