Add GamepadService, owns 1 gamepad bg thread

content/browser/gamepad/gamepad_provider.cc

Index: content/browser/gamepad/gamepad_provider.cc
diff --git a/content/browser/gamepad/gamepad_provider.cc b/content/browser/gamepad/gamepad_provider.cc
index c8eed65a30f00b447a888bfa75eedf7932c9f521..80ca1a6f4636f60cf6492d251f2994fa0bb0b563 100644
--- a/content/browser/gamepad/gamepad_provider.cc
+++ b/content/browser/gamepad/gamepad_provider.cc
@@ -9,6 +9,7 @@
 #include "base/bind.h"
 #include "base/logging.h"
 #include "base/message_loop.h"
+#include "base/synchronization/lock.h"

[jam, 2011/12/01 20:07:31]

nit: not needed since in the header

[scottmg, 2011/12/01 22:05:32]

Done.

 #include "base/task.h"
 #include "base/threading/thread.h"
 #include "base/threading/thread_restrictions.h"
@@ -23,8 +24,6 @@
 
 namespace content {
 
-GamepadProvider* GamepadProvider::instance_ = NULL;
-
 // Define the default data fetcher that GamepadProvider will use if none is
 // supplied. (GamepadPlatformDataFetcher).
 #if defined(OS_WIN)
@@ -44,9 +43,10 @@ typedef GamepadEmptyDataFetcher GamepadPlatformDataFetcher;
 #endif
 
 GamepadProvider::GamepadProvider(GamepadDataFetcher* fetcher)
-    : creator_loop_(MessageLoop::current()->message_loop_proxy()),
-      provided_fetcher_(fetcher),
+    : is_paused_(false),
       devices_changed_(true),
+      creator_loop_(MessageLoop::current()->message_loop_proxy()),
+      provided_fetcher_(fetcher),
       ALLOW_THIS_IN_INITIALIZER_LIST(weak_factory_(this)) {
   size_t data_size = sizeof(GamepadHardwareBuffer);
   base::SystemMonitor* monitor = base::SystemMonitor::Get();
@@ -55,13 +55,29 @@ GamepadProvider::GamepadProvider(GamepadDataFetcher* fetcher)
   gamepad_shared_memory_.CreateAndMapAnonymous(data_size);
   GamepadHardwareBuffer* hwbuf = SharedMemoryAsHardwareBuffer();
   memset(hwbuf, 0, sizeof(GamepadHardwareBuffer));
+
+  DCHECK(MessageLoop::current()->message_loop_proxy() == creator_loop_);

[jam, 2011/12/01 20:07:31]

nit: creator_loop_ isn't needed anymore

[scottmg, 2011/12/01 22:05:32]

Done.

+
+  polling_thread_.reset(new base::Thread("Gamepad polling thread"));
+  if (!polling_thread_->Start()) {

[jam, 2011/12/01 20:07:31]

can this really ever fail? most likely not (if threads aren't starting, the
system is really hosed). that way we can avoid writing the code to work around
it, which still leaves the feature broken.

[scottmg, 2011/12/01 22:05:32]

Done.

+    LOG(ERROR) << "Failed to start gamepad polling thread";
+    polling_thread_.reset();
+    return;
+  }
+
+  MessageLoop* polling_loop = polling_thread_->message_loop();
+  polling_loop->PostTask(
+      FROM_HERE,
+      base::Bind(&GamepadProvider::DoInitializePollingThread, this));
 }
 
 GamepadProvider::~GamepadProvider() {
   base::SystemMonitor* monitor = base::SystemMonitor::Get();
   if (monitor)
     monitor->RemoveDevicesChangedObserver(this);
-  Stop();
+
+  polling_thread_.reset();
+  data_fetcher_.reset();
 }
 
 base::SharedMemoryHandle GamepadProvider::GetRendererSharedMemoryHandle(
@@ -71,34 +87,27 @@ base::SharedMemoryHandle GamepadProvider::GetRendererSharedMemoryHandle(
   return renderer_handle;
 }
 
-void GamepadProvider::OnDevicesChanged() {
-  devices_changed_ = true;
+void GamepadProvider::Pause() {
+  base::AutoLock lock(is_paused_lock_);

[jam, 2011/12/01 20:07:31]

nit: better to use the lock for as little scope as possible, especially not when
posting tasks. can you make this ust around 98-101?

[scottmg, 2011/12/01 22:05:32]

Done.

+  is_paused_ = true;
 }
 
-void GamepadProvider::Start() {
-  DCHECK(MessageLoop::current()->message_loop_proxy() == creator_loop_);
+void GamepadProvider::Resume() {
+  base::AutoLock lock(is_paused_lock_);
 
-  if (polling_thread_.get())
+  if (!is_paused_)
     return;
 
-  polling_thread_.reset(new base::Thread("Gamepad polling thread"));
-  if (!polling_thread_->Start()) {
-    LOG(ERROR) << "Failed to start gamepad polling thread";
-    polling_thread_.reset();
-    return;
-  }
-
+  is_paused_ = false;
   MessageLoop* polling_loop = polling_thread_->message_loop();
   polling_loop->PostTask(
       FROM_HERE,
-      base::Bind(&GamepadProvider::DoInitializePollingThread, this));
+      base::Bind(&GamepadProvider::ScheduleDoPoll, this));
 }
 
-void GamepadProvider::Stop() {
-  DCHECK(MessageLoop::current()->message_loop_proxy() == creator_loop_);
-
-  polling_thread_.reset();
-  data_fetcher_.reset();
+void GamepadProvider::OnDevicesChanged() {
+  base::AutoLock lock(devices_changed_lock_);
+  devices_changed_ = true;
 }
 
 void GamepadProvider::DoInitializePollingThread() {
@@ -123,18 +132,27 @@ void GamepadProvider::DoPoll() {
       sizeof(WebKit::WebGamepads),
       "Racey reads are discarded");
 
+  devices_changed_lock_.Acquire();

[jam, 2011/12/01 20:07:31]

nit: please use base::AutoLock and scope 137-142

[scottmg, 2011/12/01 22:05:32]

Done. Made a copy/reset in the scoped access to avoid calling any functions
while locked.

+
   // Acquire the SeqLock. There is only ever one writer to this data.
   // See gamepad_hardware_buffer.h.
   hwbuf->sequence.WriteBegin();
   data_fetcher_->GetGamepadData(&hwbuf->buffer, devices_changed_);
   hwbuf->sequence.WriteEnd();
   devices_changed_ = false;
+
+  devices_changed_lock_.Release();
+
   // Schedule our next interval of polling.
   ScheduleDoPoll();
 }
 
 void GamepadProvider::ScheduleDoPoll() {
   DCHECK(MessageLoop::current() == polling_thread_->message_loop());
+  base::AutoLock lock(is_paused_lock_);

[jam, 2011/12/01 20:07:31]

ditto for the post task in the lock

[scottmg, 2011/12/01 22:05:32]

Done.

+
+  if (is_paused_)
+    return;
 
   MessageLoop::current()->PostDelayedTask(
       FROM_HERE,