Fix bug with TaskQueueSelector and blocked queues

components/scheduler/base/work_queue_sets.cc

Index: components/scheduler/base/work_queue_sets.cc
diff --git a/components/scheduler/base/work_queue_sets.cc b/components/scheduler/base/work_queue_sets.cc
index f952334e733c31c99b3209cd3f9e806764795634..99b51e2c34c70751b6f10ad4d61fd02d38a3f11e 100644
--- a/components/scheduler/base/work_queue_sets.cc
+++ b/components/scheduler/base/work_queue_sets.cc
@@ -10,15 +10,29 @@
 namespace scheduler {
 namespace internal {
 
-WorkQueueSets::WorkQueueSets(size_t num_sets)
-    : enqueue_order_to_work_queue_maps_(num_sets) {}
+WorkQueueSets::WorkQueueSets(size_t num_sets, const char* name)
+    : enqueue_order_to_work_queue_maps_(num_sets), name_(name) {}
 
 WorkQueueSets::~WorkQueueSets() {}
 
+void WorkQueueSets::AddQueue(WorkQueue* work_queue, size_t set_index) {
+  DCHECK(!work_queue->work_queue_sets());
+  DCHECK_LT(set_index, enqueue_order_to_work_queue_maps_.size());
+  EnqueueOrder enqueue_order;
+  bool has_enqueue_order = work_queue->GetFrontTaskEnqueueOrder(&enqueue_order);
+  work_queue->AssignToWorkQueueSets(this);
+  work_queue->AssignSetIndex(set_index);
+  if (!has_enqueue_order)
+    return;
+  enqueue_order_to_work_queue_maps_[set_index].insert(

[Sami, 2016/02/10 13:15:47]

Should we call OnPushQueue instead? If we ever add anything there we might
forget to do it here too.

[alex clarke (OOO till 29th), 2016/02/10 14:37:12]

We'd end up calling work_queue->GetFrontTaskEnqueueOrder twice which feels a bit
unfortunate :(

[Sami, 2016/02/10 15:30:38]

Yeah, I was hoping the compiler would noticed that. Let's add a comment to
OnPushQueue that points here instead?

[alex clarke (OOO till 29th), 2016/02/10 17:26:08]

Done.

+      std::make_pair(enqueue_order, work_queue));
+}
+
 void WorkQueueSets::RemoveQueue(WorkQueue* work_queue) {
   DCHECK_EQ(this, work_queue->work_queue_sets());
   EnqueueOrder enqueue_order;
   bool has_enqueue_order = work_queue->GetFrontTaskEnqueueOrder(&enqueue_order);
+  work_queue->AssignToWorkQueueSets(nullptr);
   if (!has_enqueue_order)
     return;
   size_t set_index = work_queue->work_queue_set_index();
@@ -29,13 +43,15 @@ void WorkQueueSets::RemoveQueue(WorkQueue* work_queue) {
   enqueue_order_to_work_queue_maps_[set_index].erase(enqueue_order);
 }
 
-void WorkQueueSets::AssignQueueToSet(WorkQueue* work_queue, size_t set_index) {
+void WorkQueueSets::ChangeSetIndex(WorkQueue* work_queue, size_t set_index) {
+  DCHECK_EQ(this, work_queue->work_queue_sets());
   DCHECK_LT(set_index, enqueue_order_to_work_queue_maps_.size());
   EnqueueOrder enqueue_order;
   bool has_enqueue_order = work_queue->GetFrontTaskEnqueueOrder(&enqueue_order);
   size_t old_set = work_queue->work_queue_set_index();
   DCHECK_LT(old_set, enqueue_order_to_work_queue_maps_.size());
-  work_queue->AssignToWorkQueueSets(this, set_index);
+  DCHECK_NE(old_set, set_index);
+  work_queue->AssignSetIndex(set_index);
   if (!has_enqueue_order)
     return;
   enqueue_order_to_work_queue_maps_[old_set].erase(enqueue_order);
@@ -114,6 +130,11 @@ bool WorkQueueSets::ContainsWorkQueueForTest(WorkQueue* work_queue) const {
     }
   }
 
+  if (work_queue->work_queue_sets() == this) {

[Sami, 2016/02/10 13:15:47]

When would the work queue be assigned to a set but not actually be part of it?
Could we avoid this situation?

[alex clarke (OOO till 29th), 2016/02/10 14:37:12]

Consider:

WorkQueueSets a;
WorkQueueSets b;
WorkQueue q;

a.AddQueue(&q, 1);
DCHECK(a.ContainsWorkQueueForTest(&q));
DCHECK(!b.ContainsWorkQueueForTest(&q));

|q| must only be in one WorkQueueSets at a time.

if |has_enqueue_order| is not null (ie. its not empty) then it must have an
entry in enqueue_order_to_work_queue_maps_.

That's what it's guarding against.

[Sami, 2016/02/10 15:30:38]

Ah, the case of an empty queue. Makes sense, thanks.

+    DCHECK(!has_enqueue_order);
+    return true;
+  }
+
   return false;
 }
 #endif