contacts: Defer updates when offline.

This make GoogleContactStore postpone trying to update
contacts via GData when the system doesn't have a network
connection.

BUG=128805
TEST=none


Review URL: https://chromiumcodereview.appspot.com/10829247

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

chrome/browser/chromeos/contacts/google_contact_store.cc

@@ -74,19 +74,32 @@ void GoogleContactStore::TestAPI::DoUpdate() {
   store_->UpdateContacts();
 }
 
+void GoogleContactStore::TestAPI::NotifyAboutNetworkStateChange(bool online) {
+  net::NetworkChangeNotifier::ConnectionType type =
+      online ?
+      net::NetworkChangeNotifier::CONNECTION_UNKNOWN :
+      net::NetworkChangeNotifier::CONNECTION_NONE;
+  store_->OnConnectionTypeChanged(type);
+}
+
 GoogleContactStore::GoogleContactStore(Profile* profile)
     : profile_(profile),
       contacts_deleter_(&contacts_),
       db_(new ContactDatabase),
       update_delay_on_next_failure_(
           base::TimeDelta::FromSeconds(kUpdateFailureInitialRetrySec)),
+      is_online_(true),
+      should_update_when_online_(false),
       ALLOW_THIS_IN_INITIALIZER_LIST(weak_ptr_factory_(this)) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  net::NetworkChangeNotifier::AddConnectionTypeObserver(this);
+  is_online_ = !net::NetworkChangeNotifier::IsOffline();
 }
 
 GoogleContactStore::~GoogleContactStore() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   weak_ptr_factory_.InvalidateWeakPtrs();
+  net::NetworkChangeNotifier::RemoveConnectionTypeObserver(this);
   DestroyDatabase();
 }
 
@@ -129,6 +142,17 @@ void GoogleContactStore::RemoveObserver(ContactStoreObserver* observer) {
   observers_.RemoveObserver(observer);
 }
 
