[Sync] Remove now-unneeded initialized_mutex_.

chrome/browser/sync/engine/syncapi.cc

 // Copyright (c) 2011 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/browser/sync/engine/syncapi.h"
 
 #include <algorithm>
 #include <bitset>
 #include <iomanip>
 #include <list>
 #include <map>
 #include <queue>
 #include <string>
 #include <vector>
 
 #include "base/base64.h"
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/command_line.h"
 #include "base/compiler_specific.h"
 #include "base/json/json_writer.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop.h"
 #include "base/observer_list.h"
 #include "base/sha1.h"
 #include "base/string_number_conversions.h"
 #include "base/string_util.h"
-#include "base/synchronization/lock.h"
 #include "base/task.h"
 #include "base/time.h"
 #include "base/utf_string_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/sync/engine/all_status.h"
 #include "chrome/browser/sync/engine/change_reorder_buffer.h"
 #include "chrome/browser/sync/engine/model_safe_worker.h"
 #include "chrome/browser/sync/engine/nudge_source.h"
 #include "chrome/browser/sync/engine/net/server_connection_manager.h"
 #include "chrome/browser/sync/engine/net/syncapi_server_connection_manager.h"
@@ skipping 1220 matching lines @@
   bool HaveObservers() const;
   void AddObserver(SyncManager::Observer* observer);
   void RemoveObserver(SyncManager::Observer* observer);
 
   // Accessors for the private members.
   DirectoryManager* dir_manager() { return share_.dir_manager.get(); }
   SyncAPIServerConnectionManager* connection_manager() {
     return connection_manager_.get();
   }
   SyncScheduler* scheduler() { return scheduler_.get(); }
-  UserShare* GetUserShare() { return &share_; }
+  UserShare* GetUserShare() {
+    DCHECK(initialized_);
+    return &share_;
+  }
 
   // Return the currently active (validated) username for use with syncable
   // types.
   const std::string& username_for_share() const {
     return share_.name;
   }
 
   Status GetStatus();
 
   void RequestNudge(const tracked_objects::Location& nudge_location);
 
   void RequestNudgeWithDataTypes(const TimeDelta& delay,
       browser_sync::NudgeSource source, const ModelTypeBitSet& types,
       const tracked_objects::Location& nudge_location);
 
   void RequestEarlyExit();
 
   // See SyncManager::Shutdown for information.
   void Shutdown();
 
-  // Whether we're initialized to the point of being able to accept changes
-  // (and hence allow transaction creation). See initialized_ for details.
-  bool initialized() const {
-    base::AutoLock lock(initialized_mutex_);
-    return initialized_;
-  }
-
   // If this is a deletion for a password, sets the legacy
   // ExtraPasswordChangeRecordData field of |buffer|. Otherwise sets
   // |buffer|'s specifics field to contain the unencrypted data.
   void SetExtraChangeRecordData(int64 id,
                                 syncable::ModelType type,
                                 ChangeReorderBuffer* buffer,
                                 Cryptographer* cryptographer,
                                 const syncable::EntryKernel& original,
                                 bool existed_before,
                                 bool exists_now);
@@ skipping 93 matching lines @@
       (SyncManager::SyncInternal::*UnboundJsMessageHandler)(
           const browser_sync::JsArgList&);
   typedef base::Callback<browser_sync::JsArgList(browser_sync::JsArgList)>
       JsMessageHandler;
   typedef std::map<std::string, JsMessageHandler> JsMessageHandlerMap;
 
   // Helper to call OnAuthError when no authentication credentials are
   // available.
   void RaiseAuthNeededEvent();
 
