Fixing a couple cases that cause ProcessAllMessageQueues to hang.

webrtc/base/messagequeue.cc

Index: webrtc/base/messagequeue.cc
diff --git a/webrtc/base/messagequeue.cc b/webrtc/base/messagequeue.cc
index ebf98f58a34e3539a85bb4068bddadf8eab9c678..e80c12fab7b8807403541c3b75b687ceb0ed0865 100644
--- a/webrtc/base/messagequeue.cc
+++ b/webrtc/base/messagequeue.cc
@@ -126,17 +126,34 @@ void MessageQueueManager::ProcessAllMessageQueues() {
 }
 
 void MessageQueueManager::ProcessAllMessageQueuesInternal() {
-  // Post a delayed message at the current time and wait for it to be dispatched
-  // on all queues, which will ensure that all messages that came before it were
-  // also dispatched.
-  volatile int queues_not_done;
-  auto functor = [&queues_not_done] { AtomicOps::Decrement(&queues_not_done); };
-  FunctorMessageHandler<void, decltype(functor)> handler(functor);
+  // This works by posting a delayed message at the current time and waiting
+  // for it to be dispatched on all queues, which will ensure that all messages
+  // that came before it were also dispatched.
+  volatile int queues_not_done = 0;
+
+  // This class is used so that whether the posted message is processed, or the
+  // message queue is simply cleared, queues_not_done gets decremented.
+  class AutoDecrementer : public MessageData {

[honghaiz3, 2016/09/09 17:59:02]

Why do you replace the original Message handler with a new NoOpHandler and an
AutoDecrementer? 
You replaced two lines of code with two Classes. 
The NoOphandler is not really no op. You just hide the op in the Message
Constructor/Destructor. 

[Taylor Brandstetter, 2016/09/09 23:33:12]

I tried to explain in the comment above. It's needed for the case where the
message queue is cleared (typically due to the thread being destroyed) before
the posted message is processed. This will delete the MessageData, ultimately
decrementing the count, even though the message handler isn't invoked.

As far as the name "NoOpHandler": a MessageHandler is required to delete the
MessageData to avoid memory leaks. So I guess it's not a "no op", but it's the
smallest set of necessary operations. I'm open to suggestions on a name.

+   public:
+    AutoDecrementer(volatile int* value) : value_(value) {}

[pthatcher1, 2016/09/09 17:11:34]

Why not also have the class increment in the constructor?   Then it becomes kind
of like unique_ptr/scoped_ptr, but becomes something like ScopedIncrement.  

[Taylor Brandstetter, 2016/09/09 17:28:32]

Sure, might as well.

+    ~AutoDecrementer() override { AtomicOps::Decrement(value_); }
+
+   private:
+    volatile int* value_;
+  };
+
+  NoOpMessageHandler handler;
   {
     DebugNonReentrantCritScope cs(&crit_, &locked_);
-    queues_not_done = static_cast<int>(message_queues_.size());
     for (MessageQueue* queue : message_queues_) {
-      queue->PostDelayed(RTC_FROM_HERE, 0, &handler);
+      if (queue->IsQuitting()) {
+        // If the queue is quitting, it's done processing messages so it can
+        // be ignored. If we tried to post a message to it, it would be dropped.
+        continue;
+      }
+      AtomicOps::Increment(&queues_not_done);
+      queue->PostDelayed(RTC_FROM_HERE, 0, &handler, 0,
+                         new AutoDecrementer(&queues_not_done));
     }
   }
   // Note: One of the message queues may have been on this thread, which is why