Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Uplift to 0.61.x of #1643, Sync revert network connection tracker #1727

Merged
merged 1 commit into from
Feb 22, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 51 additions & 48 deletions browser/ui/webui/brave_webui_source.cc

Large diffs are not rendered by default.

22 changes: 0 additions & 22 deletions components/brave_sync/brave_sync_service_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
#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"
#include "net/base/network_interfaces.h"

namespace brave_sync {
Expand Down Expand Up @@ -92,8 +91,6 @@ 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());
profile_pref_change_registrar_.Add(
Expand Down Expand Up @@ -128,23 +125,12 @@ BraveSyncServiceImpl::BraveSyncServiceImpl(Profile* profile) :
}

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_) {
OnSyncSetupError("ERR_SYNC_NO_INTERNET");
}
}
}

bool BraveSyncServiceImpl::IsSyncConfigured() {
return sync_configured_;
}
Expand All @@ -163,10 +149,6 @@ 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()) {
OnSyncSetupError("ERR_SYNC_NO_INTERNET");
return;
}

if (sync_words.empty()) {
OnSyncSetupError("ERR_SYNC_WRONG_WORDS");
Expand All @@ -193,10 +175,6 @@ 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()) {
OnSyncSetupError("ERR_SYNC_NO_INTERNET");
return;
}

if (initializing_) {
NotifyLogMessage("currently initializing");
Expand Down
26 changes: 12 additions & 14 deletions components/brave_sync/brave_sync_service_impl.h
Original file line number Diff line number Diff line change
@@ -1,18 +1,20 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
/* Copyright 2016 The Brave Authors. All rights reserved.
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */

#ifndef BRAVE_COMPONENTS_SYNC_BRAVE_SYNC_SERVICE_IMPL_H_
#define BRAVE_COMPONENTS_SYNC_BRAVE_SYNC_SERVICE_IMPL_H_
#ifndef BRAVE_COMPONENTS_BRAVE_SYNC_BRAVE_SYNC_SERVICE_IMPL_H_
#define BRAVE_COMPONENTS_BRAVE_SYNC_BRAVE_SYNC_SERVICE_IMPL_H_

#include <map>
#include <memory>
#include <string>

#include "base/macros.h"
#include "base/scoped_observer.h"
#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 @@ -57,10 +59,9 @@ using SendDeviceSyncRecordCallback = base::OnceCallback<void(const int,

class BraveSyncServiceImpl
: public BraveSyncService,
public SyncMessageHandler,
public network::NetworkConnectionTracker::NetworkConnectionObserver {
public SyncMessageHandler {
public:
BraveSyncServiceImpl(Profile *profile);
explicit BraveSyncServiceImpl(Profile *profile);
~BraveSyncServiceImpl() override;

// KeyedService overrides
Expand All @@ -87,9 +88,6 @@ class BraveSyncServiceImpl

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 Expand Up @@ -165,7 +163,7 @@ class BraveSyncServiceImpl
void GetExistingHistoryObjects(
const RecordsList &records,
const base::Time &last_record_time_stamp,
const bool is_truncated );
const bool is_truncated);

void NotifyLogMessage(const std::string& message);
void NotifySyncSetupError(const std::string& error);
Expand Down Expand Up @@ -193,7 +191,7 @@ class BraveSyncServiceImpl

std::unique_ptr<brave_sync::Settings> settings_;

Profile *profile_;
Profile* profile_;
std::unique_ptr<brave_sync::prefs::Prefs> sync_prefs_;

std::unique_ptr<BookmarkChangeProcessor> bookmark_change_processor_;
Expand All @@ -212,6 +210,6 @@ class BraveSyncServiceImpl
DISALLOW_COPY_AND_ASSIGN(BraveSyncServiceImpl);
};

} // namespace brave_sync
} // namespace brave_sync

#endif //BRAVE_COMPONENTS_SYNC_BRAVE_SYNC_SERVICE_IMPL_H_
#endif // BRAVE_COMPONENTS_BRAVE_SYNC_BRAVE_SYNC_SERVICE_IMPL_H_
50 changes: 1 addition & 49 deletions components/brave_sync/brave_sync_service_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,7 @@ class MockBraveSyncServiceObserver : public BraveSyncServiceObserver {
MOCK_METHOD2(OnHaveSyncWords, void(BraveSyncService*, const std::string&));
};