-  // Helper to set initialized_ to true and raise an event to clients to notify
-  // that initialization is complete and it is safe to send us changes. If
-  // already initialized, this is a no-op.
-  void MarkAndNotifyInitializationComplete();
-
   // Determine if the parents or predecessors differ between the old and new
   // versions of an entry stored in |a| and |b|.  Note that a node's index may
   // change without its NEXT_ID changing if the node at NEXT_ID also moved (but
   // the relative order is unchanged).  To handle such cases, we rely on the
   // caller to treat a position update on any sibling as updating the positions
   // of all siblings.
   static bool VisiblePositionsDiffer(const syncable::EntryKernel& a,
                                      const syncable::Entry& b) {
     // If the datatype isn't one where the browser model cares about position,
     // don't bother notifying that data model of position-only changes.
@@ skipping 155 matching lines @@
   // Event listener hookup for the ServerConnectionManager.
   scoped_ptr<EventListenerHookup> connection_manager_hookup_;
 
   // The sync dir_manager to which we belong.
   SyncManager* const sync_manager_;
 
   // The entity that provides us with information about which types to sync.
   // The instance is shared between the SyncManager and the Syncer.
   ModelSafeWorkerRegistrar* registrar_;
 
-  // Set to true once Init has been called, and we know of an
-  // authenticated valid) username either from a fresh authentication
-  // attempt (as in first-use case) or from a previous attempt stored
-  // in our UserSettings (as in the steady-state), and the
-  // syncable::Directory has been opened, meaning we are ready to
-  // accept changes.  Protected by initialized_mutex_ as it can get
-  // read/set by both the SyncScheduler and the AuthWatcherThread.
+  // Set to true once Init has been called.
   bool initialized_;
-  mutable base::Lock initialized_mutex_;
 
   // True if the SyncManager should be running in test mode (no sync
   // scheduler actually communicating with the server).
   bool setup_for_test_mode_;
 
   ScopedRunnableMethodFactory<SyncManager::SyncInternal> method_factory_;
 
   // Map used to store the notification info to be displayed in
   // about:sync page.
   NotificationInfoMap notification_info_map_;
@@ skipping 131 matching lines @@
     const std::string& sync_server_and_path,
     int port,
     bool use_ssl,
     HttpPostProviderFactory* post_factory,
     ModelSafeWorkerRegistrar* model_safe_worker_registrar,
     const char* user_agent,
     const SyncCredentials& credentials,
     sync_notifier::SyncNotifier* sync_notifier,
     const std::string& restored_key_for_bootstrapping,
     bool setup_for_test_mode) {
+  CHECK(!initialized_);
 
   VLOG(1) << "Starting SyncInternal initialization.";
 
   sync_loop_ = MessageLoop::current();
   DCHECK(sync_loop_);
   registrar_ = model_safe_worker_registrar;
   setup_for_test_mode_ = setup_for_test_mode;
 
   sync_notifier_ = sync_notifier;
 
@@ skipping 28 matching lines @@
     scheduler_.reset(new SyncScheduler(name_, context, new Syncer()));
   }
 
   bool signed_in = SignIn(credentials);
 
   if (signed_in && scheduler()) {
     scheduler()->Start(
         browser_sync::SyncScheduler::CONFIGURATION_MODE, NULL);
   }
 
-  // Do this once the directory is opened.
+  initialized_ = true;
+
+  // Notify that initialization is complete.
+  ObserverList<SyncManager::Observer> temp_obs_list;
+  CopyObservers(&temp_obs_list);
+  FOR_EACH_OBSERVER(SyncManager::Observer, temp_obs_list,
+                    OnInitializationComplete());
+
+  // The following calls check that initialized_ is true.
+
   BootstrapEncryption(restored_key_for_bootstrapping);
-  MarkAndNotifyInitializationComplete();
 
-  // Only listen to notification once we are completely initialized. This is
-  // necessary because calls to e.g. |SetState| or |UpdateCredentials| may
-  // trigger notifications, and those can be called before initialized_ is set
-  // to true.
   sync_notifier_->AddObserver(this);
 
   return signed_in;
 }
 
 void SyncManager::SyncInternal::BootstrapEncryption(
     const std::string& restored_key_for_bootstrapping) {
   // Cryptographer should only be accessed while holding a transaction.
   ReadTransaction trans(GetUserShare());
   Cryptographer* cryptographer = trans.GetCryptographer();
 
   // Set the bootstrap token before bailing out if nigori node is not there.
   // This could happen if server asked us to migrate nigri.
   cryptographer->Bootstrap(restored_key_for_bootstrapping);
 }
 
 bool SyncManager::SyncInternal::UpdateCryptographerFromNigori() {
+  DCHECK(initialized_);
   syncable::ScopedDirLookup lookup(dir_manager(), username_for_share());
   if (!lookup.good()) {
     NOTREACHED() << "BootstrapEncryption: lookup not good so bailing out";
     return false;
   }
   if (!lookup->initial_sync_ended_for_type(syncable::NIGORI))
     return false;  // Should only happen during first time sync.
 
   ReadTransaction trans(GetUserShare());
   Cryptographer* cryptographer = trans.GetCryptographer();
@@ skipping 16 matching lines @@
 }
 
 void SyncManager::SyncInternal::StartSyncingNormally() {
   // Start the sync scheduler. This won't actually result in any
   // syncing until at least the DirectoryManager broadcasts the OPENED
   // event, and a valid server connection is detected.
   if (scheduler())  // NULL during certain unittests.
     scheduler()->Start(SyncScheduler::NORMAL_MODE, NULL);
 }
 
