Porting in chrome/common/

Review URL: http://codereview.chromium.org/17604

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

8 files changed, 191 insertions, 93 deletions

diff --git a/chrome/common/chrome_plugin_lib.h b/chrome/common/chrome_plugin_lib.h
index 8cdfa70..1609bad 100644
--- a/chrome/common/chrome_plugin_lib.h
+++ b/chrome/common/chrome_plugin_lib.h
@@ -57,8 +57,10 @@ class ChromePluginLib : public base::RefCounted<ChromePluginLib>  {
   // Method to call a test function in the plugin, used for unit tests.
   int CP_Test(void* param);
 
-  // The registroy path to search for Chrome Plugins/
+#if defined(OS_WIN)
+  // The registry path to search for Chrome Plugins/
   static const TCHAR kRegistryChromePlugins[];
+#endif  // defined(OS_WIN)
 
  private:
   friend class base::RefCounted<ChromePluginLib>;
@@ -81,7 +83,10 @@ class ChromePluginLib : public base::RefCounted<ChromePluginLib>  {
   void Unload();
 
   FilePath filename_;  // the path to the plugin
+#if defined(OS_WIN)
+  // TODO(port): Remove ifdefs when we have portable replacement for HMODULE.
   HMODULE module_;  // the opened plugin handle
+#endif  // defined(OS_WIN)
   bool initialized_;  // is the plugin initialized
 
   // Exported symbols from the plugin, looked up by name.
diff --git a/chrome/common/common.scons b/chrome/common/common.scons
index 34c5a05..86255d8 100644
--- a/chrome/common/common.scons
+++ b/chrome/common/common.scons
@@ -63,6 +63,7 @@ input_files.extend([
   'logging_chrome.cc',
   'message_router.cc',
   'net/cookie_monster_sqlite.cc',
+  'net/url_request_intercept_job.cc',
   'notification_registrar.cc',
   'notification_service.cc',
   'pref_member.cc',
@@ -82,6 +83,7 @@ input_files.extend([
   'thumbnail_score.cc',
   'unzip.cc',
   'visitedlink_common.cc',
+  'worker_thread_ticker.cc',
 ])
 
 if env.Bit('windows'):
@@ -91,9 +93,7 @@ if env.Bit('windows'):
       'chrome_plugin_lib.cc',
       'chrome_plugin_util.cc',
       'chrome_process_filter.cc',
-      'classfactory.cc',
       'drag_drop_types.cc',
-      'gfx/chrome_canvas_win.cc',
       'gfx/emf.cc',
       'gfx/icon_util.cc',
       'gfx/path.cc',
@@ -101,19 +101,19 @@ if env.Bit('windows'):
       'ipc_logging.cc',
       'ipc_sync_channel.cc',
       'jstemplate_builder.cc',
-      'net/url_request_intercept_job.cc',
       'os_exchange_data.cc',
       'plugin_messages.cc',
       'process_watcher.cc',
       'security_filter_peer.cc',
       'win_safe_util.cc',
       'win_util.cc',
-      'worker_thread_ticker.cc',
   ])
 
 if env.Bit('windows'):
   # Windows specific files
   input_files.extend([
+      'classfactory.cc',
+      'gfx/chrome_canvas_win.cc',
       'gfx/chrome_font_win.cc',
       'ipc_channel_win.cc',
       'resource_bundle_win.cc',
diff --git a/chrome/common/net/url_request_intercept_job.cc b/chrome/common/net/url_request_intercept_job.cc
index 398f2b0..b11fbb2 100644
--- a/chrome/common/net/url_request_intercept_job.cc
+++ b/chrome/common/net/url_request_intercept_job.cc
@@ -26,8 +26,8 @@ URLRequestInterceptJob::URLRequestInterceptJob(URLRequest* request,
     : URLRequestJob(request),
       cprequest_(cprequest),
       plugin_(plugin),
-      read_buffer_(NULL),
-      got_headers_(false) {
+      got_headers_(false),
+      read_buffer_(NULL) {
   cprequest_->data = this;  // see FromCPRequest().
 
   NotificationService::current()->AddObserver(
diff --git a/chrome/common/worker_thread_ticker.cc b/chrome/common/worker_thread_ticker.cc
index 5d57206..3cc877b 100644
--- a/chrome/common/worker_thread_ticker.cc
+++ b/chrome/common/worker_thread_ticker.cc
@@ -2,13 +2,40 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "chrome/common/worker_thread_ticker.h"
+
 #include <algorithm>
+
 #include "base/logging.h"
-#include "chrome/common/worker_thread_ticker.h"
+#include "base/task.h"
+#include "base/time.h"
+#include "base/thread.h"
+
+class WorkerThreadTicker::TimerTask : public Task {
+ public:
+  TimerTask(WorkerThreadTicker* ticker) : ticker_(ticker) {
+  }
+
+  virtual void Run() {
+    // When the ticker is running, the handler list CANNOT be modified.
+    // So we can do the enumeration safely without a lock
+    TickHandlerListType* handlers = &ticker_->tick_handler_list_;
+    for (TickHandlerListType::const_iterator i = handlers->begin();
+         i != handlers->end(); ++i) {
+      (*i)->OnTick();
+    }
+
+    ticker_->ScheduleTimerTask();
+  }
+
+ private:
+  WorkerThreadTicker* ticker_;
+};
 
 WorkerThreadTicker::WorkerThreadTicker(int tick_interval)
-    : tick_interval_(tick_interval),
-      wait_handle_(NULL) {
+    : timer_thread_("worker_thread_ticker"),
+      is_running_(false),
+      tick_interval_(tick_interval) {
 }
 
 WorkerThreadTicker::~WorkerThreadTicker() {
@@ -17,19 +44,18 @@ WorkerThreadTicker::~WorkerThreadTicker() {
 
 bool WorkerThreadTicker::RegisterTickHandler(Callback *tick_handler) {
   DCHECK(tick_handler);
-  AutoLock lock(tick_handler_list_lock_);
+  AutoLock lock(lock_);
   // You cannot change the list of handlers when the timer is running.
   // You need to call Stop first.
-  if (IsRunning()) {
+  if (IsRunning())
     return false;
-  }
   tick_handler_list_.push_back(tick_handler);
   return true;
 }
 
 bool WorkerThreadTicker::UnregisterTickHandler(Callback *tick_handler) {
   DCHECK(tick_handler);
-  AutoLock lock(tick_handler_list_lock_);
+  AutoLock lock(lock_);
   // You cannot change the list of handlers when the timer is running.
   // You need to call Stop first.
   if (IsRunning()) {
@@ -48,69 +74,27 @@ bool WorkerThreadTicker::UnregisterTickHandler(Callback *tick_handler) {
 bool WorkerThreadTicker::Start() {
   // Do this in a lock because we don't want 2 threads to
   // call Start at the same time
-  AutoLock lock(tick_handler_list_lock_);
-  if (IsRunning()) {
+  AutoLock lock(lock_);
+  if (IsRunning())
     return false;
-  }
-  bool ret = false;
-  HANDLE event = CreateEvent(NULL, FALSE, FALSE, NULL);
-  if (!event) {
-    NOTREACHED();
-  } else {
-    if (!RegisterWaitForSingleObject(
-            &wait_handle_,
-            event,
-            reinterpret_cast<WAITORTIMERCALLBACK>(TickCallback),
-            this,
-            tick_interval_,
-            WT_EXECUTEDEFAULT)) {
-      NOTREACHED();
-      CloseHandle(event);
-    } else {
-      dummy_event_.Attach(event);
-      ret = true;
-    }
-  }
-  return ret;
+  is_running_ = true;
+  timer_thread_.Start();
+  ScheduleTimerTask();
+  return true;
 }
 
 bool WorkerThreadTicker::Stop() {
   // Do this in a lock because we don't want 2 threads to
   // call Stop at the same time
-  AutoLock lock(tick_handler_list_lock_);
-  if (!IsRunning()) {
+  AutoLock lock(lock_);
+  if (!IsRunning())
     return false;
-  }
-  DCHECK(wait_handle_);
-
-  // Wait for the callbacks to be done. Passing
-  // INVALID_HANDLE_VALUE to UnregisterWaitEx achieves this.
-  UnregisterWaitEx(wait_handle_, INVALID_HANDLE_VALUE);
-  wait_handle_ = NULL;
-  dummy_event_.Close();
+  is_running_ = false;
+  timer_thread_.Stop();
   return true;
 }
 
-bool WorkerThreadTicker::IsRunning() const {
-  return (wait_handle_ != NULL);
-}
-
-void WorkerThreadTicker::InvokeHandlers() {
-  // When the ticker is running, the handler list CANNOT be modified.
-  // So we can do the enumeration safely without a lock
-  TickHandlerListType::iterator index = tick_handler_list_.begin();
-  while (index != tick_handler_list_.end()) {
-    (*index)->OnTick();
-    index++;
-  }
+void WorkerThreadTicker::ScheduleTimerTask() {
+  timer_thread_.message_loop()->PostDelayedTask(FROM_HERE, new TimerTask(this),
+                                                tick_interval_);
 }
-
-void CALLBACK WorkerThreadTicker::TickCallback(WorkerThreadTicker* this_ticker,
-                                               BOOLEAN timer_or_wait_fired) {
-  DCHECK(NULL != this_ticker);
-  if (NULL != this_ticker) {
-    this_ticker->InvokeHandlers();
-  }
-}
-
-
diff --git a/chrome/common/worker_thread_ticker.h b/chrome/common/worker_thread_ticker.h
index b3ae8a7..0e200a9 100644
--- a/chrome/common/worker_thread_ticker.h
+++ b/chrome/common/worker_thread_ticker.h
@@ -5,20 +5,17 @@
 #ifndef CHROME_COMMON_WORKER_THREAD_TICKER_H__
 #define CHROME_COMMON_WORKER_THREAD_TICKER_H__
 
-#include <windows.h>
-#include <atlbase.h>
 #include <vector>
 
 #include "base/lock.h"
+#include "base/thread.h"
 
-// This class provides the follwoing functionality:
+// This class provides the following functionality:
 // It invokes a set of registered handlers at periodic intervals in
 // the context of an arbitrary worker thread.
-// This functionality is similar to a waitable timer except that the
-// timer in this case is a low-resolution timer (millisecond granularity)
-// and it does not require the caller to be in an alertable wait state.
-// The callbacks are invoked in the context of an arbitrary worker thread
-// from the system thread pool
+// The timer runs on a separate thread, so it will run even if the current
+// thread is hung. Similarly, the callbacks will be called on a separate
+// thread so they won't block the main thread.
 class WorkerThreadTicker {
  public:
   // This callback interface to be implemented by clients of this
@@ -30,7 +27,7 @@ class WorkerThreadTicker {
   };
 
   // tick_interval is the periodic interval in which to invoke the
-  // registered handlers
+  // registered handlers (in milliseconds)
   explicit WorkerThreadTicker(int tick_interval);
 
   ~WorkerThreadTicker();
@@ -52,7 +49,9 @@ class WorkerThreadTicker {
   // done because this is inherently risky.
   // Returns false is the ticker is not running
   bool Stop();
-  bool IsRunning() const;
+  bool IsRunning() const {
+    return is_running_;
+  }
 
   void set_tick_interval(int tick_interval) {
     tick_interval_ = tick_interval;
@@ -63,30 +62,26 @@ class WorkerThreadTicker {
   }
 
  private:
+  class TimerTask;
+
+  void ScheduleTimerTask();
+
   // A list type that holds all registered callback interfaces
   typedef std::vector<Callback*> TickHandlerListType;
 
-  // This is the callback function registered with the
-  // RegisterWaitForSingleObject API. It gets invoked in a system worker thread
-  // periodically at intervals of tick_interval_ miliiseconds
-  static void CALLBACK TickCallback(WorkerThreadTicker* this_ticker,
-                                    BOOLEAN timer_or_wait_fired);
+  // Lock to protect is_running_ and tick_handler_list_
+  Lock lock_;
 
-  // Helper that invokes all registered handlers
-  void InvokeHandlers();
+  base::Thread timer_thread_;
+  bool is_running_;
 
-  // A dummy event to be used by the RegisterWaitForSingleObject API
-  CHandle dummy_event_;
-  // The wait handle returned by the RegisterWaitForSingleObject API
-  HANDLE wait_handle_;
   // The interval at which the callbacks are to be invoked
   int tick_interval_;
-  // Lock for the tick_handler_list_ list
-  Lock tick_handler_list_lock_;
+
   // A list that holds all registered callback interfaces
   TickHandlerListType tick_handler_list_;
 
-  DISALLOW_EVIL_CONSTRUCTORS(WorkerThreadTicker);
+  DISALLOW_COPY_AND_ASSIGN(WorkerThreadTicker);
 };
 
 #endif  // CHROME_COMMON_WORKER_THREAD_TICKER_H__
diff --git a/chrome/common/worker_thread_ticker_unittest.cc b/chrome/common/worker_thread_ticker_unittest.cc
new file mode 100644
index 0000000..c6d4db2
--- /dev/null
+++ b/chrome/common/worker_thread_ticker_unittest.cc
@@ -0,0 +1,109 @@
+// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/common/worker_thread_ticker.h"
+
+#include "base/logging.h"
+#include "base/message_loop.h"
+#include "base/platform_thread.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace {
+
+class TestCallback : public WorkerThreadTicker::Callback {
+ public:
+  TestCallback() : counter_(0), message_loop_(MessageLoop::current()) {
+  }
+
+  virtual void OnTick() {
+    counter_++;
+
+    // Finish the test faster.
+    message_loop_->PostTask(FROM_HERE, new MessageLoop::QuitTask());
+  }
+
+  int counter() const { return counter_; }
+
+ private:
+  int counter_;
+  MessageLoop* message_loop_;
+};
+
+class LongCallback : public WorkerThreadTicker::Callback {
+ public:
+  virtual void OnTick() {
+    PlatformThread::Sleep(1500);
+  }
+};
+
+void RunMessageLoopForAWhile() {
+  MessageLoop::current()->PostDelayedTask(FROM_HERE,
+                                          new MessageLoop::QuitTask(), 500);
+  MessageLoop::current()->Run();
+}
+
+}  // namespace
+
+TEST(WorkerThreadTickerTest, Basic) {
+  MessageLoop loop;
+
+  WorkerThreadTicker ticker(50);
+  TestCallback callback;
+  EXPECT_FALSE(ticker.IsRunning());
+  EXPECT_TRUE(ticker.RegisterTickHandler(&callback));
+  EXPECT_TRUE(ticker.UnregisterTickHandler(&callback));
+  EXPECT_TRUE(ticker.Start());
+  EXPECT_FALSE(ticker.RegisterTickHandler(&callback));
+  EXPECT_FALSE(ticker.UnregisterTickHandler(&callback));
+  EXPECT_TRUE(ticker.IsRunning());
+  EXPECT_FALSE(ticker.Start());  // Can't start when it is running.
+  EXPECT_TRUE(ticker.Stop());
+  EXPECT_FALSE(ticker.IsRunning());
+  EXPECT_FALSE(ticker.Stop());  // Can't stop when it isn't running.
+}
+
+TEST(WorkerThreadTickerTest, Callback) {
+  MessageLoop loop;
+
+  WorkerThreadTicker ticker(50);
+  TestCallback callback;
+  ASSERT_TRUE(ticker.RegisterTickHandler(&callback));
+
+  ASSERT_TRUE(ticker.Start());
+  RunMessageLoopForAWhile();
+  EXPECT_TRUE(callback.counter() > 0);
+
+  ASSERT_TRUE(ticker.Stop());
+  const int counter_value = callback.counter();
+  RunMessageLoopForAWhile();
+  EXPECT_EQ(counter_value, callback.counter());
+}
+
+TEST(WorkerThreadTickerTest, OutOfScope) {
+  MessageLoop loop;
+
+  TestCallback callback;
+  {
+    WorkerThreadTicker ticker(50);
+    ASSERT_TRUE(ticker.RegisterTickHandler(&callback));
+
+    ASSERT_TRUE(ticker.Start());
+    RunMessageLoopForAWhile();
+    EXPECT_TRUE(callback.counter() > 0);
+  }
+  const int counter_value = callback.counter();
+  RunMessageLoopForAWhile();
+  EXPECT_EQ(counter_value, callback.counter());
+}
+
+TEST(WorkerThreadTickerTest, LongCallback) {
+  MessageLoop loop;
+
+  WorkerThreadTicker ticker(50);
+  LongCallback callback;
+  ASSERT_TRUE(ticker.RegisterTickHandler(&callback));
+
+  ASSERT_TRUE(ticker.Start());
+  RunMessageLoopForAWhile();
+}
diff --git a/chrome/test/unit/unit_tests.scons b/chrome/test/unit/unit_tests.scons
index 011bfc2..ed28d02 100644
--- a/chrome/test/unit/unit_tests.scons
+++ b/chrome/test/unit/unit_tests.scons
@@ -152,6 +152,7 @@ if not env.Bit('mac'):
       '$CHROME_DIR/common/mru_cache_unittest.cc',
       '$CHROME_DIR/common/notification_service_unittest.cc',
       '$CHROME_DIR/common/pref_member_unittest.cc',
+      '$CHROME_DIR/common/worker_thread_ticker_unittest.cc',
       '$CHROME_DIR/renderer/net/render_dns_queue_unittest.cc',
       '$CHROME_DIR/test/test_notification_tracker.cc',
 
diff --git a/chrome/test/unit/unittests.vcproj b/chrome/test/unit/unittests.vcproj
index 502117f..4a0194f 100644
--- a/chrome/test/unit/unittests.vcproj
+++ b/chrome/test/unit/unittests.vcproj
@@ -798,6 +798,10 @@
 			RelativePath="..\..\browser\window_sizer_unittest.cc"
 			>
 		</File>
+    <File
+      RelativePath="..\..\common\worker_thread_ticker_unittest.cc"
+      >
+    </File>
 	</Files>
 	<Globals>
 	</Globals>