base: Fix DCHECK when thread creation fails

When thread creation fails[1], base::Thread tries to delete an unbound
MessageLoop on the originating thread. If that thread already has a
MessageLoop, this trips a DCHECK in MessageLoop's destructor that checks
against having two MessageLoops on the same thread.

This patch adjusts the DCHECK logic to ignore destroying unbound message
loops on a thread that already has a separate message loop (which it was
already trying to do according to the comments).

[1] https://code.google.com/p/chromium/codesearch#chromium/src/base/threading/thread.cc&l=116

BUG=480100

Review URL: https://codereview.chromium.org/1559353002

Cr-Commit-Position: refs/heads/master@{#368057}

3 files changed, 20 insertions, 4 deletions

diff --git a/base/message_loop/message_loop.cc b/base/message_loop/message_loop.cc
index cfa86d9..13bc854 100644
--- a/base/message_loop/message_loop.cc
+++ b/base/message_loop/message_loop.cc
@@ -133,9 +133,11 @@ MessageLoop::MessageLoop(scoped_ptr<MessagePump> pump)
 }
 
 MessageLoop::~MessageLoop() {
-  // current() could be NULL if this message loop is destructed before it is
-  // bound to a thread.
-  DCHECK(current() == this || !current());
+  // If |pump_| is non-null, this message loop has been bound and should be the
+  // current one on this thread. Otherwise, this loop is being destructed before
+  // it was bound to a thread, so a different message loop (or no loop at all)
+  // may be current.
+  DCHECK((pump_ && current() == this) || (!pump_ && current() != this));
 
   // iOS just attaches to the loop, it doesn't Run it.
   // TODO(stuartmorgan): Consider wiring up a Detach().
@@ -177,7 +179,8 @@ MessageLoop::~MessageLoop() {
   task_runner_ = NULL;
 
   // OK, now make it so that no one can find us.
-  lazy_tls_ptr.Pointer()->Set(NULL);
+  if (current() == this)
+    lazy_tls_ptr.Pointer()->Set(nullptr);
 }
 
 // static
diff --git a/base/message_loop/message_loop.h b/base/message_loop/message_loop.h
index 364d04d..8ef8d6a 100644
--- a/base/message_loop/message_loop.h
+++ b/base/message_loop/message_loop.h
@@ -11,6 +11,7 @@
 #include "base/base_export.h"
 #include "base/callback_forward.h"
 #include "base/debug/task_annotator.h"
+#include "base/gtest_prod_util.h"
 #include "base/location.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
@@ -403,6 +404,7 @@ class BASE_EXPORT MessageLoop : public MessagePump::Delegate {
   friend class internal::IncomingTaskQueue;
   friend class ScheduleWorkTest;
   friend class Thread;
+  FRIEND_TEST_ALL_PREFIXES(MessageLoopTest, DeleteUnboundLoop);
 
   using MessagePumpFactoryCallback = Callback<scoped_ptr<MessagePump>()>;
 
diff --git a/base/message_loop/message_loop_unittest.cc b/base/message_loop/message_loop_unittest.cc
index 8b7a84d..1a3a925 100644
--- a/base/message_loop/message_loop_unittest.cc
+++ b/base/message_loop/message_loop_unittest.cc
@@ -1041,4 +1041,15 @@ TEST(MessageLoopTest, OriginalRunnerWorks) {
   EXPECT_EQ(1, foo->test_count());
 }
 
+TEST(MessageLoopTest, DeleteUnboundLoop) {
+  // It should be possible to delete an unbound message loop on a thread which
+  // already has another active loop. This happens when thread creation fails.
+  MessageLoop loop;
+  scoped_ptr<MessageLoop> unbound_loop(MessageLoop::CreateUnbound(
+      MessageLoop::TYPE_DEFAULT, MessageLoop::MessagePumpFactoryCallback()));
+  unbound_loop.reset();
+  EXPECT_EQ(&loop, MessageLoop::current());
+  EXPECT_EQ(loop.task_runner(), ThreadTaskRunnerHandle::Get());
+}
+
 }  // namespace base