Support re-entrant calls to MessageQueueManager::Clear.

webrtc/rtc_base/messagequeue_unittest.cc

 /*
  *  Copyright 2004 The WebRTC Project Authors. All rights reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
 #include "webrtc/rtc_base/messagequeue.h"
 
 #include <functional>
 
 #include "webrtc/rtc_base/atomicops.h"
 #include "webrtc/rtc_base/bind.h"
 #include "webrtc/rtc_base/event.h"
 #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"
 
 using namespace rtc;
 
 class MessageQueueTest: public testing::Test, public MessageQueue {
  public:
   MessageQueueTest() : MessageQueue(SocketServer::CreateDefault(), true) {}
   bool IsLocked_Worker() {
     if (!crit_.TryEnter()) {
@@ skipping 177 matching lines @@
   FunctorMessageHandler<void, std::function<void()>> event_signaler(
       [&entered_process_all_message_queues] {
         entered_process_all_message_queues.Set();
       });
 
   // Post messages (both delayed and non delayed) to both threads.
   t.Post(RTC_FROM_HERE, &clearer);
   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>()));
+}