Moved Init() before startup_data_->event.Signal() because derived

classes may not be safe to use until Init() has been called.  As an
example, RenderThread() creates it's IPC::SyncChannel in Init(), so it
isn't safe to call Send() method until after.

Update use of a Thread (hit with ui_tests) to no longer require Init()
be called after the startup event is signaled (required old thread
behavior).

Ran a bunch more tests on Windows to try harder to make sure I didn't break anything.
Review URL: http://codereview.chromium.org/18824

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

6 files changed, 53 insertions, 10 deletions

diff --git a/base/thread.cc b/base/thread.cc
index 9bc81a4..387b1c5 100644
--- a/base/thread.cc
+++ b/base/thread.cc
@@ -143,13 +143,14 @@ void Thread::ThreadMain() {
   message_loop.set_thread_name(name_);
   message_loop_ = &message_loop;
 
+  // Let the thread do extra initialization.
+  // Let's do this before signaling we are started.
+  Init();
+
   startup_data_->event.Signal();
   // startup_data_ can't be touched anymore since the starting thread is now
   // unlocked.
 
-  // Let the thread do extra initialization.
-  Init();
-
   message_loop.Run();
 
   // Let the thread do extra cleanup.
diff --git a/base/thread_unittest.cc b/base/thread_unittest.cc
index 085df71..f741951 100644
--- a/base/thread_unittest.cc
+++ b/base/thread_unittest.cc
@@ -37,6 +37,20 @@ class SleepSome : public Task {
   int msec_;
 };
 
+class SleepInsideInitThread : public Thread {
+ public:
+  SleepInsideInitThread() : Thread("none") { init_called_ = false; }
+  virtual ~SleepInsideInitThread() { }
+
+  virtual void Init() {
+    PlatformThread::Sleep(500);
+    init_called_ = true;
+  }
+  bool InitCalled() { return init_called_; }
+ private:
+  bool init_called_;
+};
+
 }  // namespace
 
 TEST_F(ThreadTest, Restart) {
@@ -107,3 +121,12 @@ TEST_F(ThreadTest, ThreadName) {
   EXPECT_TRUE(a.Start());
   EXPECT_EQ("ThreadName", a.thread_name());
 }
+
+// Make sure we can't use a thread between Start() and Init().
+TEST_F(ThreadTest, SleepInsideInit) {
+  SleepInsideInitThread t;
+  EXPECT_FALSE(t.InitCalled());
+  t.Start();
+  EXPECT_TRUE(t.InitCalled());
+}
+
diff --git a/webkit/default_plugin/plugin_impl.cc b/webkit/default_plugin/plugin_impl.cc
index c0f99b4..6f51008 100644
--- a/webkit/default_plugin/plugin_impl.cc
+++ b/webkit/default_plugin/plugin_impl.cc
@@ -39,12 +39,14 @@ PluginInstallerImpl::PluginInstallerImpl(int16 mode)
       underline_font_(NULL),
       tooltip_(NULL),
       activex_installer_(NULL),
+      installation_job_monitor_thread_(
+          new PluginInstallationJobMonitorThread()),
       plugin_database_handler_(*this),
       plugin_download_url_for_display_(false) {
 }
 
 PluginInstallerImpl::~PluginInstallerImpl() {
-  installation_job_monitor_thread_.Stop();
+  installation_job_monitor_thread_->Stop();
 
   if (bold_font_)
     ::DeleteObject(bold_font_);
@@ -77,7 +79,7 @@ bool PluginInstallerImpl::Initialize(HINSTANCE module_handle, NPP instance,
     return false;
   }
 
-  if (!installation_job_monitor_thread_.Initialize()) {
+  if (!installation_job_monitor_thread_->Initialize()) {
     DLOG(ERROR) << "Failed to initialize plugin install job";
     NOTREACHED();
     return false;
@@ -335,7 +337,7 @@ bool PluginInstallerImpl::SetWindow(HWND parent_window) {
   ::GetClientRect(parent_window, &parent_rect);
   Create(parent_window, parent_rect, NULL, WS_CHILD | WS_BORDER);
   DCHECK(IsWindow());
-  installation_job_monitor_thread_.set_plugin_window(m_hWnd);
+  installation_job_monitor_thread_->set_plugin_window(m_hWnd);
 
   CreateToolTip();
   UpdateToolTip();
@@ -637,7 +639,7 @@ LRESULT PluginInstallerImpl::OnCopyData(UINT message, WPARAM wparam,
     } else {
       DLOG(INFO) << "Successfully launched plugin installer";
       set_plugin_installer_state(PluginInstallerLaunchSuccess);
-      installation_job_monitor_thread_.AssignProcessToJob(
+      installation_job_monitor_thread_->AssignProcessToJob(
           shell_execute_info.hProcess);
       DisplayStatus(IDS_DEFAULT_PLUGIN_REFRESH_PLUGIN_MSG);
       enable_click_ = true;
diff --git a/webkit/default_plugin/plugin_impl.h b/webkit/default_plugin/plugin_impl.h
index 8da77cb..1e94a3a 100644
--- a/webkit/default_plugin/plugin_impl.h
+++ b/webkit/default_plugin/plugin_impl.h
@@ -350,7 +350,8 @@ class PluginInstallerImpl : public CWindowImpl<PluginInstallerImpl> {
   // which enables the downloaded plugin to be instantiated.
   // The completion events from the job are monitored in an independent
   // thread.
-  PluginInstallationJobMonitorThread installation_job_monitor_thread_;
+  scoped_refptr<PluginInstallationJobMonitorThread>
+      installation_job_monitor_thread_;
   // This object handles download and parsing of the plugins database.
   PluginDatabaseHandler plugin_database_handler_;
   // Indicates if the left click to download/refresh should be enabled or not.
diff --git a/webkit/default_plugin/plugin_install_job_monitor.cc b/webkit/default_plugin/plugin_install_job_monitor.cc
index c4de879..5851bee 100644
--- a/webkit/default_plugin/plugin_install_job_monitor.cc
+++ b/webkit/default_plugin/plugin_install_job_monitor.cc
@@ -36,6 +36,12 @@ bool PluginInstallationJobMonitorThread::Initialize() {
 }
 
 void PluginInstallationJobMonitorThread::Init() {
+  this->message_loop()->PostTask(FROM_HERE,
+    NewRunnableMethod(this,
+                      &PluginInstallationJobMonitorThread::WaitForJobThread));
+}
+
+void PluginInstallationJobMonitorThread::WaitForJobThread() {
   if (!install_job_) {
     DLOG(WARNING) << "Invalid job information";
     NOTREACHED();
diff --git a/webkit/default_plugin/plugin_install_job_monitor.h b/webkit/default_plugin/plugin_install_job_monitor.h
index 825c2e2..d86384a 100644
--- a/webkit/default_plugin/plugin_install_job_monitor.h
+++ b/webkit/default_plugin/plugin_install_job_monitor.h
@@ -9,12 +9,16 @@
 
 #include "base/logging.h"
 #include "base/thread.h"
+#include "base/ref_counted.h"
 
 // Provides the plugin installation job monitoring functionality.
 // The PluginInstallationJobMonitorThread class represents a background
 // thread which monitors the install job completion port which is associated
 // with the job when an instance of this class is initialized.
-class PluginInstallationJobMonitorThread : public base::Thread {
+class PluginInstallationJobMonitorThread :
+    public base::Thread,
+    public base::RefCountedThreadSafe<PluginInstallationJobMonitorThread> {
+
  public:
   PluginInstallationJobMonitorThread();
   virtual ~PluginInstallationJobMonitorThread();
@@ -46,10 +50,16 @@ class PluginInstallationJobMonitorThread : public base::Thread {
   bool AssignProcessToJob(HANDLE process_handle);
 
  protected:
+  // Installs a task on our thread to call WaitForJobThread().  We
+  // can't call it directly since we would deadlock (thread which
+  // creates me blocks until Start() returns, and Start() doesn't
+  // return until Init() does).
+  virtual void Init();
+
   // Blocks on the plugin installation job completion port by invoking the
   // GetQueuedCompletionStatus API.
   // We return from this function when the job monitoring thread is stopped.
-  virtual void Init();
+  virtual void WaitForJobThread();
 
  private:
   // The install job completion port. Created in Init.