Make TaskGraphWorkQueue more robust against changing dependencies

Currently, if a task in the TaskGraphWorkQueue goes from having no
dependencies to having dependencies, the TaskGraphWorkQueue may
incorrectly schedule it to run twice.

This change adds an additional check to make sure we handle this case.

R=vmpstr
BUG=652718
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2643153002
Cr-Commit-Position: refs/heads/master@{#445425}

diff --git a/cc/BUILD.gn b/cc/BUILD.gn
index 3f37b8b..911fe486 100644
--- a/cc/BUILD.gn
+++ b/cc/BUILD.gn

@@ -813,6 +813,7 @@
     "raster/scoped_gpu_raster_unittest.cc",
     "raster/single_thread_task_graph_runner_unittest.cc",
     "raster/synchronous_task_graph_runner_unittest.cc",
+    "raster/task_graph_work_queue_unittest.cc",
     "raster/texture_compressor_etc1_unittest.cc",
     "resources/platform_color_unittest.cc",
     "resources/resource_pool_unittest.cc",

diff --git a/cc/raster/task_graph_work_queue.cc b/cc/raster/task_graph_work_queue.cc
index 702b82d0..aa0dc6ac 100644
--- a/cc/raster/task_graph_work_queue.cc
+++ b/cc/raster/task_graph_work_queue.cc

@@ -308,8 +308,9 @@
 
     DCHECK_LT(0u, dependent_node.dependencies);
     dependent_node.dependencies--;
-    // Task is ready if it has no dependencies. Add it to |ready_to_run_tasks_|.
-    if (!dependent_node.dependencies) {
+    // Task is ready if it has no dependencies and is in the new state, Add it
+    // to |ready_to_run_tasks_|.
+    if (!dependent_node.dependencies && dependent_node.task->state().IsNew()) {
       PrioritizedTask::Vector& ready_to_run_tasks =
           task_namespace->ready_to_run_tasks[dependent_node.category];
 

diff --git a/cc/raster/task_graph_work_queue_unittest.cc b/cc/raster/task_graph_work_queue_unittest.cc
new file mode 100644
index 0000000..cf135c3
--- /dev/null
+++ b/cc/raster/task_graph_work_queue_unittest.cc

@@ -0,0 +1,62 @@
+// Copyright 2013 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 "cc/raster/task_graph_work_queue.h"
+
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace cc {
+namespace {
+
+class FakeTaskImpl : public Task {
+ public:
+  FakeTaskImpl() {}
+
+  // Overridden from Task:
+  void RunOnWorkerThread() override{};
+
+ private:
+  ~FakeTaskImpl() override {}
+  DISALLOW_COPY_AND_ASSIGN(FakeTaskImpl);
+};
+
+TEST(TaskGraphWorkQueueTest, TestChangingDependency) {
+  TaskGraphWorkQueue work_queue;
+  NamespaceToken token = work_queue.GenerateNamespaceToken();
+
+  // Create a graph with one task
+  TaskGraph graph1;
+  scoped_refptr<FakeTaskImpl> task(new FakeTaskImpl());
+  graph1.nodes.push_back(TaskGraph::Node(task.get(), 0u, 0u, 0u));
+
+  // Schedule the graph
+  work_queue.ScheduleTasks(token, &graph1);
+
+  // Run the task.
+  TaskGraphWorkQueue::PrioritizedTask prioritized_task =
+      work_queue.GetNextTaskToRun(0u);
+  work_queue.CompleteTask(std::move(prioritized_task));
+
+  // Create a graph where task1 has a dependency
+  TaskGraph graph2;
+  scoped_refptr<FakeTaskImpl> dependency_task(new FakeTaskImpl());
+  graph2.nodes.push_back(TaskGraph::Node(task.get(), 0u, 0u, 1u));
+  graph2.nodes.push_back(TaskGraph::Node(dependency_task.get(), 0u, 0u, 0u));
+  graph2.edges.push_back(TaskGraph::Edge(dependency_task.get(), task.get()));
+
+  // Schedule the second graph.
+  work_queue.ScheduleTasks(token, &graph2);
+
+  // Run the |dependency_task|
+  TaskGraphWorkQueue::PrioritizedTask prioritized_dependency_task =
+      work_queue.GetNextTaskToRun(0u);
+  EXPECT_EQ(prioritized_dependency_task.task.get(), dependency_task.get());
+  work_queue.CompleteTask(std::move(prioritized_dependency_task));
+
+  // We should have no tasks to run, as the dependent task already completed.
+  EXPECT_FALSE(work_queue.HasReadyToRunTasks());
+}
+
+}  // namespace
+}  // namespace cc