Switch from OnIPAddressChanged and OnConnectionTypeChange to OnNetworkChanged

chrome/browser/chromeos/drive/drive_sync_client.cc

 // Copyright (c) 2012 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/chromeos/drive/drive_sync_client.h"
 
 #include <algorithm>
 #include <vector>
 
 #include "base/bind.h"
@@ skipping 86 matching lines @@
       ALLOW_THIS_IN_INITIALIZER_LIST(weak_ptr_factory_(this)) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 }
 
 DriveSyncClient::~DriveSyncClient() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   if (file_system_)
     file_system_->RemoveObserver(this);
   if (cache_)
     cache_->RemoveObserver(this);
-  net::NetworkChangeNotifier::RemoveConnectionTypeObserver(this);
+  net::NetworkChangeNotifier::RemoveNetworkChangeObserver(this);
 }
 
 void DriveSyncClient::Initialize() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   file_system_->AddObserver(this);
   cache_->AddObserver(this);
 
-  net::NetworkChangeNotifier::AddConnectionTypeObserver(this);
+  net::NetworkChangeNotifier::AddNetworkChangeObserver(this);
 
   registrar_->Init(profile_->GetPrefs());
   base::Closure callback = base::Bind(
       &DriveSyncClient::OnDriveSyncPreferenceChanged, base::Unretained(this));
   registrar_->Add(prefs::kDisableDrive, callback);
   registrar_->Add(prefs::kDisableDriveOverCellular, callback);
 }
 
 void DriveSyncClient::StartProcessingBacklog() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
@@ skipping 20 matching lines @@
     SyncType sync_type) const {
   std::vector<std::string> resource_ids;
   for (size_t i = 0; i < queue_.size(); ++i) {
     const SyncTask& sync_task = queue_[i];
     if (sync_task.sync_type == sync_type)
       resource_ids.push_back(sync_task.resource_id);
   }
   return resource_ids;
 }
 
-void DriveSyncClient::StartSyncLoop() {
+void DriveSyncClient::StartSyncLoop(bool force_offline) {
   if (!sync_loop_is_running_)
-    DoSyncLoop();
+    DoSyncLoop(force_offline);
 }
 
-void DriveSyncClient::DoSyncLoop() {
+void DriveSyncClient::DoSyncLoop(bool force_offline) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
-  if (ShouldStopSyncLoop()) {
+  if (ShouldStopSyncLoop(force_offline)) {
     // Note that |queue_| is not cleared so the sync loop can resume.
     sync_loop_is_running_ = false;
     FOR_EACH_OBSERVER(DriveSyncClientObserver, observers_,
                       OnSyncClientStopped());
     return;
   }
   if (queue_.empty()) {
     sync_loop_is_running_ = false;
     FOR_EACH_OBSERVER(DriveSyncClientObserver, observers_, OnSyncClientIdle());
     return;
   }
   sync_loop_is_running_ = true;
 
   // Should copy before calling queue_.pop_front().
   const SyncTask sync_task = queue_.front();
 
   // Check if we are ready to process the task.
   const base::TimeDelta elapsed = base::Time::Now() - sync_task.timestamp;
   if (elapsed < delay_) {
     // Not yet ready. Revisit at a later time.
     const bool posted = base::MessageLoopProxy::current()->PostDelayedTask(
         FROM_HERE,
         base::Bind(&DriveSyncClient::DoSyncLoop,
-                   weak_ptr_factory_.GetWeakPtr()),
+                   weak_ptr_factory_.GetWeakPtr(),
+                   force_offline),

[szym, 2013/01/20 06:52:08]

I was concerned that, because it's using PostDelayedTask, it's possible that
there's a spurious |DoSyncLoop(true)| scheduled to happen after a subsequent
call |DoSyncLoop(false)|.

But now that I am reading this carefully, |force_offline| MUST be false to even
get here. Otherwise ShouldStopSyncLoop would have returned true. Therefore, I
suggest using |false| here.

         delay_);
     DCHECK(posted);
     return;
   }
 
   FOR_EACH_OBSERVER(DriveSyncClientObserver, observers_, OnSyncTaskStarted());
 
   queue_.pop_front();
   if (sync_task.sync_type == FETCH) {
     DVLOG(1) << "Fetching " << sync_task.resource_id;
     file_system_->GetFileByResourceId(
         sync_task.resource_id,
         base::Bind(&DriveSyncClient::OnFetchFileComplete,
                    weak_ptr_factory_.GetWeakPtr(),
                    sync_task),
         google_apis::GetContentCallback());
   } else if (sync_task.sync_type == UPLOAD) {
     DVLOG(1) << "Uploading " << sync_task.resource_id;
     file_system_->UpdateFileByResourceId(
         sync_task.resource_id,
         base::Bind(&DriveSyncClient::OnUploadFileComplete,
                    weak_ptr_factory_.GetWeakPtr(),
                    sync_task.resource_id));
   } else {
     NOTREACHED() << ": Unexpected sync type: " << sync_task.sync_type;
   }
 }
 
-bool DriveSyncClient::ShouldStopSyncLoop() {
+bool DriveSyncClient::ShouldStopSyncLoop(bool force_offline) {
   // Should stop if the drive feature was disabled while running the fetch
   // loop.
   if (profile_->GetPrefs()->GetBoolean(prefs::kDisableDrive))
     return true;
 
   // Should stop if the network is not online.
-  if (net::NetworkChangeNotifier::IsOffline())
+  if (force_offline || net::NetworkChangeNotifier::IsOffline())
     return true;
 
   // Should stop if the current connection is on cellular network, and
   // fetching is disabled over cellular.
   if (profile_->GetPrefs()->GetBoolean(prefs::kDisableDriveOverCellular) &&
       net::NetworkChangeNotifier::IsConnectionCellular(
           net::NetworkChangeNotifier::GetConnectionType()))
     return true;
 
   return false;
@@ skipping 10 matching lines @@
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   StartCheckingExistingPinnedFiles();
 }
 
 void DriveSyncClient::OnCachePinned(const std::string& resource_id,
                                     const std::string& md5) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   AddTaskToQueue(SyncTask(FETCH, resource_id, base::Time::Now()));
-  StartSyncLoop();
+  StartSyncLoop(false);
 }
 
 void DriveSyncClient::OnCacheUnpinned(const std::string& resource_id,
                                       const std::string& md5) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   // Remove the resource_id if it's in the queue. This can happen if the user
   // cancels pinning before the file is fetched.
   std::deque<SyncTask>::iterator iter =
       std::find_if(queue_.begin(), queue_.end(),
                    CompareTypeAndResourceId(FETCH, resource_id));
   if (iter != queue_.end())
     queue_.erase(iter);
 }
 
 void DriveSyncClient::OnCacheCommitted(const std::string& resource_id) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   AddTaskToQueue(SyncTask(UPLOAD, resource_id, base::Time::Now()));