+void GoogleContactStore::OnConnectionTypeChanged(
+    net::NetworkChangeNotifier::ConnectionType type) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  bool was_online = is_online_;
+  is_online_ = (type != net::NetworkChangeNotifier::CONNECTION_NONE);
+  if (!was_online && is_online_ && should_update_when_online_) {
+    should_update_when_online_ = false;
+    UpdateContacts();
+  }
+}
+
 base::Time GoogleContactStore::GetCurrentTime() const {
   return !current_time_for_testing_.is_null() ?
          current_time_for_testing_ :
@@ -145,6 +169,14 @@ void GoogleContactStore::DestroyDatabase() {
 
 void GoogleContactStore::UpdateContacts() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  // If we're offline, defer the update.
+  if (!is_online_) {
+    VLOG(1) << "Deferring contact update due to offline state";
+    should_update_when_online_ = true;
+    return;
+  }
+
   base::Time min_update_time;
   base::TimeDelta time_since_last_update =
       last_successful_update_start_time_.is_null() ?

chrome/browser/chromeos/contacts/google_contact_store.h

@@ -20,6 +20,7 @@
 #include "base/stl_util.h"
 #include "base/time.h"
 #include "base/timer.h"
+#include "net/base/network_change_notifier.h"
 
 class Profile;
 
@@ -34,7 +35,9 @@ class ContactDatabaseInterface;
 class UpdateMetadata;
 
 // A collection of contacts from a Google account.
-class GoogleContactStore : public ContactStore {
+class GoogleContactStore
+    : public ContactStore,
+      public net::NetworkChangeNotifier::ConnectionTypeObserver {
  public:
   class TestAPI {
    public:
@@ -56,6 +59,9 @@ class GoogleContactStore : public ContactStore {
     // Triggers an update, similar to what happens when the update timer fires.
     void DoUpdate();
 
+    // Notifies the store that the system has gone online or offline.
+    void NotifyAboutNetworkStateChange(bool online);
+
    private:
     GoogleContactStore* store_;  // not owned
 
@@ -73,6 +79,10 @@ class GoogleContactStore : public ContactStore {
   virtual void AddObserver(ContactStoreObserver* observer) OVERRIDE;
   virtual void RemoveObserver(ContactStoreObserver* observer) OVERRIDE;
 
+  // net::NetworkChangeNotifier::ConnectionTypeObserver implementation:
+  virtual void OnConnectionTypeChanged(
+      net::NetworkChangeNotifier::ConnectionType type) OVERRIDE;
+
  private:
   // Map from a contact's Google-assigned ID to the contact itself.
   typedef std::map<std::string, Contact*> ContactMap;
@@ -150,6 +160,14 @@ class GoogleContactStore : public ContactStore {
   // fails.
   base::TimeDelta update_delay_on_next_failure_;
 
+  // Do we believe that it's likely that we'll be able to make network
+  // connections?
+  bool is_online_;
+
+  // Should we call UpdateContacts() when |is_online_| becomes true?  Set when
+  // UpdateContacts() is called while we're offline.
+  bool should_update_when_online_;
+
   // If non-null, used in place of base::Time::Now() when the current time is
   // needed.
   base::Time current_time_for_testing_;

chrome/browser/chromeos/contacts/google_contact_store_unittest.cc

@@ -17,6 +17,7 @@
 #include "chrome/browser/chromeos/gdata/gdata_util.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/test/test_browser_thread.h"
+#include "net/base/network_change_notifier.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using content::BrowserThread;
@@ -55,7 +56,9 @@ class GoogleContactStoreTest : public testing::Test {
  protected:
   // testing::Test implementation.
   virtual void SetUp() OVERRIDE {
-    testing::Test::SetUp();
+    // Create a mock NetworkChangeNotifier so the store won't be notified about
+    // changes to the system's actual network state.
+    network_change_notifier_.reset(net::NetworkChangeNotifier::CreateMock());
     profile_.reset(new TestingProfile);
 
     store_.reset(new GoogleContactStore(profile_.get()));
@@ -74,6 +77,7 @@ class GoogleContactStoreTest : public testing::Test {
   content::TestBrowserThread ui_thread_;
 
   TestContactStoreObserver observer_;
+  scoped_ptr<net::NetworkChangeNotifier> network_change_notifier_;
   scoped_ptr<TestingProfile> profile_;
   scoped_ptr<GoogleContactStore> store_;
   scoped_ptr<GoogleContactStore::TestAPI> test_api_;
@@ -377,5 +381,35 @@ TEST_F(GoogleContactStoreTest, HandleDatabaseInitFailure) {
   EXPECT_TRUE(test_api_->update_scheduled());
 }
 
+TEST_F(GoogleContactStoreTest, AvoidUpdatesWhenOffline) {
+  EXPECT_EQ(0, gdata_service_->num_download_requests());
+
+  // Notify the store that we're offline.  Init() shouldn't attempt an update
+  // and the update timer shouldn't be running.
+  test_api_->NotifyAboutNetworkStateChange(false);
+  store_->Init();
+  EXPECT_EQ(0, gdata_service_->num_download_requests());
+  EXPECT_FALSE(test_api_->update_scheduled());
+
+  // We should do an update and schedule further updates as soon as we go
+  // online.
+  gdata_service_->reset_stats();
+  test_api_->NotifyAboutNetworkStateChange(true);
+  EXPECT_EQ(1, gdata_service_->num_download_requests());
+  EXPECT_TRUE(test_api_->update_scheduled());
+
+  // If we call DoUpdate() to mimic the code path that's used for a timer-driven
+  // update while we're offline, we should again defer the update.
+  gdata_service_->reset_stats();
+  test_api_->NotifyAboutNetworkStateChange(false);
+  test_api_->DoUpdate();
+  EXPECT_EQ(0, gdata_service_->num_download_requests());
+
+  // When we're back online, the update should happen.
+  gdata_service_->reset_stats();
+  test_api_->NotifyAboutNetworkStateChange(true);
+  EXPECT_EQ(1, gdata_service_->num_download_requests());
+}
+
 }  // namespace test
 }  // namespace contacts

chrome/browser/chromeos/gdata/gdata_contacts_service_stub.cc

@@ -14,7 +14,8 @@ using content::BrowserThread;
 namespace gdata {
 
 GDataContactsServiceStub::GDataContactsServiceStub()
-    : download_should_succeed_(true) {
+    : num_download_requests_(0),
+      download_should_succeed_(true) {
 }
 
 GDataContactsServiceStub::~GDataContactsServiceStub() {
@@ -35,6 +36,7 @@ void GDataContactsServiceStub::DownloadContacts(
     FailureCallback failure_callback,
     const base::Time& min_update_time) {
   CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  num_download_requests_++;
 
   if (!download_should_succeed_) {
     failure_callback.Run();

chrome/browser/chromeos/gdata/gdata_contacts_service_stub.h

@@ -24,6 +24,8 @@ class GDataContactsServiceStub : public GDataContactsServiceInterface {
   GDataContactsServiceStub();
   virtual ~GDataContactsServiceStub();
 
+  int num_download_requests() const { return num_download_requests_; }
+  void reset_stats() { num_download_requests_ = 0; }
   void set_download_should_succeed(bool succeed) {
     download_should_succeed_ = succeed;
   }
@@ -40,6 +42,9 @@ class GDataContactsServiceStub : public GDataContactsServiceInterface {
                                 const base::Time& min_update_time) OVERRIDE;
 
  private:
+  // How many times has DownloadContacts() been called?
+  int num_download_requests_;
+
   // Should calls to DownloadContacts() succeed?
   bool download_should_succeed_;