Merge pull request #973 from brave/sync-offline

Handle offline cases for sync

components/brave_sync/BUILD.gn

@@ -30,6 +30,7 @@ source_set("js_sync_lib_impl") {
     "//components/pref_registry",
     "//content/public/browser",
     "//extensions/browser",
+    "//services/network/public/cpp",
     "//ui/base",
   ]
 }

components/brave_sync/brave_sync_service_impl.cc

@@ -21,6 +21,7 @@
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
 #include "components/bookmarks/browser/bookmark_model.h"
+#include "content/public/browser/network_service_instance.h"
 
 namespace brave_sync {
 
@@ -84,6 +85,7 @@ BraveSyncServiceImpl::BraveSyncServiceImpl(Profile* profile) :
         sync_prefs_.get())),
     timer_(std::make_unique<base::RepeatingTimer>()),
     unsynced_send_interval_(base::TimeDelta::FromMinutes(10)) {
+  content::GetNetworkConnectionTracker()->AddNetworkConnectionObserver(this);
 
   // Moniter syncs prefs required in GetSettingsAndDevices
   profile_pref_change_registrar_.Init(profile->GetPrefs());
@@ -118,12 +120,25 @@ BraveSyncServiceImpl::BraveSyncServiceImpl(Profile* profile) :
   }
 }
 
-BraveSyncServiceImpl::~BraveSyncServiceImpl() {}
+BraveSyncServiceImpl::~BraveSyncServiceImpl() {
+  content::GetNetworkConnectionTracker()->RemoveNetworkConnectionObserver(this);
+}
 
 BraveSyncClient* BraveSyncServiceImpl::GetSyncClient() {
   return sync_client_.get();
 }
 
+void BraveSyncServiceImpl::OnConnectionChanged(
+    network::mojom::ConnectionType type) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  if (type == network::mojom::ConnectionType::CONNECTION_NONE) {
+    if (initializing_) {
+      // TODO(cezaraugusto): ERR_SYNC_NO_INTERNET in #971
+      OnSyncSetupError("network connection is currently unavailable");
+    }
+  }
+}
+
 bool BraveSyncServiceImpl::IsSyncConfigured() {
   return sync_configured_;
 }