-void SyncManager::SyncInternal::MarkAndNotifyInitializationComplete() {
-  // There is only one real time we need this mutex.  If we get an auth
-  // success, and before the initial sync ends we get an auth failure.  In this
-  // case we'll be listening to both the AuthWatcher and Syncer, and it's a race
-  // between their respective threads to call MarkAndNotify.  We need to make
-  // sure the observer is notified once and only once.
-  {
-    base::AutoLock lock(initialized_mutex_);
-    if (initialized_)
-      return;
-    initialized_ = true;
-  }
-
-  // Notify that initialization is complete.
-  ObserverList<SyncManager::Observer> temp_obs_list;
-  CopyObservers(&temp_obs_list);
-  FOR_EACH_OBSERVER(SyncManager::Observer, temp_obs_list,
-                    OnInitializationComplete());
-}
-
 bool SyncManager::SyncInternal::OpenDirectory() {
-  DCHECK(!initialized()) << "Should only happen once";
+  DCHECK(!initialized_) << "Should only happen once";
 
   bool share_opened = dir_manager()->Open(username_for_share());
   DCHECK(share_opened);
   if (!share_opened) {
     ObserverList<SyncManager::Observer> temp_obs_list;
     CopyObservers(&temp_obs_list);
     FOR_EACH_OBSERVER(SyncManager::Observer, temp_obs_list,
                       OnStopSyncingPermanently());
 
     LOG(ERROR) << "Could not open share for:" << username_for_share();
@@ skipping 178 matching lines @@
     // TODO(albertb): Plumb an UnrecoverableError all the way back to the PSS.
     NOTREACHED();
     return false;
   }
 
   return node.GetNigoriSpecifics().using_explicit_passphrase();
 }
 
 void SyncManager::SyncInternal::EncryptDataTypes(
     const syncable::ModelTypeSet& encrypted_types) {
+  DCHECK(initialized_);
   VLOG(1) << "Attempting to encrypt datatypes "
           << syncable::ModelTypeSetToString(encrypted_types);
 
   WriteTransaction trans(GetUserShare());
   WriteNode node(&trans);
   if (!node.InitByTagLookup(kNigoriTag)) {
     NOTREACHED() << "Unable to set encrypted datatypes because Nigori node not "
                  << "found.";
     return;
   }
@@ skipping 467 matching lines @@
           FOR_EACH_OBSERVER(SyncManager::Observer, temp_obs_list,
                             OnPassphraseRequired(sync_api::REASON_ENCRYPTION));
         }
         // If everything is in order(we have the passphrase) then there is no
         // need to inform the listeners. They will just wait for sync
         // completion event and if no errors have been raised it means
         // encryption was succesful.
       }
     }
 
-    if (!initialized()) {
+    if (!initialized_) {
       LOG(INFO) << "OnSyncCycleCompleted not sent because sync api is not "
                 << "initialized";
       return;
     }
 
     if (!event.snapshot->has_more_to_sync) {
       VLOG(1) << "OnSyncCycleCompleted sent";
       ObserverList<SyncManager::Observer> temp_obs_list;
       CopyObservers(&temp_obs_list);
       FOR_EACH_OBSERVER(SyncManager::Observer, temp_obs_list,
@@ skipping 415 matching lines @@
                                           share->name);
   cryptographer_ = share->dir_manager->GetCryptographer(this);
   if (!(lookup_->good()))
     DCHECK(false) << "ScopedDirLookup failed on valid DirManager.";
 }
 BaseTransaction::~BaseTransaction() {
   delete lookup_;
 }
 
 UserShare* SyncManager::GetUserShare() const {
-  DCHECK(data_->initialized()) << "GetUserShare requires initialization!";
   return data_->GetUserShare();
 }
 
 void SyncManager::RefreshEncryption() {
-  DCHECK(data_->initialized());
   if (data_->UpdateCryptographerFromNigori())
     data_->EncryptDataTypes(syncable::ModelTypeSet());
 }
 
 syncable::ModelTypeSet SyncManager::GetEncryptedDataTypes() const {
   sync_api::ReadTransaction trans(GetUserShare());
   return GetEncryptedTypes(&trans);
 }
 
 bool SyncManager::HasUnsyncedItems() const {
@@ skipping 27 matching lines @@
 void SyncManager::TriggerOnIncomingNotificationForTest(
     const syncable::ModelTypeBitSet& model_types) {
   syncable::ModelTypePayloadMap model_types_with_payloads =
       syncable::ModelTypePayloadMapFromBitSet(model_types,
           std::string());
 
   data_->OnIncomingNotification(model_types_with_payloads);
 }
 
 }  // namespace sync_api