class BraveSyncServiceTest
: public testing::Test,
public network::NetworkConnectionTracker::NetworkConnectionObserver {
class BraveSyncServiceTest : public testing::Test {
public:
BraveSyncServiceTest() {}
~BraveSyncServiceTest() override {}
Expand Down Expand Up @@ -158,11 +156,6 @@ class BraveSyncServiceTest
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 @@ -182,8 +175,6 @@ class BraveSyncServiceTest
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 @@ -286,18 +277,6 @@ TEST_F(BraveSyncServiceTest, OnSetupSyncHaveCode_EmptyDeviceName) {
brave_sync::prefs::kSyncDeviceName), net::GetHostName());
}

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 @@ -307,18 +286,6 @@ 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, OnSetupSyncNewToSync_EmptyDeviceName) {
EXPECT_CALL(*sync_client(), OnSyncEnabledChanged);
// Expecting sync state changed twice: for enabled state and for device name
Expand All @@ -330,21 +297,6 @@ TEST_F(BraveSyncServiceTest, OnSetupSyncNewToSync_EmptyDeviceName) {
brave_sync::prefs::kSyncDeviceName), net::GetHostName());
}

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
8 changes: 0 additions & 8 deletions components/brave_sync/ui/components/enabledContent.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -195,14 +195,6 @@ export default class SyncEnabledContent extends React.PureComponent<Props, State
return (
<EnabledContent>
<Main>
{
syncData.error === 'ERR_SYNC_NO_INTERNET'
? <AlertBox okString={getLocale('ok')} onClickOk={this.onUserNoticedError}>
<Title>{getLocale('errorNoInternetTitle')}</Title>
<SubTitle>{getLocale('errorNoInternetDescription')}</SubTitle>
</AlertBox>
: null
}
{
syncData.error === 'ERR_SYNC_INIT_FAILED'
? <AlertBox okString={getLocale('ok')} onClickOk={this.onUserNoticedError}>
Expand Down
8 changes: 0 additions & 8 deletions components/brave_sync/ui/components/modals/deviceType.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -80,14 +80,6 @@ export default class DeviceTypeModal extends React.PureComponent<Props, State> {

return (
<Modal id='deviceTypeModal' displayCloseButton={false} size='small'>
{
syncData.error === 'ERR_SYNC_NO_INTERNET'
? <AlertBox okString={getLocale('ok')} onClickOk={this.onUserNoticedError}>
<Title>{getLocale('errorNoInternetTitle')}</Title>
<SubTitle>{getLocale('errorNoInternetDescription')}</SubTitle>
</AlertBox>
: null
}
{
syncData.error === 'ERR_SYNC_INIT_FAILED'
? <AlertBox okString={getLocale('ok')} onClickOk={this.onUserNoticedError}>
Expand Down
8 changes: 0 additions & 8 deletions components/brave_sync/ui/components/modals/enterSyncCode.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -117,14 +117,6 @@ export default class EnterSyncCodeModal extends React.PureComponent<Props, State
</AlertBox>
: null
}
{
syncData.error === 'ERR_SYNC_NO_INTERNET'
? <AlertBox okString={getLocale('ok')} onClickOk={this.onUserNoticedError}>
<Title>{getLocale('errorNoInternetTitle')}</Title>
<SubTitle>{getLocale('errorNoInternetDescription')}</SubTitle>
</AlertBox>
: null
}
{
syncData.error === 'ERR_SYNC_INIT_FAILED'
? <AlertBox okString={getLocale('ok')} onClickOk={this.onUserNoticedError}>
Expand Down
1 change: 0 additions & 1 deletion components/definitions/sync.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ declare namespace Sync {
}

export type SetupErrorType =
'ERR_SYNC_NO_INTERNET' |
'ERR_SYNC_MISSING_WORDS' |
'ERR_SYNC_WRONG_WORDS' |
'ERR_SYNC_INIT_FAILED' |
Expand Down
2 changes: 0 additions & 2 deletions components/resources/brave_components_strings.grd
Original file line number Diff line number Diff line change
Expand Up @@ -530,8 +530,6 @@
<!-- WebUI brave sync resources: Errors -->
<message name="IDS_BRAVE_SYNC_ERROR_WRONG_CODE_TITLE" desc="Error message in dialog for when sync words are invalid (title)">Invalid sync code.</message>
<message name="IDS_BRAVE_SYNC_ERROR_WRONG_CODE_DESCRIPTION" desc="Error message in dialog for when sync words are invalid (description)">Please try again.</message>
<message name="IDS_BRAVE_SYNC_ERROR_NO_INTERNET_TITLE" desc="Error message in dialog for when user is offline (title)">No internet connection.</message>
<message name="IDS_BRAVE_SYNC_ERROR_NO_INTERNET_DESCRIPTION" desc="Error message in dialog for when user is offline (description)">Please try again when your connection is available.</message>
<message name="IDS_BRAVE_SYNC_ERROR_MISSING_DEVICE_NAME_TITLE" desc="Error message in dialog for when device name is missing (title)">Device name is required.</message>
<message name="IDS_BRAVE_SYNC_ERROR_MISSING_SYNC_CODE_TITLE" desc="Error message in dialog for when device name is missing (description)">Please add a sync code.</message>
<message name="IDS_BRAVE_SYNC_ERROR_INIT_FAILED_TITLE" desc="Error message in dialog for when a Sync error happens in the back-end">Couldn't contact Sync servers</message>
Expand Down