-  StartSyncLoop();
+  StartSyncLoop(false);
 }
 
 void DriveSyncClient::AddObserver(DriveSyncClientObserver* observer) {
   observers_.AddObserver(observer);
 }
 
 void DriveSyncClient::RemoveObserver(DriveSyncClientObserver* observer) {
   observers_.RemoveObserver(observer);
 }
 
@@ skipping 24 matching lines @@
     DVLOG(1) << "Queuing to upload: " << resource_id;
     AddTaskToQueue(SyncTask(UPLOAD, resource_id, base::Time::Now()));
   }
 
   for (size_t i = 0; i < to_fetch->size(); ++i) {
     const std::string& resource_id = (*to_fetch)[i];
     DVLOG(1) << "Queuing to fetch: " << resource_id;
     AddTaskToQueue(SyncTask(FETCH, resource_id, base::Time::Now()));
   }
 
-  StartSyncLoop();
+  StartSyncLoop(false);
 }
 
 void DriveSyncClient::OnGetResourceIdOfExistingPinnedFile(
     const std::string& resource_id,
     const DriveCacheEntry& cache_entry) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   if (cache_entry.is_pinned() && cache_entry.is_present()) {
     file_system_->GetEntryInfoByResourceId(
         resource_id,
@@ skipping 54 matching lines @@
                                DriveFileError error) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   if (error != DRIVE_FILE_OK) {
     LOG(WARNING) << "Failed to pin cache entry: " << resource_id;
     return;
   }
 
   // Finally, adding to the queue.
   AddTaskToQueue(SyncTask(FETCH, resource_id, base::Time::Now()));
-  StartSyncLoop();
+  StartSyncLoop(false);
 }
 
 void DriveSyncClient::OnFetchFileComplete(const SyncTask& sync_task,
                                           DriveFileError error,
                                           const FilePath& local_path,
                                           const std::string& ununsed_mime_type,
                                           DriveFileType file_type) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   if (error == DRIVE_FILE_OK) {
     DVLOG(1) << "Fetched " << sync_task.resource_id << ": "
              << local_path.value();
   } else {
     switch (error) {
       case DRIVE_FILE_ERROR_NO_CONNECTION:
         // Re-queue the task so that we'll retry once the connection is back.
         queue_.push_front(sync_task);
         break;
       default:
         LOG(WARNING) << "Failed to fetch " << sync_task.resource_id
                      << ": " << error;
     }
   }
 
   // Continue the loop.
-  DoSyncLoop();
+  DoSyncLoop(false);
 }
 
 void DriveSyncClient::OnUploadFileComplete(const std::string& resource_id,
                                            DriveFileError error) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   if (error == DRIVE_FILE_OK) {
     DVLOG(1) << "Uploaded " << resource_id;
   } else {
     // TODO(satorux): We should re-queue if the error is recoverable.
     LOG(WARNING) << "Failed to upload " << resource_id << ": " << error;
   }
 
   // Continue the loop.
-  DoSyncLoop();
+  DoSyncLoop(false);
 }
 
 void DriveSyncClient::OnDriveSyncPreferenceChanged() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   // Resume the sync loop if gdata preferences are changed. Note that we
   // don't need to check the new values here as these will be checked in
   // ShouldStopSyncLoop() as soon as the loop is resumed.
-  StartSyncLoop();
+  StartSyncLoop(false);
 }
 
-void DriveSyncClient::OnConnectionTypeChanged(
+void DriveSyncClient::OnNetworkChanged(
     net::NetworkChangeNotifier::ConnectionType type) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   // Resume the sync loop if the network is changed. Note that we don't need to
   // check the type of the network as it will be checked in ShouldStopSyncLoop()
   // as soon as the loop is resumed.
-  StartSyncLoop();
+  StartSyncLoop(type == net::NetworkChangeNotifier::CONNECTION_NONE);

[szym, 2013/01/20 06:52:08]

Consider the result of |StartSyncLoop(true)|.

Case 1) |sync_loop_running_ == true|, then it's a noop.
Case 2) otherwise it's just FOR_EACH_OBSERVER(OnSyncClientStopped)

Something tells me that is wrong.

 }
 
 }  // namespace drive