-
Notifications
You must be signed in to change notification settings - Fork 6.6k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Introduce Safe Browsing sync observer interface and implementation.
All sync dependencies under safe_browsing need to live within /core/browser/sync/, so introduce an interface under core/browser/ and have it implemented within /core/browser/sync/. This observer will be injected to verdict cache manager to observe sync change events. Bug: 1268158 Change-Id: I093ffbf81ff4c57a300a4da533ddfc85793d7e5d Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3564941 Reviewed-by: Daniel Rubery <drubery@chromium.org> Commit-Queue: Xinghui Lu <xinghuilu@chromium.org> Cr-Commit-Position: refs/heads/main@{#989071}
- Loading branch information
Xinghui Lu
authored and
Chromium LUCI CQ
committed
Apr 5, 2022
1 parent
fb8b122
commit 073c5df
Showing
6 changed files
with
191 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
26 changes: 26 additions & 0 deletions
26
components/safe_browsing/core/browser/safe_browsing_sync_observer.h
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
// Copyright 2022 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. | ||
|
||
#ifndef COMPONENTS_SAFE_BROWSING_CORE_BROWSER_SAFE_BROWSING_SYNC_OBSERVER_H_ | ||
#define COMPONENTS_SAFE_BROWSING_CORE_BROWSER_SAFE_BROWSING_SYNC_OBSERVER_H_ | ||
|
||
#include "base/callback.h" | ||
|
||
namespace safe_browsing { | ||
|
||
// Interface used to observe sync events. | ||
class SafeBrowsingSyncObserver { | ||
public: | ||
using Callback = base::RepeatingCallback<void()>; | ||
|
||
virtual ~SafeBrowsingSyncObserver() = default; | ||
|
||
// Starts to observe sync state changed events. `callback` will be invoked | ||
// if sync state has changed. `callback` can be invoked multiple times. | ||
virtual void ObserveSyncStateChanged(Callback callback) = 0; | ||
}; | ||
|
||
} // namespace safe_browsing | ||
|
||
#endif // COMPONENTS_SAFE_BROWSING_CORE_BROWSER_SAFE_BROWSING_SYNC_OBSERVER_H_ |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
39 changes: 39 additions & 0 deletions
39
components/safe_browsing/core/browser/sync/safe_browsing_sync_observer_impl.cc
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
// Copyright 2022 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 "components/safe_browsing/core/browser/sync/safe_browsing_sync_observer_impl.h" | ||
|
||
namespace safe_browsing { | ||
|
||
SafeBrowsingSyncObserverImpl::SafeBrowsingSyncObserverImpl( | ||
syncer::SyncService* sync_service) { | ||
// sync can be null in tests and in Incognito. | ||
if (sync_service) { | ||
sync_service_observer_.Observe(sync_service); | ||
is_sync_feature_enabled_ = sync_service->IsSyncFeatureEnabled(); | ||
} | ||
} | ||
|
||
SafeBrowsingSyncObserverImpl::~SafeBrowsingSyncObserverImpl() = default; | ||
|
||
void SafeBrowsingSyncObserverImpl::ObserveSyncStateChanged(Callback callback) { | ||
callback_ = std::move(callback); | ||
} | ||
|
||
void SafeBrowsingSyncObserverImpl::OnStateChanged(syncer::SyncService* sync) { | ||
bool is_sync_feature_enabled = sync->IsSyncFeatureEnabled(); | ||
if (is_sync_feature_enabled == is_sync_feature_enabled_) { | ||
return; | ||
} | ||
is_sync_feature_enabled_ = is_sync_feature_enabled; | ||
if (!callback_.is_null()) { | ||
callback_.Run(); | ||
} | ||
} | ||
|
||
void SafeBrowsingSyncObserverImpl::OnSyncShutdown(syncer::SyncService* sync) { | ||
sync_service_observer_.Reset(); | ||
} | ||
|
||
} // namespace safe_browsing |
44 changes: 44 additions & 0 deletions
44
components/safe_browsing/core/browser/sync/safe_browsing_sync_observer_impl.h
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
// Copyright 2022 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. | ||
|
||
#ifndef COMPONENTS_SAFE_BROWSING_CORE_BROWSER_SYNC_SAFE_BROWSING_SYNC_OBSERVER_IMPL_H_ | ||
#define COMPONENTS_SAFE_BROWSING_CORE_BROWSER_SYNC_SAFE_BROWSING_SYNC_OBSERVER_IMPL_H_ | ||
|
||
#include "base/scoped_observation.h" | ||
#include "components/safe_browsing/core/browser/safe_browsing_sync_observer.h" | ||
#include "components/sync/driver/sync_service.h" | ||
#include "components/sync/driver/sync_service_observer.h" | ||
|
||
namespace syncer { | ||
class SyncService; | ||
} | ||
|
||
namespace safe_browsing { | ||
|
||
// This class observes sync events and notifies the observer. | ||
class SafeBrowsingSyncObserverImpl : public SafeBrowsingSyncObserver, | ||
public syncer::SyncServiceObserver { | ||
public: | ||
explicit SafeBrowsingSyncObserverImpl(syncer::SyncService* sync_service); | ||
|
||
~SafeBrowsingSyncObserverImpl() override; | ||
|
||
// SafeBrowsingSyncObserver: | ||
void ObserveSyncStateChanged(Callback callback) override; | ||
|
||
// syncer::SyncServiceObserver: | ||
void OnStateChanged(syncer::SyncService* sync) override; | ||
void OnSyncShutdown(syncer::SyncService* sync) override; | ||
|
||
private: | ||
Callback callback_; | ||
bool is_sync_feature_enabled_ = false; | ||
|
||
base::ScopedObservation<syncer::SyncService, syncer::SyncServiceObserver> | ||
sync_service_observer_{this}; | ||
}; | ||
|
||
} // namespace safe_browsing | ||
|
||
#endif // COMPONENTS_SAFE_BROWSING_CORE_BROWSER_SYNC_SAFE_BROWSING_SYNC_OBSERVER_IMPL_H_ |
72 changes: 72 additions & 0 deletions
72
components/safe_browsing/core/browser/sync/safe_browsing_sync_observer_impl_unittest.cc
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,72 @@ | ||
// Copyright 2022 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 "components/safe_browsing/core/browser/sync/safe_browsing_sync_observer_impl.h" | ||
|
||
#include "base/bind.h" | ||
#include "base/test/task_environment.h" | ||
#include "components/sync/driver/test_sync_service.h" | ||
#include "testing/gtest/include/gtest/gtest.h" | ||
#include "testing/platform_test.h" | ||
|
||
namespace safe_browsing { | ||
|
||
class SafeBrowsingSyncObserverImplTest : public PlatformTest { | ||
public: | ||
SafeBrowsingSyncObserverImplTest() = default; | ||
|
||
void SetUp() override { | ||
sync_service_.SetDisableReasons( | ||
syncer::SyncService::DISABLE_REASON_NOT_SIGNED_IN); | ||
} | ||
|
||
protected: | ||
void EnableSync() { | ||
sync_service_.SetDisableReasons({}); | ||
sync_service_.FireStateChanged(); | ||
} | ||
|
||
void DisableSync() { | ||
sync_service_.SetDisableReasons( | ||
syncer::SyncService::DISABLE_REASON_NOT_SIGNED_IN); | ||
sync_service_.FireStateChanged(); | ||
} | ||
|
||
base::test::TaskEnvironment task_environment_; | ||
syncer::TestSyncService sync_service_; | ||
}; | ||
|
||
TEST_F(SafeBrowsingSyncObserverImplTest, ObserveSyncState) { | ||
SafeBrowsingSyncObserverImpl observer(&sync_service_); | ||
int invoke_cnt = 0; | ||
observer.ObserveSyncStateChanged(base::BindRepeating( | ||
[](int* invoke_cnt) { (*invoke_cnt)++; }, &invoke_cnt)); | ||
|
||
EnableSync(); | ||
EXPECT_EQ(invoke_cnt, 1); | ||
|
||
DisableSync(); | ||
EXPECT_EQ(invoke_cnt, 2); | ||
|
||
DisableSync(); | ||
// Not invoked since the state didn't change. | ||
EXPECT_EQ(invoke_cnt, 2); | ||
} | ||
|
||
TEST_F(SafeBrowsingSyncObserverImplTest, NullSyncService) { | ||
SafeBrowsingSyncObserverImpl observer(nullptr); | ||
int invoke_cnt = 0; | ||
observer.ObserveSyncStateChanged(base::BindRepeating( | ||
[](int* invoke_cnt) { (*invoke_cnt)++; }, &invoke_cnt)); | ||
|
||
EnableSync(); | ||
EXPECT_EQ(invoke_cnt, 0); | ||
} | ||
|
||
TEST_F(SafeBrowsingSyncObserverImplTest, NullCallback) { | ||
SafeBrowsingSyncObserverImpl observer(&sync_service_); | ||
EnableSync(); | ||
} | ||
|
||
} // namespace safe_browsing |