@@ -142,6 +157,12 @@ void BraveSyncServiceImpl::Shutdown() {
 void BraveSyncServiceImpl::OnSetupSyncHaveCode(const std::string& sync_words,
     const std::string& device_name) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  if (content::GetNetworkConnectionTracker()->IsOffline()) {
+    // TODO(cezaraugusto): ERR_SYNC_NO_INTERNET in #971
+    OnSyncSetupError("network connection is currently unavailable");
+    return;
+  }
+
   if (sync_words.empty() || device_name.empty()) {
     OnSyncSetupError("missing sync words or device name");
     return;
@@ -167,6 +188,12 @@ void BraveSyncServiceImpl::OnSetupSyncHaveCode(const std::string& sync_words,
 void BraveSyncServiceImpl::OnSetupSyncNewToSync(
     const std::string& device_name) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  if (content::GetNetworkConnectionTracker()->IsOffline()) {
+    // TODO(cezaraugusto): ERR_SYNC_NO_INTERNET in #971
+    OnSyncSetupError("network connection is currently unavailable");
+    return;
+  }
+
   if (device_name.empty()) {
     OnSyncSetupError("missing device name");
     return;
@@ -287,9 +314,9 @@ void BraveSyncServiceImpl::OnSyncDebug(const std::string& message) {
 }
 
 void BraveSyncServiceImpl::OnSyncSetupError(const std::string& error) {
-  initializing_ = false;
-  if (!sync_initialized_) {
+  if (initializing_) {
     sync_prefs_->Clear();
+    initializing_ = false;
   }
   NotifySyncSetupError(error);
 }

components/brave_sync/brave_sync_service_impl.h

@@ -12,6 +12,7 @@
 #include "base/time/time.h"
 #include "brave/components/brave_sync/brave_sync_service.h"
 #include "brave/components/brave_sync/client/brave_sync_client.h"
+#include "services/network/public/cpp/network_connection_tracker.h"
 #include "components/prefs/pref_change_registrar.h"
 
 FORWARD_DECLARE_TEST(BraveSyncServiceTest, BookmarkAdded);
@@ -52,8 +53,10 @@ using SendDeviceSyncRecordCallback = base::OnceCallback<void(const int,
                                                            const std::string&,
                                                            const std::string&)>;
 
-class BraveSyncServiceImpl : public BraveSyncService,
-                             public SyncMessageHandler {
+class BraveSyncServiceImpl
+    : public BraveSyncService,
+      public SyncMessageHandler,
+      public network::NetworkConnectionTracker::NetworkConnectionObserver {
  public:
   BraveSyncServiceImpl(Profile *profile);
   ~BraveSyncServiceImpl() override;
@@ -82,6 +85,9 @@ class BraveSyncServiceImpl : public BraveSyncService,
 
   BraveSyncClient* GetSyncClient() override;
 
+  // network::NetworkConnectionTracker::NetworkConnectionObserver:
+  void OnConnectionChanged(network::mojom::ConnectionType type) override;
+
  private:
   FRIEND_TEST_ALL_PREFIXES(::BraveSyncServiceTest, BookmarkAdded);
   FRIEND_TEST_ALL_PREFIXES(::BraveSyncServiceTest, BookmarkDeleted);

components/brave_sync/brave_sync_service_unittest.cc

@@ -22,7 +22,9 @@
 #include "components/bookmarks/browser/bookmark_utils.h"
 #include "components/bookmarks/test/test_bookmark_client.h"
 #include "components/prefs/pref_service.h"
+#include "content/public/browser/network_service_instance.h"
 #include "content/public/test/test_browser_thread_bundle.h"
+#include "services/network/test/test_network_connection_tracker.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -87,6 +89,8 @@
 using testing::_;
 using testing::AtLeast;
 using namespace brave_sync;
+using network::TestNetworkConnectionTracker;
+using network::mojom::ConnectionType;
 
 class MockBraveSyncServiceObserver : public BraveSyncServiceObserver {
  public:
@@ -97,7 +101,9 @@ class MockBraveSyncServiceObserver : public BraveSyncServiceObserver {
   MOCK_METHOD2(OnHaveSyncWords, void(BraveSyncService*, const std::string&));
 };
 
-class BraveSyncServiceTest : public testing::Test {
+class BraveSyncServiceTest
+    : public testing::Test,
+      public network::NetworkConnectionTracker::NetworkConnectionObserver {
  public:
   BraveSyncServiceTest() {}
   ~BraveSyncServiceTest() override {}
@@ -128,6 +134,12 @@ class BraveSyncServiceTest : public testing::Test {
     observer_.reset(new MockBraveSyncServiceObserver);
     sync_service_->AddObserver(observer_.get());
     EXPECT_TRUE(sync_service_ != NULL);
+
+    // TestNetworkConnectionTracker::CreateInstance has been called in
+    // TestingBrowserProcess
+    TestNetworkConnectionTracker* tracker =
+      TestNetworkConnectionTracker::GetInstance();
+    tracker->SetConnectionType(ConnectionType::CONNECTION_UNKNOWN);
   }
 
   void TearDown() override {
@@ -136,6 +148,11 @@ class BraveSyncServiceTest : public testing::Test {
     profile_.reset();
   }
 
+  // network::NetworkConnectionTracker::NetworkConnectionObserver:
+  void OnConnectionChanged(ConnectionType type) override {
+    wait_for_network_type_change_.QuitWhenIdle();
+  }
+
   void BookmarkAddedImpl();
 
   Profile* profile() { return profile_.get(); }
@@ -155,6 +172,8 @@ class BraveSyncServiceTest : public testing::Test {
   std::unique_ptr<MockBraveSyncServiceObserver> observer_;
 
   base::ScopedTempDir temp_dir_;
+ public:
+  base::RunLoop wait_for_network_type_change_;
 };
 
 TEST_F(BraveSyncServiceTest, SetSyncEnabled) {
@@ -244,6 +263,18 @@ TEST_F(BraveSyncServiceTest, OnSetupSyncHaveCode) {
        brave_sync::prefs::kSyncEnabled));
 }
 
+TEST_F(BraveSyncServiceTest, OnSetupSyncHaveCode_Offline) {
+  TestNetworkConnectionTracker* tracker =
+    TestNetworkConnectionTracker::GetInstance();
+  tracker->SetConnectionType(ConnectionType::CONNECTION_NONE);
+  EXPECT_CALL(*observer(), OnSyncSetupError(sync_service(), _));
+  sync_service()->OnSetupSyncHaveCode("word1 word2 word3", "test_device");
+  EXPECT_FALSE(profile()->GetPrefs()->GetBoolean(
+       brave_sync::prefs::kSyncEnabled));
+  // Restore network connection
+  tracker->SetConnectionType(ConnectionType::CONNECTION_UNKNOWN);
+}
+
 TEST_F(BraveSyncServiceTest, OnSetupSyncNewToSync) {
   EXPECT_CALL(*sync_client(), OnSyncEnabledChanged);
   // Expecting sync state changed twice: for enabled state and for device name
@@ -253,6 +284,33 @@ TEST_F(BraveSyncServiceTest, OnSetupSyncNewToSync) {
        brave_sync::prefs::kSyncEnabled));
 }
 
+TEST_F(BraveSyncServiceTest, OnSetupSyncNewToSync_Offline) {
+  TestNetworkConnectionTracker* tracker =
+    TestNetworkConnectionTracker::GetInstance();
+  tracker->SetConnectionType(ConnectionType::CONNECTION_NONE);
+  EXPECT_CALL(*observer(), OnSyncSetupError(sync_service(), _));
+  sync_service()->OnSetupSyncNewToSync("test_device");
+  EXPECT_FALSE(profile()->GetPrefs()->GetBoolean(
+       brave_sync::prefs::kSyncEnabled));
+  // Restore network connection
+  tracker->SetConnectionType(ConnectionType::CONNECTION_UNKNOWN);
+}
+
+TEST_F(BraveSyncServiceTest, OnConnectionChanged_After_OnSetupSyncNewToSync) {
+  content::GetNetworkConnectionTracker()->AddNetworkConnectionObserver(this);
+  TestNetworkConnectionTracker* tracker =
+    TestNetworkConnectionTracker::GetInstance();
+  EXPECT_CALL(*observer(), OnSyncSetupError(sync_service(), _));
+  sync_service()->OnSetupSyncNewToSync("test_device");
+  tracker->SetConnectionType(ConnectionType::CONNECTION_NONE);
+  wait_for_network_type_change_.Run();
+  content::GetNetworkConnectionTracker()->RemoveNetworkConnectionObserver(this);
+  EXPECT_FALSE(profile()->GetPrefs()->GetBoolean(
+       brave_sync::prefs::kSyncEnabled));
+  // Restore network connection
+  tracker->SetConnectionType(ConnectionType::CONNECTION_UNKNOWN);
+}
+
 TEST_F(BraveSyncServiceTest, GetSettingsAndDevices) {
   // The test absorbs OnSetupSyncNewToSync test
   auto callback1 = base::BindRepeating(

components/brave_sync/extension/background.js

@@ -231,6 +231,7 @@ class InjectedObject {
         break;
       case "sync-setup-error":
         console.log(`"sync-setup-error" error=${JSON.stringify(arg1)}`);
+        // TODO(cezaraugusto): ERR_SYNC_INIT_FAILED without arg in #971
         chrome.braveSync.syncSetupError(arg1/*error*/);
         break;
       case "sync-debug":