Sync: Replace pthread mutex with base::Lock in a few more places.

This leaves just ServerConnectionManager behind.

BUG=19895
TEST=sync_unit_tests

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

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

10 files changed, 40 insertions, 63 deletions

diff --git a/chrome/browser/sync/engine/all_status.cc b/chrome/browser/sync/engine/all_status.cc
index 2cdd4fe..4ee697b 100644
--- a/chrome/browser/sync/engine/all_status.cc
+++ b/chrome/browser/sync/engine/all_status.cc
@@ -281,7 +281,7 @@ void AllStatus::HandleTalkMediatorEvent(
 }
 
 AllStatus::Status AllStatus::status() const {
-  MutexLock lock(&mutex_);
+  AutoLock lock(mutex_);
   return status_;
 }
 
@@ -313,17 +313,17 @@ int AllStatus::GetRecommendedDelay(int base_delay_ms) const {
 ScopedStatusLockWithNotify::ScopedStatusLockWithNotify(AllStatus* allstatus)
   : allstatus_(allstatus), plan_(NOTIFY_IF_STATUS_CHANGED) {
   event_.what_changed = 0;
-  allstatus->mutex_.Lock();
+  allstatus->mutex_.Acquire();
   event_.status = allstatus->status_;
 }
 
 ScopedStatusLockWithNotify::~ScopedStatusLockWithNotify() {
   if (DONT_NOTIFY == plan_) {
-    allstatus_->mutex_.Unlock();
+    allstatus_->mutex_.Release();
     return;
   }
   event_.what_changed |= allstatus_->CalcStatusChanges(&event_.status);
-  allstatus_->mutex_.Unlock();
+  allstatus_->mutex_.Release();
   if (event_.what_changed)
     allstatus_->channel()->NotifyListeners(event_);
 }
diff --git a/chrome/browser/sync/engine/all_status.h b/chrome/browser/sync/engine/all_status.h
index e0badd2..b5023a5 100644
--- a/chrome/browser/sync/engine/all_status.h
+++ b/chrome/browser/sync/engine/all_status.h
@@ -11,10 +11,10 @@
 #include <map>
 
 #include "base/atomicops.h"
+#include "base/lock.h"
 #include "base/scoped_ptr.h"
 #include "chrome/browser/sync/engine/syncer_status.h"
 #include "chrome/browser/sync/util/event_sys.h"
-#include "chrome/browser/sync/util/pthread_helpers.h"
 
 namespace browser_sync {
 
@@ -133,7 +133,6 @@ class AllStatus {
   int GetRecommendedDelay(int base_delay) const;
 
  protected:
-  typedef PThreadScopedLock<PThreadMutex> MutexLock;
   typedef std::map<Syncer*, EventListenerHookup*> Syncers;
 
   // Examines syncer to calculate syncing and the unsynced count,
@@ -154,7 +153,7 @@ class AllStatus {
   scoped_ptr<EventListenerHookup> diskfull_hookup_;
   scoped_ptr<EventListenerHookup> talk_mediator_hookup_;
 
-  mutable PThreadMutex mutex_;  // Protects all data members.
+  mutable Lock mutex_;  // Protects all data members.
   DISALLOW_COPY_AND_ASSIGN(AllStatus);
 };
 
diff --git a/chrome/browser/sync/engine/sync_cycle_state.h b/chrome/browser/sync/engine/sync_cycle_state.h
index 0a27fbf..c134c3a 100644
--- a/chrome/browser/sync/engine/sync_cycle_state.h
+++ b/chrome/browser/sync/engine/sync_cycle_state.h
@@ -18,7 +18,6 @@
 #include "chrome/browser/sync/engine/syncer_types.h"
 #include "chrome/browser/sync/engine/syncproto.h"
 #include "chrome/browser/sync/util/event_sys.h"
-#include "chrome/browser/sync/util/pthread_helpers.h"
 
 namespace syncable {
 class WriteTransaction;
diff --git a/chrome/browser/sync/engine/syncer.h b/chrome/browser/sync/engine/syncer.h
index 8c267c1..408af6a 100644
--- a/chrome/browser/sync/engine/syncer.h
+++ b/chrome/browser/sync/engine/syncer.h
@@ -18,7 +18,6 @@
 #include "chrome/browser/sync/syncable/directory_event.h"
 #include "chrome/browser/sync/util/event_sys-inl.h"
 #include "chrome/browser/sync/util/event_sys.h"
-#include "chrome/browser/sync/util/pthread_helpers.h"
 #include "testing/gtest/include/gtest/gtest_prod.h"  // For FRIEND_TEST
 
 namespace syncable {
diff --git a/chrome/browser/sync/engine/syncer_session.h b/chrome/browser/sync/engine/syncer_session.h
index 6fc22ed..a8c389f 100644
--- a/chrome/browser/sync/engine/syncer_session.h
+++ b/chrome/browser/sync/engine/syncer_session.h
@@ -21,7 +21,6 @@
 #include "chrome/browser/sync/engine/syncer_types.h"
 #include "chrome/browser/sync/engine/syncproto.h"
 #include "chrome/browser/sync/util/event_sys.h"
-#include "chrome/browser/sync/util/pthread_helpers.h"
 #include "chrome/browser/sync/util/sync_types.h"
 #include "testing/gtest/include/gtest/gtest_prod.h"  // For FRIEND_TEST
 
diff --git a/chrome/browser/sync/syncable/directory_manager.cc b/chrome/browser/sync/syncable/directory_manager.cc
index 4ce70ff..edce2e27c 100644
--- a/chrome/browser/sync/syncable/directory_manager.cc
+++ b/chrome/browser/sync/syncable/directory_manager.cc
@@ -39,16 +39,13 @@ DirectoryManager::DirectoryManager(const PathString& path)
     : root_path_(AppendSlash(path)),
       managed_directory_(NULL),
       channel_(new Channel(DirectoryManagerShutdownEvent())) {
-  CHECK(0 == pthread_mutex_init(&mutex_, NULL));
 }
 
 DirectoryManager::~DirectoryManager() {
+  AutoLock lock(lock_);
   DCHECK_EQ(managed_directory_, static_cast<Directory*>(NULL))
       << "Dir " << managed_directory_->name() << " not closed!";
-  pthread_mutex_lock(&mutex_);
   delete channel_;
-  pthread_mutex_unlock(&mutex_);
-  CHECK(0 == pthread_mutex_destroy(&mutex_));
 }
 
 bool DirectoryManager::Open(const PathString& name) {
@@ -73,15 +70,17 @@ bool DirectoryManager::Open(const PathString& name) {
 DirOpenResult DirectoryManager::OpenImpl(const PathString& name,
                                          const PathString& path,
                                          bool* was_open) {
-  pthread_mutex_lock(&mutex_);
-  // Check to see if it's already open.
   bool opened = false;
-  if (managed_directory_) {
-    DCHECK_EQ(ComparePathNames(name, managed_directory_->name()), 0)
-        << "Can't open more than one directory.";
-    opened = *was_open = true;
+  {
+    AutoLock lock(lock_);
+    // Check to see if it's already open.
+    if (managed_directory_) {
+      DCHECK_EQ(ComparePathNames(name, managed_directory_->name()), 0)
+          << "Can't open more than one directory.";
+      opened = *was_open = true;
+    }
   }
-  pthread_mutex_unlock(&mutex_);
+
   if (opened)
     return syncable::OPENED;
   // Otherwise, open it.
@@ -89,9 +88,8 @@ DirOpenResult DirectoryManager::OpenImpl(const PathString& name,
   Directory* dir = new Directory;
   const DirOpenResult result = dir->Open(path, name);
   if (syncable::OPENED == result) {
-    pthread_mutex_lock(&mutex_);
+    AutoLock lock(lock_);
     managed_directory_ = dir;
-    pthread_mutex_unlock(&mutex_);
   } else {
     delete dir;
   }
@@ -102,16 +100,16 @@ DirOpenResult DirectoryManager::OpenImpl(const PathString& name,
 // handles and resources are freed by other threads.
 void DirectoryManager::Close(const PathString& name) {
   // Erase from mounted and opened directory lists.
-  pthread_mutex_lock(&mutex_);
-
-  if (!managed_directory_ ||
-      ComparePathNames(name, managed_directory_->name()) != 0) {
-    // It wasn't open;
-    pthread_mutex_unlock(&mutex_);
-    return;
+  {
+    AutoLock lock(lock_);
+    if (!managed_directory_ ||
+        ComparePathNames(name, managed_directory_->name()) != 0) {
+      // It wasn't open.
+      return;
+    }
   }
-  pthread_mutex_unlock(&mutex_);
 
+  // TODO(timsteele): No lock?!
   // Notify listeners.
   managed_directory_->channel()->NotifyListeners(DIRECTORY_CLOSED);
   DirectoryManagerEvent event = { DirectoryManagerEvent::CLOSED, name };
@@ -121,26 +119,17 @@ void DirectoryManager::Close(const PathString& name) {
   managed_directory_ = NULL;
 }
 
-// Marks all directories as closed.  It might take a while until all the file
-// handles and resources are freed by other threads.
-void DirectoryManager::CloseAllDirectories() {
-  if (managed_directory_)
-    Close(managed_directory_->name());
-}
-
 void DirectoryManager::FinalSaveChangesForAll() {
-  pthread_mutex_lock(&mutex_);
+  AutoLock lock(lock_);
   if (managed_directory_)
     managed_directory_->SaveChanges();
-  pthread_mutex_unlock(&mutex_);
 }
 
 void DirectoryManager::GetOpenDirectories(DirNames* result) {
   result->clear();
-  pthread_mutex_lock(&mutex_);
+  AutoLock lock(lock_);
   if (managed_directory_)
     result->push_back(managed_directory_->name());
-  pthread_mutex_unlock(&mutex_);
 }
 
 ScopedDirLookup::ScopedDirLookup(DirectoryManager* dirman,
diff --git a/chrome/browser/sync/syncable/directory_manager.h b/chrome/browser/sync/syncable/directory_manager.h
index 197b46b..f059257 100644
--- a/chrome/browser/sync/syncable/directory_manager.h
+++ b/chrome/browser/sync/syncable/directory_manager.h
@@ -12,11 +12,10 @@
 #ifndef CHROME_BROWSER_SYNC_SYNCABLE_DIRECTORY_MANAGER_H_
 #define CHROME_BROWSER_SYNC_SYNCABLE_DIRECTORY_MANAGER_H_
 
-#include <pthread.h>
-
 #include <vector>
 
 #include "base/atomicops.h"
+#include "base/lock.h"
 #include "base/basictypes.h"
 #include "chrome/browser/sync/syncable/dir_open_result.h"
 #include "chrome/browser/sync/syncable/path_name_cmp.h"
@@ -66,10 +65,6 @@ class DirectoryManager {
   // file handles and resources are freed by other threads.
   void Close(const PathString& name);
 
-  // Marks all directories as closed.  It might take a while until all the
-  // file handles and resources are freed by other threads.
-  void CloseAllDirectories();
-
   // Should be called at App exit.
   void FinalSaveChangesForAll();
 
@@ -87,8 +82,9 @@ class DirectoryManager {
   friend class ScopedDirLookup;
 
   const PathString root_path_;
+
   // protects managed_directory_
-  mutable pthread_mutex_t mutex_;
+  Lock lock_;
   Directory* managed_directory_;
 
   Channel* const channel_;
diff --git a/chrome/browser/sync/syncable/syncable_unittest.cc b/chrome/browser/sync/syncable/syncable_unittest.cc
index 41ad4d5..e271ade 100644
--- a/chrome/browser/sync/syncable/syncable_unittest.cc
+++ b/chrome/browser/sync/syncable/syncable_unittest.cc
@@ -40,7 +40,6 @@
 #include "chrome/browser/sync/util/compat_file.h"
 #include "chrome/browser/sync/util/event_sys-inl.h"
 #include "chrome/browser/sync/util/path_helpers.h"
-#include "chrome/browser/sync/util/pthread_helpers.h"
 #include "chrome/browser/sync/util/query_helpers.h"
 #include "chrome/test/sync/engine/test_id_factory.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -921,7 +920,7 @@ namespace DirectoryKernelStalenessBug {
           CHECK(dir.good());
           dir->SaveChanges();
         }
-        dirman->CloseAllDirectories();
+        dirman->Close(dirname);
         break;
       case 1:
         {
diff --git a/chrome/browser/sync/util/user_settings.cc b/chrome/browser/sync/util/user_settings.cc
index 6dbde96..a8ab972 100644
--- a/chrome/browser/sync/util/user_settings.cc
+++ b/chrome/browser/sync/util/user_settings.cc
@@ -40,7 +40,7 @@ static const int kSaltSize = 20;
 static const int kCurrentDBVersion = 11;
 
 UserSettings::ScopedDBHandle::ScopedDBHandle(UserSettings* settings) :
-  mutex_lock_(&settings->dbhandle_mutex_), handle_(&settings->dbhandle_) {
+  mutex_lock_(settings->dbhandle_mutex_), handle_(&settings->dbhandle_) {
 }
 
 UserSettings::UserSettings() :
@@ -48,7 +48,7 @@ UserSettings::UserSettings() :
 }
 
 string UserSettings::email() const {
-  ScopedLock lock(&mutex_);
+  AutoLock lock(mutex_);
   return email_;
 }
 
@@ -260,10 +260,8 @@ void UserSettings::StoreHashedPassword(const string& email,
                                        const string& password) {
   // Save one-way hashed password:
   char binary_salt[kSaltSize];
-  {
-    ScopedLock lock(&mutex_);
-    GetRandomBytes(binary_salt, sizeof(binary_salt));
-  }
+  GetRandomBytes(binary_salt, sizeof(binary_salt));
+
   const string salt = APEncode(string(binary_salt, sizeof(binary_salt)));
   MD5Calculator md5;
   md5.AddData(salt.data(), salt.size());
@@ -311,7 +309,7 @@ bool UserSettings::VerifyAgainstStoredHash(const string& email,
 
 void UserSettings::SwitchUser(const string& username) {
   {
-    ScopedLock lock(&mutex_);
+    AutoLock lock(mutex_);
     email_ = username;
   }
 }
diff --git a/chrome/browser/sync/util/user_settings.h b/chrome/browser/sync/util/user_settings.h
index 303397a..60ad1a9 100644
--- a/chrome/browser/sync/util/user_settings.h
+++ b/chrome/browser/sync/util/user_settings.h
@@ -9,8 +9,8 @@
 #include <set>
 #include <string>
 
+#include "base/lock.h"
 #include "build/build_config.h"
-#include "chrome/browser/sync/util/pthread_helpers.h"
 #include "chrome/browser/sync/util/signin.h"
 #include "chrome/browser/sync/util/sync_types.h"
 
@@ -80,7 +80,7 @@ class UserSettings {
   struct ScopedDBHandle {
     ScopedDBHandle(UserSettings* settings);
     inline sqlite3* get() const { return *handle_; }
-    PThreadScopedLock<PThreadMutex> mutex_lock_;
+    AutoLock mutex_lock_;
     sqlite3** const handle_;
   };
 
@@ -91,12 +91,11 @@ class UserSettings {
 
  private:
   std::string email_;
-  mutable PThreadMutex mutex_;  // protects email_
-  typedef PThreadScopedLock<PThreadMutex> ScopedLock;
+  mutable Lock mutex_;  // protects email_.
 
   // We keep a single dbhandle.
   sqlite3* dbhandle_;
-  PThreadMutex dbhandle_mutex_;
+  Lock dbhandle_mutex_;
 
   // TODO(sync): Use in-memory cache for service auth tokens on posix.
   // Have someone competent in Windows switch it over to not use Sqlite in the