Shared Raster Worker Threads

cc/resources/worker_pool_unittest.cc

Index: cc/resources/worker_pool_unittest.cc
diff --git a/cc/resources/worker_pool_unittest.cc b/cc/resources/worker_pool_unittest.cc
old mode 100644
new mode 100755
index f7c5a951df6de92dffb4b344f2fce408b06f3298..50e3fe77ef876d4227b1ffbac3ff34261bff55c7
--- a/cc/resources/worker_pool_unittest.cc
+++ b/cc/resources/worker_pool_unittest.cc
@@ -60,7 +60,7 @@ class FakeWorkerPool : public WorkerPool {
     unsigned dependent_count;
     unsigned priority;
   };
-  FakeWorkerPool() : WorkerPool(1, "test") {}
+  FakeWorkerPool() : WorkerPool() {}
   virtual ~FakeWorkerPool() {}
 
   static scoped_ptr<FakeWorkerPool> Create() {
@@ -181,6 +181,15 @@ class WorkerPoolTest : public testing::Test {
     return on_task_completed_ids_;
   }
 
+  unsigned find_run_task_id(unsigned prio, unsigned size) {
+    unsigned id = 9999;
+    for (unsigned i = 0; i < size; ++i) {
+      if (run_task_ids_[i] == prio)
+        id = i;
+    }
+    return id;
+  }

[reveman, 2014/01/10 21:22:03]

not sure why you need this but it looks like you're better of using std::find
instead if necessary.

[sohanjg, 2014/01/13 13:24:51]

Done.

+
  private:
   scoped_ptr<FakeWorkerPool> worker_pool_;

[reveman, 2014/01/10 21:22:03]

why not make this a ScopedVector and add a "const int kWorkerPoolCount = 3"
constant that is used to determine the size? instead of having to manually
create worker pools in each test..

[sohanjg, 2014/01/13 13:24:51]

Done.

   std::vector<unsigned> run_task_ids_;
@@ -191,24 +200,26 @@ TEST_F(WorkerPoolTest, Basic) {
   EXPECT_EQ(0u, run_task_ids().size());
   EXPECT_EQ(0u, on_task_completed_ids().size());
 
-  worker_pool()->ScheduleTasks(
-      std::vector<FakeWorkerPool::Task>(
-          1,
-          FakeWorkerPool::Task(base::Bind(&WorkerPoolTest::RunTask,
-                                          base::Unretained(this),
-                                          0u),
-                               base::Bind(&WorkerPoolTest::OnTaskCompleted,
-                                          base::Unretained(this),
-                                          0u),
-                               base::Closure(),
-                               1u,
-                               0u)));
-  RunAllTasks();
-
-  EXPECT_EQ(1u, run_task_ids().size());
-  EXPECT_EQ(1u, on_task_completed_ids().size());
-
-  worker_pool()->ScheduleTasks(
+  const int pool_size = 3;
+  scoped_ptr<FakeWorkerPool> worker_pools[pool_size];
+
+  for (int i = 0; i < pool_size ; i++)
+    worker_pools[i] = FakeWorkerPool::Create();
+
+  worker_pools[0]->ScheduleTasks(
+        std::vector<FakeWorkerPool::Task>(
+            1,
+            FakeWorkerPool::Task(base::Bind(&WorkerPoolTest::RunTask,
+                                            base::Unretained(this),
+                                            0u),
+                                 base::Bind(&WorkerPoolTest::OnTaskCompleted,
+                                            base::Unretained(this),
+                                            0u),
+                                 base::Closure(),
+                                 1u,
+                                 0u)));
+
+  worker_pools[1]->ScheduleTasks(
       std::vector<FakeWorkerPool::Task>(
           1,
           FakeWorkerPool::Task(base::Bind(&WorkerPoolTest::RunTask,
@@ -222,12 +233,8 @@ TEST_F(WorkerPoolTest, Basic) {
                                           0u),
                                1u,
                                0u)));
-  RunAllTasks();
 
-  EXPECT_EQ(3u, run_task_ids().size());
-  EXPECT_EQ(2u, on_task_completed_ids().size());
-
-  worker_pool()->ScheduleTasks(
+  worker_pools[2]->ScheduleTasks(
       std::vector<FakeWorkerPool::Task>(
           1, FakeWorkerPool::Task(base::Bind(&WorkerPoolTest::RunTask,
                                              base::Unretained(this),
@@ -240,14 +247,22 @@ TEST_F(WorkerPoolTest, Basic) {
                                              0u),
                                   2u,
                                   0u)));
-  RunAllTasks();
+  for (int i = 0; i < pool_size ; i++) {
+    worker_pools[i]->WaitForTasksToComplete();
+    worker_pools[i]->CheckForCompletedTasks();
+  }
 
   EXPECT_EQ(6u, run_task_ids().size());
   EXPECT_EQ(3u, on_task_completed_ids().size());
 }
 
 TEST_F(WorkerPoolTest, Dependencies) {
-  worker_pool()->ScheduleTasks(
+  const int pool_size = 2;
+  scoped_ptr<FakeWorkerPool> worker_pools[pool_size];
+  for (int i = 0; i < pool_size ; i++)
+    worker_pools[i] = FakeWorkerPool::Create();
+
+  worker_pools[0]->ScheduleTasks(
       std::vector<FakeWorkerPool::Task>(
           1, FakeWorkerPool::Task(base::Bind(&WorkerPoolTest::RunTask,
                                              base::Unretained(this),
@@ -260,16 +275,8 @@ TEST_F(WorkerPoolTest, Dependencies) {
                                              1u),
                                   1u,
                                   0u)));
-  RunAllTasks();
-
-  // Check if task ran before dependent.
-  ASSERT_EQ(2u, run_task_ids().size());
-  EXPECT_EQ(0u, run_task_ids()[0]);
-  EXPECT_EQ(1u, run_task_ids()[1]);
-  ASSERT_EQ(1u, on_task_completed_ids().size());
-  EXPECT_EQ(0u, on_task_completed_ids()[0]);
 
-  worker_pool()->ScheduleTasks(
+  worker_pools[1]->ScheduleTasks(
       std::vector<FakeWorkerPool::Task>(
           1, FakeWorkerPool::Task(base::Bind(&WorkerPoolTest::RunTask,
                                              base::Unretained(this),
@@ -282,20 +289,31 @@ TEST_F(WorkerPoolTest, Dependencies) {
                                              3u),
                                   2u,
                                   0u)));
-  RunAllTasks();
+  for (int i = 0; i < pool_size ; i++) {
+    worker_pools[i]->WaitForTasksToComplete();
+    worker_pools[i]->CheckForCompletedTasks();
+  }
 
-  // Task should only run once.
+  // Check if task ran before dependent.
   ASSERT_EQ(5u, run_task_ids().size());
-  EXPECT_EQ(2u, run_task_ids()[2]);
-  EXPECT_EQ(3u, run_task_ids()[3]);
-  EXPECT_EQ(3u, run_task_ids()[4]);
+  unsigned id_task = find_run_task_id(0u, run_task_ids().size());
+  unsigned id_dep = find_run_task_id(1u, run_task_ids().size());
+  EXPECT_EQ((id_dep-id_task) > 0, 1);
+  id_task = find_run_task_id(2u, run_task_ids().size());
+  id_dep = find_run_task_id(3u, run_task_ids().size());
+  EXPECT_EQ((id_dep-id_task) > 0, 1);
   ASSERT_EQ(2u, on_task_completed_ids().size());
   EXPECT_EQ(2u, on_task_completed_ids()[1]);
 }
 
 TEST_F(WorkerPoolTest, Priority) {
+  const int pool_size = 4;
+  scoped_ptr<FakeWorkerPool> worker_pools[pool_size];
+  for (int i = 0; i < pool_size ; i++)
+    worker_pools[i] = FakeWorkerPool::Create();
+
   {
-    FakeWorkerPool::Task tasks[] = {
+    FakeWorkerPool::Task tasks1[] = {
         FakeWorkerPool::Task(base::Bind(&WorkerPoolTest::RunTask,
                                         base::Unretained(this),
                                         0u),
@@ -319,25 +337,60 @@ TEST_F(WorkerPoolTest, Priority) {
                              1u,
                              0u)  // Priority 0
     };
-    worker_pool()->ScheduleTasks(
-        std::vector<FakeWorkerPool::Task>(tasks, tasks + arraysize(tasks)));
+    FakeWorkerPool::Task tasks2[] = {
+        FakeWorkerPool::Task(base::Bind(&WorkerPoolTest::RunTask,
+                                        base::Unretained(this),
+                                        0u),
+                             base::Bind(&WorkerPoolTest::OnTaskCompleted,
+                                        base::Unretained(this),
+                                        0u),
+                             base::Bind(&WorkerPoolTest::RunTask,
+                                        base::Unretained(this),
+                                        4u),
+                             1u,
+                             2u),  // Priority 2
+        FakeWorkerPool::Task(base::Bind(&WorkerPoolTest::RunTask,
+                                        base::Unretained(this),
+                                        1u),
+                             base::Bind(&WorkerPoolTest::OnTaskCompleted,
+                                        base::Unretained(this),
+                                        1u),
+                             base::Bind(&WorkerPoolTest::RunTask,
+                                        base::Unretained(this),
+                                        5u),
+                             1u,
+                             3u)  // Priority 3
+    };
+    worker_pools[0]->ScheduleTasks(
+        std::vector<FakeWorkerPool::Task>(tasks1, tasks1 + arraysize(tasks1)));
+    worker_pools[1]->ScheduleTasks(
+        std::vector<FakeWorkerPool::Task>(tasks2, tasks2 + arraysize(tasks2)));
+  }
+  for (int i = 0; i < 2 ; i++) {
+    worker_pools[i]->WaitForTasksToComplete();
+    worker_pools[i]->CheckForCompletedTasks();
   }
-  RunAllTasks();
 
   // Check if tasks ran in order of priority.
-  ASSERT_EQ(4u, run_task_ids().size());
+  ASSERT_EQ(8u, run_task_ids().size());
   EXPECT_EQ(1u, run_task_ids()[0]);
   EXPECT_EQ(3u, run_task_ids()[1]);
   EXPECT_EQ(0u, run_task_ids()[2]);
   EXPECT_EQ(2u, run_task_ids()[3]);
-  ASSERT_EQ(2u, on_task_completed_ids().size());
+  EXPECT_EQ(0u, run_task_ids()[4]);
+  EXPECT_EQ(4u, run_task_ids()[5]);
+  EXPECT_EQ(1u, run_task_ids()[6]);
+  EXPECT_EQ(5u, run_task_ids()[7]);
+  ASSERT_EQ(4u, on_task_completed_ids().size());
   EXPECT_EQ(1u, on_task_completed_ids()[0]);
   EXPECT_EQ(0u, on_task_completed_ids()[1]);
+  EXPECT_EQ(0u, on_task_completed_ids()[2]);
+  EXPECT_EQ(1u, on_task_completed_ids()[3]);
 
   ResetIds();
   {
-    std::vector<FakeWorkerPool::Task> tasks;
-    tasks.push_back(
+    std::vector<FakeWorkerPool::Task> tasks1;
+    tasks1.push_back(
         FakeWorkerPool::Task(base::Bind(&WorkerPoolTest::RunTask,
                                         base::Unretained(this),
                                         0u),
@@ -349,7 +402,7 @@ TEST_F(WorkerPoolTest, Priority) {
                                         3u),
                              1u,    // 1 dependent
                              1u));  // Priority 1
-    tasks.push_back(
+    tasks1.push_back(
         FakeWorkerPool::Task(base::Bind(&WorkerPoolTest::RunTask,
                                         base::Unretained(this),
                                         1u),
@@ -361,7 +414,7 @@ TEST_F(WorkerPoolTest, Priority) {
                                         4u),
                              2u,    // 2 dependents
                              1u));  // Priority 1
-    tasks.push_back(
+    tasks1.push_back(
         FakeWorkerPool::Task(base::Bind(&WorkerPoolTest::RunTask,
                                         base::Unretained(this),
                                         2u),
@@ -373,20 +426,69 @@ TEST_F(WorkerPoolTest, Priority) {
                                         5u),
                              1u,    // 1 dependent
                              0u));  // Priority 0
-    worker_pool()->ScheduleTasks(tasks);
+    worker_pools[3]->ScheduleTasks(tasks1);
+
+    std::vector<FakeWorkerPool::Task> tasks2;
+    tasks2.push_back(
+        FakeWorkerPool::Task(base::Bind(&WorkerPoolTest::RunTask,
+                                        base::Unretained(this),
+                                        5u),
+                             base::Bind(&WorkerPoolTest::OnTaskCompleted,
+                                        base::Unretained(this),
+                                        5u),
+                             base::Bind(&WorkerPoolTest::RunTask,
+                                        base::Unretained(this),
+                                        6u),
+                             3u,    // 3 dependent
+                             3u));  // Priority 3
+    tasks2.push_back(
+        FakeWorkerPool::Task(base::Bind(&WorkerPoolTest::RunTask,
+                                        base::Unretained(this),
+                                        4u),
+                             base::Bind(&WorkerPoolTest::OnTaskCompleted,
+                                        base::Unretained(this),
+                                        4u),
+                             base::Bind(&WorkerPoolTest::RunTask,
+                                        base::Unretained(this),
+                                        7u),
+                             3u,    // 3 dependents
+                             2u));  // Priority 2
+    tasks2.push_back(
+        FakeWorkerPool::Task(base::Bind(&WorkerPoolTest::RunTask,
+                                        base::Unretained(this),
+                                        3u),
+                             base::Bind(&WorkerPoolTest::OnTaskCompleted,
+                                        base::Unretained(this),
+                                        3u),
+                             base::Bind(&WorkerPoolTest::RunTask,
+                                        base::Unretained(this),
+                                        8u),
+                             4u,    // 4 dependent
+                             0u));  // Priority 0
+    worker_pools[4]->ScheduleTasks(tasks2);
+  }
+  for (int i = 2; i < pool_size ; i++) {
+    worker_pools[i]->WaitForTasksToComplete();
+    worker_pools[i]->CheckForCompletedTasks();
   }
-  RunAllTasks();
 
   // Check if tasks ran in order of priority and that task with more
   // dependents ran first when priority is the same.
-  ASSERT_LE(3u, run_task_ids().size());
-  EXPECT_EQ(2u, run_task_ids()[0]);
-  EXPECT_EQ(5u, run_task_ids()[1]);
-  EXPECT_EQ(1u, run_task_ids()[2]);
-  ASSERT_EQ(3u, on_task_completed_ids().size());
+  ASSERT_LE(6u, run_task_ids().size());
+  EXPECT_EQ(3u, run_task_ids()[0]);
+  EXPECT_EQ(8u, run_task_ids()[1]);
+  EXPECT_EQ(2u, run_task_ids()[2]);
+  EXPECT_EQ(5u, run_task_ids()[3]);
+  EXPECT_EQ(8u, run_task_ids()[4]);
+  EXPECT_EQ(8u, run_task_ids()[5]);
+  EXPECT_EQ(8u, run_task_ids()[6]);
+  ASSERT_EQ(6u, on_task_completed_ids().size());
   EXPECT_EQ(2u, on_task_completed_ids()[0]);
   EXPECT_EQ(1u, on_task_completed_ids()[1]);
   EXPECT_EQ(0u, on_task_completed_ids()[2]);
+  EXPECT_EQ(3u, on_task_completed_ids()[3]);
+  EXPECT_EQ(4u, on_task_completed_ids()[4]);
+  EXPECT_EQ(5u, on_task_completed_ids()[5]);
 }

[reveman, 2014/01/10 21:22:03]

The way you're modifying these tests make it really hard see if the logic is
changing or not. Please try to keep the code and logic as intact as possible.
That makes the changes much easier for me to review.

I think adding a vector of WorkerPool instances to WorkerPoolTest as I suggested
above is a good way to accomplish this.

[sohanjg, 2014/01/13 13:24:51]

Done.

 
 }  // namespace