Skip to content

Commit

Permalink
Merge pull request #973 from brave/sync-offline
Browse files Browse the repository at this point in the history
Handle offline cases for sync
  • Loading branch information
darkdh authored Nov 29, 2018
2 parents f5720e9 + b58844b commit af8537b
Show file tree
Hide file tree
Showing 5 changed files with 99 additions and 6 deletions.
1 change: 1 addition & 0 deletions components/brave_sync/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ source_set("js_sync_lib_impl") {
"//components/pref_registry",
"//content/public/browser",
"//extensions/browser",
"//services/network/public/cpp",
"//ui/base",
]
}
Expand Down
33 changes: 30 additions & 3 deletions components/brave_sync/brave_sync_service_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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_;
}
Expand All @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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);
}
Expand Down
10 changes: 8 additions & 2 deletions components/brave_sync/brave_sync_service_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
60 changes: 59 additions & 1 deletion components/brave_sync/brave_sync_service_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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:
Expand All @@ -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 {}
Expand Down Expand Up @@ -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 {
Expand All @@ -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(); }
Expand All @@ -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) {
Expand Down Expand Up @@ -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
Expand All @@ -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(
Expand Down
1 change: 1 addition & 0 deletions components/brave_sync/extension/background.js
Original file line number Diff line number Diff line change
Expand Up @@ -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":
Expand Down

0 comments on commit af8537b

Please sign in to comment.