Support re-entrant calls to MessageQueueManager::Clear.

webrtc/rtc_base/messagequeue_unittest.cc

Index: webrtc/rtc_base/messagequeue_unittest.cc
diff --git a/webrtc/rtc_base/messagequeue_unittest.cc b/webrtc/rtc_base/messagequeue_unittest.cc
index 78bcfcbea9abbf6070285dde1dc73d9f81cb2ccd..679f95ca8a6f95598b1059b1f45d03a8651aba0b 100644
--- a/webrtc/rtc_base/messagequeue_unittest.cc
+++ b/webrtc/rtc_base/messagequeue_unittest.cc
@@ -18,6 +18,8 @@
 #include "webrtc/rtc_base/gunit.h"
 #include "webrtc/rtc_base/logging.h"
 #include "webrtc/rtc_base/nullsocketserver.h"
+#include "webrtc/rtc_base/refcount.h"
+#include "webrtc/rtc_base/refcountedobject.h"
 #include "webrtc/rtc_base/thread.h"
 #include "webrtc/rtc_base/timeutils.h"
 
@@ -215,3 +217,23 @@ TEST(MessageQueueManager, ProcessAllMessageQueuesWithClearedQueue) {
   rtc::Thread::Current()->Post(RTC_FROM_HERE, &event_signaler);
   MessageQueueManager::ProcessAllMessageQueues();
 }
+
+class RefCountedHandler
+  : public MessageHandler,
+    public rtc::RefCountInterface {
+ public:
+  void OnMessage(Message* msg) override {}
+};
+
+class EmptyHandler : public MessageHandler {
+ public:
+  void OnMessage(Message* msg) override {}
+};
+
+TEST(MessageQueueManager, ClearReentrant) {

[Taylor Brandstetter, 2017/07/05 22:41:48]

Can you leave a comment summarizing how this test works? For example, "When the
empty handler is destroyed, it will clear messages queued for itself. The
message to be cleared itself wraps a MessageHandler object (RefCountedHandler),
so this will cause the message queue to be cleared again in a re-entrant
fashion, which previously triggered a DCHECK."

Also, this test would be more robust if it posted a message to both handlers
(maybe to the inner one first, then outer). Right now I think we're safe,
because the message list is an std::list (which doesn't invalidate iterators
when things are removed). But we should have a test that would break if someone
were to change it to an std::vector without considering this use case.

[joachim, 2017/07/05 23:03:11]

Done.
I don't think this is necessary. The list of message queues that is processed
doesn't change when the handler objects are destroyed - the only queue is the
Thread.

[Taylor Brandstetter, 2017/07/06 15:51:09]

That's what I'm talking about. The thread's message list gets modified while
it's being iterated.

[joachim, 2017/07/06 16:54:57]

Ah sorry, that was a misunderstanding then - updated the test.

+  Thread t;
+  EmptyHandler handler;
+  t.Post(RTC_FROM_HERE, &handler, 0,
+      new ScopedRefMessageData<RefCountedHandler>(
+          new rtc::RefCountedObject<RefCountedHandler>()));
+}