TaskScheduler [2/9] Scheduler Lock

base/task_scheduler/scheduler_lock.h

Index: base/task_scheduler/scheduler_lock.h
diff --git a/base/task_scheduler/scheduler_lock.h b/base/task_scheduler/scheduler_lock.h
new file mode 100644
index 0000000000000000000000000000000000000000..639def31dd02b0106c00968d2baebe9673f60b0f
--- /dev/null
+++ b/base/task_scheduler/scheduler_lock.h
@@ -0,0 +1,58 @@
+// Copyright 2016 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 BASE_TASK_SCHEDULER_SCHEDULER_LOCK_H
+#define BASE_TASK_SCHEDULER_SCHEDULER_LOCK_H
+
+#include <vector>
+
+#include "base/base_export.h"
+#include "base/macros.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/synchronization/condition_variable.h"
+#include "base/synchronization/lock.h"
+#include "base/task_scheduler/scheduler_lock_impl.h"
+
+namespace base {
+namespace internal {
+

[gab, 2016/02/18 16:58:06]

Add a meta comment here on how this class should be used (e.g. both constructors
and CreateConditionVariable()). I think that's better than adding comments on
each method below since they're defined twice + CreateConditionVariable() is
only defined here for the non-impl.

So I'd say meta comment here and then no/minimal comments below and in impl
header (as you have now).

Also mention in that meta-comment that loops in predecessor chains are prevented
by the predecessor only being registerable at construction time.

[robliao, 2016/02/18 21:59:36]

Done, except for the loops part. The unit tests actually constructs some loops.

+#if DCHECK_IS_ON()
+class SchedulerLock : public SchedulerLockImpl {
+ public:
+  SchedulerLock() = default;
+  explicit SchedulerLock(const SchedulerLock* predecessor)
+      : SchedulerLockImpl(predecessor) {}
+};
+#else

[gab, 2016/02/18 16:58:06]

#else  // DCHECK_IS_ON()

[robliao, 2016/02/18 21:59:36]

Done.

+class SchedulerLock : public Lock {
+ public:
+  SchedulerLock() = default;
+  explicit SchedulerLock(const SchedulerLock*) {}
+
+  // Functionally equivalent to SchedulerLockImpl::CreateConditionVariable.
+  scoped_ptr<ConditionVariable> CreateConditionVariable() {
+    return scoped_ptr<ConditionVariable>(new ConditionVariable(this));
+  }
+};
+#endif

[gab, 2016/02/18 16:58:06]

#endif  // DCHECK_IS_ON()

[robliao, 2016/02/18 21:59:36]

Done.

+
+class AutoSchedulerLock {
+ public:
+  explicit AutoSchedulerLock(SchedulerLock& lock) : lock_(lock) {
+    lock_.Acquire();
+  }
+
+  ~AutoSchedulerLock() {
+    lock_.Release();

[gab, 2016/02/18 16:58:06]

Add

lock_.AssertAcquired();

above.

(to match AutoLock's impl:
https://code.google.com/p/chromium/codesearch#chromium/src/base/synchronizati...)

[robliao, 2016/02/18 21:59:36]

Done.

+  }
+
+ private:
+  SchedulerLock& lock_;
+  DISALLOW_COPY_AND_ASSIGN(AutoSchedulerLock);

[gab, 2016/02/18 16:58:06]

Chromium style is typically to have an empty line above DISALLOW_COPY_AND_ASSIGN
macro

[robliao, 2016/02/18 21:59:36]

Done.

+};
+
+}  // namespace internal
+}  // namespace base
+
+#endif  // BASE_TASK_SCHEDULER_SCHEDULER_LOCK_H