Revert 207278 "Make sure that the UI window created by base::Mes..."

Speculative revert: Suspected to break NavigationControllerTest.PurgeScreenshot.

> Make sure that the UI window created by base::MessagePumpForUI is destoyed on the same thread (Windows).
> 
> Currently the window created base::MessagePumpForUI can be destroyed on a wrong thread. base::MessagePumpForUI is a ref-counted class so it can (and does) outlive the owning base::MessageLoop. As the result DestroyWindow() can be called on a wrong thread. This makes TSAN unhappy and it reports races deep unside user32.dll.
> 
> Changes in this CL:
> - The message pump is now notified when the owning message loop is being destroyed. The notification is used to free all resources that hve to be released on the base::MessageLoop's thread.
> - MessagePumpForUI::ScheduleWork() synchronizes access to the message-only window handle to avoid posting messages to the window during or after its destruction.
> 
> BUG=241939
> 
> Review URL: https://chromiumcodereview.appspot.com/15709015

TBR=alexeypa@chromium.org

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@207327 0039d316-1c4b-4281-b951-d872f2087c98

14 files changed, 13 insertions, 85 deletions

diff --git a/base/message_loop/message_loop.cc b/base/message_loop/message_loop.cc
index e443012..4e0c5f6 100644
--- a/base/message_loop/message_loop.cc
+++ b/base/message_loop/message_loop.cc
@@ -221,9 +221,6 @@ MessageLoop::~MessageLoop() {
       WillDestroyCurrentMessageLoop();
   message_loop_proxy_ = NULL;
 
-  // Stop the message pump and free any thread-bound resources.
-  pump_->Shutdown();
-
   // OK, now make it so that no one can find us.
   lazy_tls_ptr.Pointer()->Set(NULL);
 
diff --git a/base/message_loop/message_pump.h b/base/message_loop/message_pump.h
index a59d71b..5b72232 100644
--- a/base/message_loop/message_pump.h
+++ b/base/message_loop/message_pump.h
@@ -119,12 +119,6 @@ class BASE_EXPORT MessagePump : public RefCountedThreadSafe<MessagePump> {
   // used on the thread that called Run.
   virtual void ScheduleDelayedWork(const TimeTicks& delayed_work_time) = 0;
 
-  // Stops the pump after exiting the last run loop. This method must be used on
-  // the same thread that called Run(). The method frees any resources
-  // affinitized to that thread because ~MessagePump() might be invoked on an
-  // arbitrary thread.
-  virtual void Shutdown() = 0;
-
  protected:
   virtual ~MessagePump();
   friend class RefCountedThreadSafe<MessagePump>;
diff --git a/base/message_loop/message_pump_android.cc b/base/message_loop/message_pump_android.cc
index 9432c73..d7bf9a9 100644
--- a/base/message_loop/message_pump_android.cc
+++ b/base/message_loop/message_pump_android.cc
@@ -131,9 +131,6 @@ void MessagePumpForUI::ScheduleDelayedWork(const TimeTicks& delayed_work_time) {
       g_system_message_handler_obj.Get().obj(), millis);
 }
 
-void MessagePumpForUI::Shutdown() {
-}
-
 // static
 bool MessagePumpForUI::RegisterBindings(JNIEnv* env) {
   return RegisterNativesImpl(env);
diff --git a/base/message_loop/message_pump_android.h b/base/message_loop/message_pump_android.h
index 0473d3e..fd934a7 100644
--- a/base/message_loop/message_pump_android.h
+++ b/base/message_loop/message_pump_android.h
@@ -26,7 +26,6 @@ class BASE_EXPORT MessagePumpForUI : public MessagePump {
   virtual void Quit() OVERRIDE;
   virtual void ScheduleWork() OVERRIDE;
   virtual void ScheduleDelayedWork(const TimeTicks& delayed_work_time) OVERRIDE;
-  virtual void Shutdown() OVERRIDE;
 
   virtual void Start(Delegate* delegate);
 
diff --git a/base/message_loop/message_pump_default.cc b/base/message_loop/message_pump_default.cc
index 3edc859..b36ff21 100644
--- a/base/message_loop/message_pump_default.cc
+++ b/base/message_loop/message_pump_default.cc
@@ -82,7 +82,4 @@ void MessagePumpDefault::ScheduleDelayedWork(
   delayed_work_time_ = delayed_work_time;
 }
 
-void MessagePumpDefault::Shutdown() {
-}
-
 }  // namespace base
diff --git a/base/message_loop/message_pump_default.h b/base/message_loop/message_pump_default.h
index 4d5684d..dd65973 100644
--- a/base/message_loop/message_pump_default.h
+++ b/base/message_loop/message_pump_default.h
@@ -20,7 +20,6 @@ class MessagePumpDefault : public MessagePump {
   virtual void Quit() OVERRIDE;
   virtual void ScheduleWork() OVERRIDE;
   virtual void ScheduleDelayedWork(const TimeTicks& delayed_work_time) OVERRIDE;
-  virtual void Shutdown() OVERRIDE;
 
  protected:
   virtual ~MessagePumpDefault() {}
diff --git a/base/message_loop/message_pump_glib.cc b/base/message_loop/message_pump_glib.cc
index d6b3822..de012fd 100644
--- a/base/message_loop/message_pump_glib.cc
+++ b/base/message_loop/message_pump_glib.cc
@@ -320,9 +320,6 @@ void MessagePumpGlib::ScheduleDelayedWork(const TimeTicks& delayed_work_time) {
   ScheduleWork();
 }
 
-void MessagePumpGlib::Shutdown() {
-}
-
 MessagePumpGlib::~MessagePumpGlib() {
   g_source_destroy(work_source_);
   g_source_unref(work_source_);
diff --git a/base/message_loop/message_pump_glib.h b/base/message_loop/message_pump_glib.h
index fb63018..e45591b 100644
--- a/base/message_loop/message_pump_glib.h
+++ b/base/message_loop/message_pump_glib.h
@@ -62,7 +62,6 @@ class BASE_EXPORT MessagePumpGlib : public MessagePump {
   virtual void Quit() OVERRIDE;
   virtual void ScheduleWork() OVERRIDE;
   virtual void ScheduleDelayedWork(const TimeTicks& delayed_work_time) OVERRIDE;
-  virtual void Shutdown() OVERRIDE;
 
  protected:
   virtual ~MessagePumpGlib();
diff --git a/base/message_loop/message_pump_libevent.cc b/base/message_loop/message_pump_libevent.cc
index 2a4cabe..8de0db2 100644
--- a/base/message_loop/message_pump_libevent.cc
+++ b/base/message_loop/message_pump_libevent.cc
@@ -302,9 +302,6 @@ void MessagePumpLibevent::ScheduleDelayedWork(
   delayed_work_time_ = delayed_work_time;
 }
 
-void MessagePumpLibevent::Shutdown() {
-}
-
 void MessagePumpLibevent::WillProcessIOEvent() {
   FOR_EACH_OBSERVER(IOObserver, io_observers_, WillProcessIOEvent());
 }
diff --git a/base/message_loop/message_pump_libevent.h b/base/message_loop/message_pump_libevent.h
index c430b9f..a54ef88 100644
--- a/base/message_loop/message_pump_libevent.h
+++ b/base/message_loop/message_pump_libevent.h
@@ -125,7 +125,6 @@ class BASE_EXPORT MessagePumpLibevent : public MessagePump {
   virtual void Quit() OVERRIDE;
   virtual void ScheduleWork() OVERRIDE;
   virtual void ScheduleDelayedWork(const TimeTicks& delayed_work_time) OVERRIDE;
-  virtual void Shutdown() OVERRIDE;
 
  protected:
   virtual ~MessagePumpLibevent();
diff --git a/base/message_loop/message_pump_mac.h b/base/message_loop/message_pump_mac.h
index db2bf8e..68c2a3d 100644
--- a/base/message_loop/message_pump_mac.h
+++ b/base/message_loop/message_pump_mac.h
@@ -73,7 +73,6 @@ class MessagePumpCFRunLoopBase : public MessagePump {
 
   virtual void ScheduleWork() OVERRIDE;
   virtual void ScheduleDelayedWork(const TimeTicks& delayed_work_time) OVERRIDE;
-  virtual void Shutdown() OVERRIDE;
 
  protected:
   virtual ~MessagePumpCFRunLoopBase();
diff --git a/base/message_loop/message_pump_mac.mm b/base/message_loop/message_pump_mac.mm
index 0dcfc23..a419d48 100644
--- a/base/message_loop/message_pump_mac.mm
+++ b/base/message_loop/message_pump_mac.mm
@@ -207,9 +207,6 @@ void MessagePumpCFRunLoopBase::ScheduleDelayedWork(
   CFRunLoopTimerSetNextFireDate(delayed_work_timer_, delayed_work_fire_time_);
 }
 
-void MessagePumpCFRunLoopBase::Shutdown() {
-}
-
 // Called from the run loop.
 // static
 void MessagePumpCFRunLoopBase::RunDelayedWorkTimer(CFRunLoopTimerRef timer,
diff --git a/base/message_loop/message_pump_win.cc b/base/message_loop/message_pump_win.cc
index 2365494..36a3210 100644
--- a/base/message_loop/message_pump_win.cc
+++ b/base/message_loop/message_pump_win.cc
@@ -67,10 +67,6 @@ void MessagePumpWin::RunWithDispatcher(
   state_ = previous_state;
 }
 
-void MessagePumpWin::Run(Delegate* delegate) {
-  RunWithDispatcher(delegate, NULL);
-}
-
 void MessagePumpWin::Quit() {
   DCHECK(state_);
   state_->should_quit = true;
@@ -107,32 +103,20 @@ MessagePumpForUI::MessagePumpForUI()
 }
 
 MessagePumpForUI::~MessagePumpForUI() {
-  DCHECK(!atom_);
-  DCHECK(!message_hwnd_);
+  DestroyWindow(message_hwnd_);
+  UnregisterClass(MAKEINTATOM(atom_),
+                  GetModuleFromAddress(&WndProcThunk));
 }
 
 void MessagePumpForUI::ScheduleWork() {
   if (InterlockedExchange(&have_work_, 1))
     return;  // Someone else continued the pumping.
 
-  // If the attempt to acquire the lock fails, the message loop is shutting down
-  // and the message window is about to be destroyed as well.
-  if (!message_hwnd_lock_.Try())
-    return;
-
-  {
-    base::AutoLock lock(message_hwnd_lock_, base::AutoLock::AlreadyAcquired());
-
-    // Do nothing if the window has been destroyed already.
-    if (!message_hwnd_)
-      return;
-
-    // Make sure the MessagePump does some work for us.
-    if (PostMessage(message_hwnd_, kMsgHaveWork,
-                    reinterpret_cast<WPARAM>(this), 0)) {
-      return;
-    }
-  }
+  // Make sure the MessagePump does some work for us.
+  BOOL ret = PostMessage(message_hwnd_, kMsgHaveWork,
+                         reinterpret_cast<WPARAM>(this), 0);
+  if (ret)
+    return;  // There was room in the Window Message queue.
 
   // We have failed to insert a have-work message, so there is a chance that we
   // will starve tasks/timers while sitting in a nested message loop.  Nested
@@ -188,24 +172,6 @@ void MessagePumpForUI::ScheduleDelayedWork(const TimeTicks& delayed_work_time) {
                             MESSAGE_LOOP_PROBLEM_MAX);
 }
 
-void MessagePumpForUI::Shutdown() {
-  HWND message_hwnd;
-  {
-    base::AutoLock lock(message_hwnd_lock_);
-
-    // Let ScheduleWork() know that the window has been destoyed.
-    message_hwnd = message_hwnd_;
-    message_hwnd_ = NULL;
-  }
-
-  // Destoy the window and its class. The destructor checks whether
-  // |message_hwnd_| and |atom_| are destroyed, so the variables should be
-  // cleared here.
-  DestroyWindow(message_hwnd);
-  UnregisterClass(MAKEINTATOM(atom_), GetModuleFromAddress(&WndProcThunk));
-  atom_ = 0;
-}
-
 void MessagePumpForUI::PumpOutPendingPaintMessages() {
   // If we are being called outside of the context of Run, then don't try to do
   // any work.
@@ -512,7 +478,7 @@ void MessagePumpForIO::ScheduleWork() {
     return;  // Post worked perfectly.
 
   // See comment in MessagePumpForUI::ScheduleWork() for this error recovery.
-  InterlockedExchange(&have_work_, 0);
+  InterlockedExchange(&have_work_, 0);  // Clarify that we didn't succeed.
   UMA_HISTOGRAM_ENUMERATION("Chrome.MessageLoopProblem", COMPLETION_POST_ERROR,
                             MESSAGE_LOOP_PROBLEM_MAX);
 }
@@ -524,9 +490,6 @@ void MessagePumpForIO::ScheduleDelayedWork(const TimeTicks& delayed_work_time) {
   delayed_work_time_ = delayed_work_time;
 }
 
-void MessagePumpForIO::Shutdown() {
-}
-
 void MessagePumpForIO::RegisterIOHandler(HANDLE file_handle,
                                          IOHandler* handler) {
   ULONG_PTR key = HandlerToKey(handler, true);
diff --git a/base/message_loop/message_pump_win.h b/base/message_loop/message_pump_win.h
index b492678..dbb41bd 100644
--- a/base/message_loop/message_pump_win.h
+++ b/base/message_loop/message_pump_win.h
@@ -16,7 +16,6 @@
 #include "base/message_loop/message_pump_dispatcher.h"
 #include "base/message_loop/message_pump_observer.h"
 #include "base/observer_list.h"
-#include "base/synchronization/lock.h"
 #include "base/time.h"
 #include "base/win/scoped_handle.h"
 
@@ -46,8 +45,8 @@ class BASE_EXPORT MessagePumpWin : public MessagePump {
   void RunWithDispatcher(Delegate* delegate, MessagePumpDispatcher* dispatcher);
 
   // MessagePump methods:
-  virtual void Run(Delegate* delegate) OVERRIDE;
-  virtual void Quit() OVERRIDE;
+  virtual void Run(Delegate* delegate) { RunWithDispatcher(delegate, NULL); }
+  virtual void Quit();
 
  protected:
   struct RunState {
@@ -167,9 +166,8 @@ class BASE_EXPORT MessagePumpForUI : public MessagePumpWin {
   void SetMessageFilter(scoped_ptr<MessageFilter> message_filter);
 
   // MessagePump methods:
-  virtual void ScheduleWork() OVERRIDE;
-  virtual void ScheduleDelayedWork(const TimeTicks& delayed_work_time) OVERRIDE;
-  virtual void Shutdown() OVERRIDE;
+  virtual void ScheduleWork();
+  virtual void ScheduleDelayedWork(const TimeTicks& delayed_work_time);
 
   // Applications can call this to encourage us to process all pending WM_PAINT
   // messages.  This method will process all paint messages the Windows Message
@@ -196,9 +194,6 @@ class BASE_EXPORT MessagePumpForUI : public MessagePumpWin {
   // A hidden message-only window.
   HWND message_hwnd_;
 
-  // Protectes access to |message_hwnd_|.
-  base::Lock message_hwnd_lock_;
-
   scoped_ptr<MessageFilter> message_filter_;
 };
 
@@ -332,7 +327,6 @@ class BASE_EXPORT MessagePumpForIO : public MessagePumpWin {
   // MessagePump methods:
   virtual void ScheduleWork();
   virtual void ScheduleDelayedWork(const TimeTicks& delayed_work_time);
-  virtual void Shutdown() OVERRIDE;
 
   // Register the handler to be used when asynchronous IO for the given file
   // completes. The registration persists as long as |file_handle| is valid, so