sync: Disable network change notifications for unauthenticated users.

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>
@@ skipping 1183 matching lines @@
       public syncable::DirectoryChangeDelegate {
   static const int kDefaultNudgeDelayMilliseconds;
   static const int kPreferencesNudgeDelayMilliseconds;
  public:
   explicit SyncInternal(const std::string& name)
       : weak_ptr_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)),
         parent_router_(NULL),
         registrar_(NULL),
         initialized_(false),
         setup_for_test_mode_(false),
+        observing_ip_address_changes_(false),
         js_transaction_observer_(ALLOW_THIS_IN_INITIALIZER_LIST(this)) {
     // Pre-fill |notification_info_map_|.
     for (int i = syncable::FIRST_REAL_MODEL_TYPE;
          i < syncable::MODEL_TYPE_COUNT; ++i) {
       notification_info_map_.insert(
           std::make_pair(syncable::ModelTypeFromInt(i), NotificationInfo()));
     }
 
     // Bind message handlers.
     BindJsMessageHandler(
@@ skipping 406 matching lines @@
   // The instance is shared between the SyncManager and the Syncer.
   ModelSafeWorkerRegistrar* registrar_;
 
   // Set to true once Init has been called.
   bool initialized_;
 
   // True if the SyncManager should be running in test mode (no sync
   // scheduler actually communicating with the server).
   bool setup_for_test_mode_;
 
+  // Whether we should respond to an IP address change notification.
+  bool observing_ip_address_changes_;

[akalin, 2011/07/29 03:08:57]

I think having 'bool has_valid_credentials_;' is clearer, since that's what
actually matters.

+
   // Map used to store the notification info to be displayed in
   // about:sync page.
   NotificationInfoMap notification_info_map_;
 
   browser_sync::JsTransactionObserver js_transaction_observer_;
 
   JsMessageHandlerMap js_message_handlers_;
 };
 const int SyncManager::SyncInternal::kDefaultNudgeDelayMilliseconds = 200;
 const int SyncManager::SyncInternal::kPreferencesNudgeDelayMilliseconds = 2000;
@@ skipping 123 matching lines @@
   setup_for_test_mode_ = setup_for_test_mode;
 
   sync_notifier_.reset(sync_notifier);
 
   share_.dir_manager.reset(new DirectoryManager(database_location));
 
   connection_manager_.reset(new SyncAPIServerConnectionManager(
       sync_server_and_path, port, use_ssl, user_agent, post_factory));
 
   net::NetworkChangeNotifier::AddIPAddressObserver(this);
+  observing_ip_address_changes_ = true;

[akalin, 2011/07/29 03:08:57]

shouldn't this remain false?  We're initing, but we don't yet have valid creds,
do we?

 
   connection_manager()->AddListener(this);
 
   // TODO(akalin): CheckServerReachable() can block, which may cause jank if we
   // try to shut down sync.  Fix this.
   MessageLoop::current()->PostTask(
       FROM_HERE, base::Bind(&SyncInternal::CheckServerReachable,
                             weak_ptr_factory_.GetWeakPtr()));
 
   // Test mode does not use a syncer context or syncer thread.
@@ skipping 151 matching lines @@
   UpdateEnabledTypes();
   return true;
 }
 
 void SyncManager::SyncInternal::UpdateCredentials(
     const SyncCredentials& credentials) {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK_EQ(credentials.email, share_.name);
   DCHECK(!credentials.email.empty());
   DCHECK(!credentials.sync_token.empty());
+
+  observing_ip_address_changes_ = true;
   connection_manager()->set_auth_token(credentials.sync_token);
   sync_notifier_->UpdateCredentials(
       credentials.email, credentials.sync_token);
   if (!setup_for_test_mode_) {
     CheckServerReachable();
   }
 }
 
 void SyncManager::SyncInternal::UpdateEnabledTypes() {
   DCHECK(thread_checker_.CalledOnValidThread());
@@ skipping 276 matching lines @@
     sync_notifier_->RemoveObserver(this);
   }
   sync_notifier_.reset();
 
   if (connection_manager_.get()) {
     connection_manager_->RemoveListener(this);
   }
   connection_manager_.reset();
 
   net::NetworkChangeNotifier::RemoveIPAddressObserver(this);
+  observing_ip_address_changes_ = false;
 
   if (dir_manager()) {
     dir_manager()->FinalSaveChangesForAll();
     dir_manager()->Close(username_for_share());
   }
 
   // Reset the DirectoryManager and UserSettings so they relinquish sqlite
   // handles to backing files.
   share_.dir_manager.reset();
 
   setup_for_test_mode_ = false;
   parent_router_ = NULL;
   registrar_ = NULL;
 
   initialized_ = false;
 
   // We reset this here, since only now we know it will not be
   // accessed from other threads (since we shut down everything).
   weak_handle_this_.Reset();
 }
 
 void SyncManager::SyncInternal::OnIPAddressChanged() {
   VLOG(1) << "IP address change detected";
+  if (!observing_ip_address_changes_) {
+    VLOG(1) << "IP address change dropped.";

[akalin, 2011/07/29 03:08:57]

VLOG can also say something like "IP address changed but no valid credentials;
ignoring"

+    return;
+  }
+
 #if defined (OS_CHROMEOS)
   // TODO(tim): This is a hack to intentionally lose a race with flimflam at
   // shutdown, so we don't cause shutdown to wait for our http request.
   // http://crosbug.com/8429
   MessageLoop::current()->PostDelayedTask(
       FROM_HERE,
       base::Bind(&SyncInternal::OnIPAddressChangedImpl,
                  weak_ptr_factory_.GetWeakPtr()),
       kChromeOSNetworkChangeReactionDelayHackMsec);
 #else
@@ skipping 12 matching lines @@
   allstatus_.HandleServerConnectionEvent(event);
   if (event.connection_code ==
       browser_sync::HttpResponse::SERVER_CONNECTION_OK) {
     ObserverList<SyncManager::Observer> temp_obs_list;
     CopyObservers(&temp_obs_list);
     FOR_EACH_OBSERVER(SyncManager::Observer, temp_obs_list,
                       OnAuthError(AuthError::None()));
   }
 
   if (event.connection_code == browser_sync::HttpResponse::SYNC_AUTH_ERROR) {
+    observing_ip_address_changes_ = false;
     ObserverList<SyncManager::Observer> temp_obs_list;
     CopyObservers(&temp_obs_list);
     FOR_EACH_OBSERVER(SyncManager::Observer, temp_obs_list,
         OnAuthError(AuthError(AuthError::INVALID_GAIA_CREDENTIALS)));
   }
 
   if (event.connection_code ==
       browser_sync::HttpResponse::SYNC_SERVER_ERROR) {
     ObserverList<SyncManager::Observer> temp_obs_list;
     CopyObservers(&temp_obs_list);
@@ skipping 740 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