TaskScheduler [4/9] Priority Queue

base/task_scheduler/priority_queue_unittest.cc

Index: base/task_scheduler/priority_queue_unittest.cc
diff --git a/base/task_scheduler/priority_queue_unittest.cc b/base/task_scheduler/priority_queue_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..c8dc1bc264b68a7a5a8d22bab9eb2a924e7e4bc7
--- /dev/null
+++ b/base/task_scheduler/priority_queue_unittest.cc
@@ -0,0 +1,162 @@
+// 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.
+
+#include "base/task_scheduler/priority_queue.h"
+
+#include "base/bind.h"
+#include "base/bind_helpers.h"
+#include "base/macros.h"
+#include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/task_scheduler/sequence.h"
+#include "base/task_scheduler/task.h"
+#include "base/task_scheduler/task_traits.h"
+#include "base/time/time.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace base {
+namespace internal {
+
+namespace {
+
+class PriorityQueueCallbackMock {
+ public:
+  PriorityQueueCallbackMock() = default;
+  MOCK_METHOD0(SequenceInsertedInPriorityQueue, void());
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(PriorityQueueCallbackMock);
+};
+
+void ExpectSequenceAndSortKey(const PriorityQueue::SequenceAndSortKey& expected,
+                              const PriorityQueue::SequenceAndSortKey& actual) {
+  EXPECT_EQ(expected.sequence, actual.sequence);
+  EXPECT_EQ(expected.sort_key.priority, actual.sort_key.priority);
+  EXPECT_EQ(expected.sort_key.next_task_sequenced_time,
+            actual.sort_key.next_task_sequenced_time);
+  EXPECT_EQ(expected.is_null(), actual.is_null());

[gab, 2016/02/23 20:58:42]

Don't think this is necessary. Since is_null() is const and the above verifies
all other state, it's impossible for any state to be equivalent above but not
result in the same is_null().

[fdoray, 2016/02/24 15:03:53]

It tested the implementation of is_null(). I created a separate test for this
instead.

+}
+
+}  // namespace
+
+// Check the behavior of the Push, Pop and Peek methods of

[gab, 2016/02/23 20:58:42]

s/Check/Verifies/

(and overall I think the test being named "PushPopPeek" is enough and a comment
isn't necessary)

[fdoray, 2016/02/24 15:03:53]

Done. Removed the comment.

+// PriorityQueue::Transaction.
+TEST(TaskSchedulerPriorityQueueTest, PushPopPeek) {
+  // Create test sequences.
+  scoped_refptr<Sequence> sequence_a = new Sequence;
+  sequence_a->PushTask(make_scoped_ptr(
+      new Task(FROM_HERE, Closure(),
+               TaskTraits().WithPriority(TaskPriority::USER_VISIBLE))));
+  SequenceSortKey sort_key_a = sequence_a->GetSortKey();
+
+  scoped_refptr<Sequence> sequence_b = new Sequence;
+  sequence_b->PushTask(make_scoped_ptr(
+      new Task(FROM_HERE, Closure(),
+               TaskTraits().WithPriority(TaskPriority::USER_BLOCKING))));
+  SequenceSortKey sort_key_b = sequence_b->GetSortKey();
+
+  scoped_refptr<Sequence> sequence_c = new Sequence;
+  sequence_c->PushTask(make_scoped_ptr(
+      new Task(FROM_HERE, Closure(),
+               TaskTraits().WithPriority(TaskPriority::USER_BLOCKING))));
+  SequenceSortKey sort_key_c = sequence_c->GetSortKey();
+
+  scoped_refptr<Sequence> sequence_d = new Sequence;
+  sequence_d->PushTask(make_scoped_ptr(
+      new Task(FROM_HERE, Closure(),
+               TaskTraits().WithPriority(TaskPriority::BACKGROUND))));
+  SequenceSortKey sort_key_d = sequence_d->GetSortKey();
+
+  // Create a priority queue and a transaction.
+  testing::StrictMock<PriorityQueueCallbackMock> mock;
+  PriorityQueue pq(
+      Bind(&PriorityQueueCallbackMock::SequenceInsertedInPriorityQueue,
+           Unretained(&mock)));
+  scoped_ptr<PriorityQueue::Transaction> transaction(pq.BeginTransaction());
+  ExpectSequenceAndSortKey(PriorityQueue::SequenceAndSortKey::Null(),
+                           transaction->Peek());
+
+  // Push sequences in the priority queue.
+  transaction->Push(make_scoped_ptr(
+      new PriorityQueue::SequenceAndSortKey(sequence_a, sort_key_a)));
+  ExpectSequenceAndSortKey(
+      PriorityQueue::SequenceAndSortKey(sequence_a, sort_key_a),
+      transaction->Peek());
+
+  transaction->Push(make_scoped_ptr(
+      new PriorityQueue::SequenceAndSortKey(sequence_b, sort_key_b)));
+  ExpectSequenceAndSortKey(
+      PriorityQueue::SequenceAndSortKey(sequence_b, sort_key_b),
+      transaction->Peek());
+
+  transaction->Push(make_scoped_ptr(
+      new PriorityQueue::SequenceAndSortKey(sequence_c, sort_key_c)));
+  ExpectSequenceAndSortKey(
+      PriorityQueue::SequenceAndSortKey(sequence_b, sort_key_b),
+      transaction->Peek());
+
+  transaction->Push(make_scoped_ptr(
+      new PriorityQueue::SequenceAndSortKey(sequence_d, sort_key_d)));
+  ExpectSequenceAndSortKey(
+      PriorityQueue::SequenceAndSortKey(sequence_b, sort_key_b),
+      transaction->Peek());
+
+  // Pop sequences from the priority queue.
+  transaction->Pop();
+  ExpectSequenceAndSortKey(
+      PriorityQueue::SequenceAndSortKey(sequence_c, sort_key_c),
+      transaction->Peek());
+
+  transaction->Pop();
+  ExpectSequenceAndSortKey(
+      PriorityQueue::SequenceAndSortKey(sequence_a, sort_key_a),
+      transaction->Peek());
+
+  transaction->Pop();
+  ExpectSequenceAndSortKey(
+      PriorityQueue::SequenceAndSortKey(sequence_d, sort_key_d),
+      transaction->Peek());
+
+  transaction->Pop();
+  ExpectSequenceAndSortKey(PriorityQueue::SequenceAndSortKey::Null(),
+                           transaction->Peek());
+
+  // Expect 4 calls to mock.SequenceInsertedInPriorityQueue() when the
+  // Transaction is destroyed.
+  EXPECT_CALL(mock, SequenceInsertedInPriorityQueue()).Times(4);
+  transaction.reset();
+}
+
+// Check that creating Transactions on the same thread for 2 unrelated

[robliao, 2016/02/23 21:00:35]

It would also be interesting to create two transactions on different threads for
the same PQ to make sure that doesn't crash as well.

[fdoray, 2016/02/24 15:03:53]

Done.

+// PriorityQueues causes a crash.
+TEST(TaskSchedulerPriorityQueueTest, IllegalMultipleTransactions) {
+  PriorityQueue pq_a(Bind(&DoNothing));

[gab, 2016/02/23 20:58:42]

s/Bind(&DoNothing)/Closure()/ 

here and below? I guess you're doing this because the compiler doesn't allow a
Closure which is_null(). Not sure this strict requirement is required. If you
think it is, add to the API comment that the param must be non-null.

[fdoray, 2016/02/24 15:03:53]

Done. Added comment in PriorityQueue to say that the callback can't be null. 

+  PriorityQueue pq_b(Bind(&DoNothing));
+
+  EXPECT_DEBUG_DEATH(
+      {

[robliao, 2016/02/23 21:00:35]

Follow the lambda expressions style for this block:
https://google.github.io/styleguide/cppguide.html#Lambda_expressions

EXPECT_DEBUG_DEATH({
  // Body 2 Space Indent
}, "");

[fdoray, 2016/02/24 15:03:53]

Done.

+        scoped_ptr<PriorityQueue::Transaction> transaction_a =
+            pq_a.BeginTransaction();
+        scoped_ptr<PriorityQueue::Transaction> transaction_b =
+            pq_b.BeginTransaction();
+      },
+      "");
+}
+
+// Check that there is no crash when Transactions are created on the same thread
+// for 2 PriorityQueues which have a predecessor relationship.
+TEST(TaskSchedulerPriorityQueueTest, LegalMultipleTransactions) {
+  PriorityQueue pq_a(Bind(&DoNothing));
+  PriorityQueue pq_b(Bind(&DoNothing), &pq_a);
+
+  // This shouldn't crash.
+  scoped_ptr<PriorityQueue::Transaction> transaction_a =
+      pq_a.BeginTransaction();
+  scoped_ptr<PriorityQueue::Transaction> transaction_b =
+      pq_b.BeginTransaction();
+}
+
+}  // namespace internal
+}  // namespace base