diff --git a/browser/sync/BUILD.gn b/browser/sync/BUILD.gn index cea0316b9f10..13db4d313236 100644 --- a/browser/sync/BUILD.gn +++ b/browser/sync/BUILD.gn @@ -11,6 +11,7 @@ source_set("sync") { deps = [ "//base", + "//components/history/core/browser", "//components/sync/service", "//components/sync_device_info", ] diff --git a/browser/sync/brave_sync_service_impl_delegate.cc b/browser/sync/brave_sync_service_impl_delegate.cc index bca01058c0c7..1ed07bde8cde 100644 --- a/browser/sync/brave_sync_service_impl_delegate.cc +++ b/browser/sync/brave_sync_service_impl_delegate.cc @@ -12,6 +12,7 @@ #include "base/metrics/histogram_functions.h" #include "base/task/single_thread_task_runner.h" #include "brave/components/sync/service/brave_sync_service_impl.h" +#include "components/history/core/browser/history_service.h" #include "components/sync_device_info/device_info_sync_service.h" #include "components/sync_device_info/device_info_tracker.h" #include "components/sync_device_info/local_device_info_provider.h" @@ -19,8 +20,10 @@ namespace syncer { BraveSyncServiceImplDelegate::BraveSyncServiceImplDelegate( - DeviceInfoSyncService* device_info_sync_service) + DeviceInfoSyncService* device_info_sync_service, + history::HistoryService* history_service) : device_info_sync_service_(device_info_sync_service), + history_service_(history_service), weak_ptr_factory_(this) { DCHECK(device_info_sync_service_); @@ -99,4 +102,15 @@ void BraveSyncServiceImplDelegate::SetLocalDeviceAppearedCallback( local_device_appeared_callback_ = std::move(local_device_appeared_callback); } +void BraveSyncServiceImplDelegate::GetKnownToSyncHistoryCount( + base::OnceCallback)> callback) { + history_service_->GetKnownToSyncCount(base::BindOnce( + [](base::OnceCallback)> callback, + history::HistoryCountResult known_to_sync) { + std::move(callback).Run( + std::pair(known_to_sync.success, known_to_sync.count)); + }, + std::move(callback))); +} + } // namespace syncer diff --git a/browser/sync/brave_sync_service_impl_delegate.h b/browser/sync/brave_sync_service_impl_delegate.h index 34568186fd74..758dc839875e 100644 --- a/browser/sync/brave_sync_service_impl_delegate.h +++ b/browser/sync/brave_sync_service_impl_delegate.h @@ -6,14 +6,17 @@ #ifndef BRAVE_BROWSER_SYNC_BRAVE_SYNC_SERVICE_IMPL_DELEGATE_H_ #define BRAVE_BROWSER_SYNC_BRAVE_SYNC_SERVICE_IMPL_DELEGATE_H_ -#include "brave/components/sync/service/sync_service_impl_delegate.h" +#include #include "base/memory/raw_ptr.h" #include "base/memory/weak_ptr.h" #include "base/scoped_observation.h" +#include "brave/components/sync/service/sync_service_impl_delegate.h" #include "components/sync_device_info/device_info_tracker.h" -class Profile; +namespace history { +class HistoryService; +} // namespace history namespace syncer { @@ -29,7 +32,8 @@ class BraveSyncServiceImplDelegate public syncer::DeviceInfoTracker::Observer { public: explicit BraveSyncServiceImplDelegate( - DeviceInfoSyncService* device_info_sync_service); + DeviceInfoSyncService* device_info_sync_service, + history::HistoryService* history_service); ~BraveSyncServiceImplDelegate() override; void SuspendDeviceObserverForOwnReset() override; @@ -38,6 +42,9 @@ class BraveSyncServiceImplDelegate void SetLocalDeviceAppearedCallback( base::OnceCallback local_device_appeared_callback) override; + void GetKnownToSyncHistoryCount( + base::OnceCallback)> callback) override; + private: // syncer::DeviceInfoTracker::Observer: void OnDeviceInfoChange() override; @@ -54,6 +61,7 @@ class BraveSyncServiceImplDelegate device_info_observer_{this}; raw_ptr device_info_sync_service_ = nullptr; + raw_ptr history_service_ = nullptr; // This is triggered once after SetLocalDeviceAppearedCallback // when the local device first appears in the changed synced devices list diff --git a/chromium_src/chrome/browser/sync/sync_service_factory.cc b/chromium_src/chrome/browser/sync/sync_service_factory.cc index ae62180cf4bb..bbd2ade1cb89 100644 --- a/chromium_src/chrome/browser/sync/sync_service_factory.cc +++ b/chromium_src/chrome/browser/sync/sync_service_factory.cc @@ -5,13 +5,16 @@ #include "brave/browser/sync/brave_sync_service_impl_delegate.h" #include "brave/components/sync/service/brave_sync_service_impl.h" +#include "chrome/browser/history/history_service_factory.h" #include "chrome/browser/sync/device_info_sync_service_factory.h" -#define BRAVE_BUILD_SERVICE_INSTANCE_FOR \ - std::make_unique( \ - std::move(init_params), \ - std::make_unique( \ - DeviceInfoSyncServiceFactory::GetForProfile(profile))); +#define BRAVE_BUILD_SERVICE_INSTANCE_FOR \ + std::make_unique( \ + std::move(init_params), \ + std::make_unique( \ + DeviceInfoSyncServiceFactory::GetForProfile(profile), \ + HistoryServiceFactory::GetForProfile( \ + profile, ServiceAccessType::IMPLICIT_ACCESS))); #include "src/chrome/browser/sync/sync_service_factory.cc" diff --git a/chromium_src/components/history/core/browser/history_backend.cc b/chromium_src/components/history/core/browser/history_backend.cc new file mode 100644 index 000000000000..1aab3df42c2a --- /dev/null +++ b/chromium_src/components/history/core/browser/history_backend.cc @@ -0,0 +1,15 @@ +/* Copyright (c) 2024 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 https://mozilla.org/MPL/2.0/. */ + +#include "src/components/history/core/browser/history_backend.cc" + +namespace history { + +HistoryCountResult HistoryBackend::GetKnownToSyncCount() { + int count = 0; + return {db_ && db_->GetKnownToSyncCount(&count), count}; +} + +} // namespace history diff --git a/chromium_src/components/history/core/browser/history_backend.h b/chromium_src/components/history/core/browser/history_backend.h new file mode 100644 index 000000000000..57e2754b8502 --- /dev/null +++ b/chromium_src/components/history/core/browser/history_backend.h @@ -0,0 +1,17 @@ +/* Copyright (c) 2024 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 https://mozilla.org/MPL/2.0/. */ + +#ifndef BRAVE_CHROMIUM_SRC_COMPONENTS_HISTORY_CORE_BROWSER_HISTORY_BACKEND_H_ +#define BRAVE_CHROMIUM_SRC_COMPONENTS_HISTORY_CORE_BROWSER_HISTORY_BACKEND_H_ + +#define GetHistoryCount \ + GetKnownToSyncCount(); \ + HistoryCountResult GetHistoryCount + +#include "src/components/history/core/browser/history_backend.h" // IWYU pragma: export + +#undef GetHistoryCount + +#endif // BRAVE_CHROMIUM_SRC_COMPONENTS_HISTORY_CORE_BROWSER_HISTORY_BACKEND_H_ diff --git a/chromium_src/components/history/core/browser/history_service.cc b/chromium_src/components/history/core/browser/history_service.cc new file mode 100644 index 000000000000..1f26dee36973 --- /dev/null +++ b/chromium_src/components/history/core/browser/history_service.cc @@ -0,0 +1,18 @@ +/* Copyright (c) 2024 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 https://mozilla.org/MPL/2.0/. */ + +#include "src/components/history/core/browser/history_service.cc" + +namespace history { + +void HistoryService::GetKnownToSyncCount( + base::OnceCallback callback) { + backend_task_runner_->PostTaskAndReplyWithResult( + FROM_HERE, + base::BindOnce(&HistoryBackend::GetKnownToSyncCount, history_backend_), + std::move(callback)); +} + +} // namespace history diff --git a/chromium_src/components/history/core/browser/history_service.h b/chromium_src/components/history/core/browser/history_service.h index dc919d0f558f..98180ad2a2d1 100644 --- a/chromium_src/components/history/core/browser/history_service.h +++ b/chromium_src/components/history/core/browser/history_service.h @@ -15,8 +15,15 @@ class BraveHistoryQuickProviderTest; friend class ::BraveHistoryQuickProviderTest; \ void CleanupUnused +#define AddRelatedSearchesForVisit \ + GetKnownToSyncCount( \ + base::OnceCallback callback); \ + void AddRelatedSearchesForVisit + #include "src/components/history/core/browser/history_service.h" // IWYU pragma: export +#undef AddRelatedSearchesForVisit + #undef Cleanup #endif // BRAVE_CHROMIUM_SRC_COMPONENTS_HISTORY_CORE_BROWSER_HISTORY_SERVICE_H_ diff --git a/chromium_src/components/history/core/browser/visit_database.cc b/chromium_src/components/history/core/browser/visit_database.cc index cea9ec61ce37..4072701c13a1 100644 --- a/chromium_src/components/history/core/browser/visit_database.cc +++ b/chromium_src/components/history/core/browser/visit_database.cc @@ -13,3 +13,21 @@ #include "src/components/history/core/browser/visit_database.cc" #undef SOURCE_SAFARI_IMPORTED + +namespace history { + +bool VisitDatabase::GetKnownToSyncCount(int* count) { + sql::Statement statement( + GetDB().GetCachedStatement(SQL_FROM_HERE, + "SELECT COUNT(*) " + "FROM visits " + "WHERE is_known_to_sync == TRUE")); + + *count = 0; + if (statement.Step()) { + *count = statement.ColumnInt(0); + } + return true; +} + +} // namespace history diff --git a/chromium_src/components/history/core/browser/visit_database.h b/chromium_src/components/history/core/browser/visit_database.h new file mode 100644 index 000000000000..a2932e49af66 --- /dev/null +++ b/chromium_src/components/history/core/browser/visit_database.h @@ -0,0 +1,17 @@ +/* Copyright (c) 2024 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 https://mozilla.org/MPL/2.0/. */ + +#ifndef BRAVE_CHROMIUM_SRC_COMPONENTS_HISTORY_CORE_BROWSER_VISIT_DATABASE_H_ +#define BRAVE_CHROMIUM_SRC_COMPONENTS_HISTORY_CORE_BROWSER_VISIT_DATABASE_H_ + +#define GetHistoryCount \ + GetKnownToSyncCount(int* count); \ + bool GetHistoryCount + +#include "src/components/history/core/browser/visit_database.h" // IWYU pragma: export + +#undef GetHistoryCount + +#endif // BRAVE_CHROMIUM_SRC_COMPONENTS_HISTORY_CORE_BROWSER_VISIT_DATABASE_H_ diff --git a/chromium_src/ios/chrome/browser/sync/model/sync_service_factory.mm b/chromium_src/ios/chrome/browser/sync/model/sync_service_factory.mm index 64009f9faef4..cc49a0b94f9a 100644 --- a/chromium_src/ios/chrome/browser/sync/model/sync_service_factory.mm +++ b/chromium_src/ios/chrome/browser/sync/model/sync_service_factory.mm @@ -5,13 +5,16 @@ #include "brave/browser/sync/brave_sync_service_impl_delegate.h" #include "brave/components/sync/service/brave_sync_service_impl.h" +#include "ios/chrome/browser/history/model/history_service_factory.h" #include "ios/chrome/browser/sync/model/device_info_sync_service_factory.h" -#define BRAVE_BUILD_SERVICE_INSTANCE_FOR \ - std::make_unique( \ - std::move(init_params), \ - std::make_unique( \ - DeviceInfoSyncServiceFactory::GetForBrowserState(browser_state))); +#define BRAVE_BUILD_SERVICE_INSTANCE_FOR \ + std::make_unique( \ + std::move(init_params), \ + std::make_unique( \ + DeviceInfoSyncServiceFactory::GetForBrowserState(browser_state), \ + ios::HistoryServiceFactory::GetForBrowserState( \ + browser_state, ServiceAccessType::IMPLICIT_ACCESS))); #include "src/ios/chrome/browser/sync/model/sync_service_factory.mm" diff --git a/components/brave_sync/brave_sync_p3a.cc b/components/brave_sync/brave_sync_p3a.cc index 42faa58cfcbc..0804cf8a5f1c 100644 --- a/components/brave_sync/brave_sync_p3a.cc +++ b/components/brave_sync/brave_sync_p3a.cc @@ -46,12 +46,12 @@ void RecordEnabledTypes(bool sync_everything_enabled, } void RecordSyncedObjectsCount(int total_entities) { - // "Brave.Sync.SyncedObjectsCount" + // "Brave.Sync.SyncedObjectsCount.2" // 0 - 0..1000 // 1 - 1001..10000 // 2 - 10001..49000 // 3 - >= 49001 - p3a_utils::RecordToHistogramBucket(kSyncedObjectsCountHistogramName, + p3a_utils::RecordToHistogramBucket(kSyncedObjectsCountHistogramNameV2, {1000, 10000, 49000}, total_entities); } diff --git a/components/brave_sync/brave_sync_p3a.h b/components/brave_sync/brave_sync_p3a.h index 66bfd68bd3ec..7c216bcbc4dc 100644 --- a/components/brave_sync/brave_sync_p3a.h +++ b/components/brave_sync/brave_sync_p3a.h @@ -14,8 +14,9 @@ namespace p3a { // TODO(alexeybarabash): move here also "Brave.Sync.Status.2" and // "Brave.Sync.ProgressTokenEverReset" inline constexpr char kEnabledTypesHistogramName[] = "Brave.Sync.EnabledTypes"; -inline constexpr char kSyncedObjectsCountHistogramName[] = - "Brave.Sync.SyncedObjectsCount"; +// Improved version of metric which includes count of synced History objects +inline constexpr char kSyncedObjectsCountHistogramNameV2[] = + "Brave.Sync.SyncedObjectsCount.2"; enum class EnabledTypesAnswer { kEmptyOrBookmarksOnly = 0, diff --git a/components/brave_sync/brave_sync_p3a_unittest.cc b/components/brave_sync/brave_sync_p3a_unittest.cc index c12af598a5f0..9158476e959a 100644 --- a/components/brave_sync/brave_sync_p3a_unittest.cc +++ b/components/brave_sync/brave_sync_p3a_unittest.cc @@ -58,22 +58,22 @@ TEST(BraveSyncP3ATest, TestSyncedObjectsCount) { base::HistogramTester histogram_tester; RecordSyncedObjectsCount(0); - histogram_tester.ExpectBucketCount(kSyncedObjectsCountHistogramName, 0, 1); + histogram_tester.ExpectBucketCount(kSyncedObjectsCountHistogramNameV2, 0, 1); RecordSyncedObjectsCount(1000); - histogram_tester.ExpectBucketCount(kSyncedObjectsCountHistogramName, 0, 2); + histogram_tester.ExpectBucketCount(kSyncedObjectsCountHistogramNameV2, 0, 2); RecordSyncedObjectsCount(1001); - histogram_tester.ExpectBucketCount(kSyncedObjectsCountHistogramName, 1, 1); + histogram_tester.ExpectBucketCount(kSyncedObjectsCountHistogramNameV2, 1, 1); RecordSyncedObjectsCount(10000); - histogram_tester.ExpectBucketCount(kSyncedObjectsCountHistogramName, 1, 2); + histogram_tester.ExpectBucketCount(kSyncedObjectsCountHistogramNameV2, 1, 2); RecordSyncedObjectsCount(10001); - histogram_tester.ExpectBucketCount(kSyncedObjectsCountHistogramName, 2, 1); + histogram_tester.ExpectBucketCount(kSyncedObjectsCountHistogramNameV2, 2, 1); RecordSyncedObjectsCount(49000); - histogram_tester.ExpectBucketCount(kSyncedObjectsCountHistogramName, 2, 2); + histogram_tester.ExpectBucketCount(kSyncedObjectsCountHistogramNameV2, 2, 2); RecordSyncedObjectsCount(49001); - histogram_tester.ExpectBucketCount(kSyncedObjectsCountHistogramName, 3, 1); + histogram_tester.ExpectBucketCount(kSyncedObjectsCountHistogramNameV2, 3, 1); } } // namespace p3a diff --git a/components/history/core/browser/BUILD.gn b/components/history/core/browser/BUILD.gn new file mode 100644 index 000000000000..0386f7d707e3 --- /dev/null +++ b/components/history/core/browser/BUILD.gn @@ -0,0 +1,16 @@ +# Copyright (c) 2024 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 https://mozilla.org/MPL/2.0/. + +source_set("unit_tests") { + testonly = true + sources = [ + "//brave/components/history/core/browser/brave_visit_database_unittest.cc", + ] + deps = [ + "//base", + "//components/history/core/browser", + "//testing/gtest", + ] +} diff --git a/components/history/core/browser/brave_visit_database_unittest.cc b/components/history/core/browser/brave_visit_database_unittest.cc new file mode 100644 index 000000000000..0c0c05df7fdc --- /dev/null +++ b/components/history/core/browser/brave_visit_database_unittest.cc @@ -0,0 +1,83 @@ +/* Copyright (c) 2024 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 https://mozilla.org/MPL/2.0/. */ + +#include "base/time/time.h" +#include "components/history/core/browser/url_database.h" +#include "components/history/core/browser/visit_database.h" +#include "testing/gtest/include/gtest/gtest.h" +#include "testing/platform_test.h" + +using base::Time; + +namespace history { + +// Inspired by Chromium's VisitDatabaseTest.IsKnownToSync +class BraveVisitDatabaseTest : public PlatformTest, + public URLDatabase, + public VisitDatabase { + public: + BraveVisitDatabaseTest() {} + + private: + // Test setup. + void SetUp() override { + PlatformTest::SetUp(); + + EXPECT_TRUE(db_.OpenInMemory()); + + // Initialize the tables for this test. + CreateURLTable(false); + CreateMainURLIndex(); + InitVisitTable(); + } + void TearDown() override { + db_.Close(); + PlatformTest::TearDown(); + } + + // Provided for URL/VisitDatabase. + sql::Database& GetDB() override { return db_; } + + sql::Database db_; +}; + +TEST_F(BraveVisitDatabaseTest, BraveGetKnownToSyncCount) { + // Insert three rows, VisitIDs 1, 2, and 3. + for (VisitID i = 1; i <= 3; i++) { + VisitRow original(i, Time::Now(), 23, ui::PageTransitionFromInt(0), 19, + false, 0); + AddVisit(&original, SOURCE_BROWSED); + ASSERT_EQ(i, original.visit_id); // Verifies that we added 1, 2, and 3 + } + + int known_to_sync_count = 0; + ASSERT_TRUE(GetKnownToSyncCount(&known_to_sync_count)); + EXPECT_EQ(known_to_sync_count, 0); + + // Set 2 and 3 to be `is_known_to_sync`. + VisitRow visit2; + ASSERT_TRUE(GetRowForVisit(2, &visit2)); + EXPECT_FALSE(visit2.is_known_to_sync); + visit2.is_known_to_sync = true; + ASSERT_TRUE(UpdateVisitRow(visit2)); + + VisitRow visit3; + ASSERT_TRUE(GetRowForVisit(3, &visit3)); + EXPECT_FALSE(visit3.is_known_to_sync); + visit3.is_known_to_sync = true; + ASSERT_TRUE(UpdateVisitRow(visit3)); + + ASSERT_TRUE(GetKnownToSyncCount(&known_to_sync_count)); + EXPECT_EQ(known_to_sync_count, 2); + + // Now clear out all `is_known_to_sync` bits and verify that we still count + // correctly. + SetAllVisitsAsNotKnownToSync(); + + ASSERT_TRUE(GetKnownToSyncCount(&known_to_sync_count)); + EXPECT_EQ(known_to_sync_count, 0); +} + +} // namespace history diff --git a/components/p3a/metric_names.h b/components/p3a/metric_names.h index 63249cdf286a..47833458d292 100644 --- a/components/p3a/metric_names.h +++ b/components/p3a/metric_names.h @@ -218,7 +218,7 @@ inline constexpr auto kCollectedSlowHistograms = "Brave.ReaderMode.NumberReaderModeActivated", "Brave.Rewards.TipsSent.2", "Brave.Sync.EnabledTypes", - "Brave.Sync.SyncedObjectsCount", + "Brave.Sync.SyncedObjectsCount.2", "Brave.Today.UsageMonthly", "Brave.Toolbar.ForwardNavigationAction", "Brave.Wallet.UsageMonthly", diff --git a/components/sync/service/BUILD.gn b/components/sync/service/BUILD.gn index b1fc73c29a58..ac9021fc8393 100644 --- a/components/sync/service/BUILD.gn +++ b/components/sync/service/BUILD.gn @@ -20,6 +20,7 @@ source_set("unit_tests") { "//base/test:test_support", "//brave/components/brave_sync:crypto", "//brave/components/brave_sync:network_time_helper", + "//brave/components/brave_sync:p3a", "//brave/components/brave_sync:prefs", "//brave/components/constants", "//brave/components/sync/test:test_support", diff --git a/components/sync/service/brave_sync_service_impl.cc b/components/sync/service/brave_sync_service_impl.cc index 0e641ae64ecb..4d4b1bc21ed5 100644 --- a/components/sync/service/brave_sync_service_impl.cc +++ b/components/sync/service/brave_sync_service_impl.cc @@ -323,13 +323,17 @@ void BraveSyncServiceImpl::LocalDeviceAppeared() { } namespace { -const int kCyclesBeforeUpdateP3AObjects = 10; +// Typical cycle takes 30 sec, let's send P3A updates each ~30 minutes +const int kCyclesBeforeUpdateP3AObjects = 60; +// And Let's do the first update in ~5 minutes after sync start +const int kCyclesBeforeFirstUpdatesP3A = 10; } // namespace void BraveSyncServiceImpl::OnSyncCycleCompleted( const SyncCycleSnapshot& snapshot) { SyncServiceImpl::OnSyncCycleCompleted(snapshot); - if (completed_cycles_count_ % kCyclesBeforeUpdateP3AObjects == 0) { + if (completed_cycles_count_ == kCyclesBeforeFirstUpdatesP3A || + completed_cycles_count_ % kCyclesBeforeUpdateP3AObjects == 0) { UpdateP3AObjectsNumber(); } ++completed_cycles_count_; @@ -347,7 +351,19 @@ void BraveSyncServiceImpl::OnGotEntityCounts( total_entities += count.non_tombstone_entities; } - brave_sync::p3a::RecordSyncedObjectsCount(total_entities); + if (GetUserSettings()->GetSelectedTypes().Has( + syncer::UserSelectableType::kHistory)) { + // History stores info about synced objects in a different way than the + // others types. Issue a separate request to achieve this info + sync_service_impl_delegate_->GetKnownToSyncHistoryCount(base::BindOnce( + [](int total_entities, std::pair known_to_sync_count) { + brave_sync::p3a::RecordSyncedObjectsCount(total_entities + + known_to_sync_count.second); + }, + total_entities)); + } else { + brave_sync::p3a::RecordSyncedObjectsCount(total_entities); + } } void BraveSyncServiceImpl::OnPreferredDataTypesPrefChange( diff --git a/components/sync/service/brave_sync_service_impl.h b/components/sync/service/brave_sync_service_impl.h index 9c1fedfc1e23..25dd108e5443 100644 --- a/components/sync/service/brave_sync_service_impl.h +++ b/components/sync/service/brave_sync_service_impl.h @@ -78,6 +78,8 @@ class BraveSyncServiceImpl : public SyncServiceImpl { FRIEND_TEST_ALL_PREFIXES(BraveSyncServiceImplTest, JoinActiveOrNewChain); FRIEND_TEST_ALL_PREFIXES(BraveSyncServiceImplTest, JoinDeletedChain); FRIEND_TEST_ALL_PREFIXES(BraveSyncServiceImplTest, HistoryPreconditions); + FRIEND_TEST_ALL_PREFIXES(BraveSyncServiceImplTest, + P3aForHistoryThroughDelegate); BraveSyncAuthManager* GetBraveSyncAuthManager(); SyncServiceCrypto* GetCryptoForTests(); diff --git a/components/sync/service/brave_sync_service_impl_unittest.cc b/components/sync/service/brave_sync_service_impl_unittest.cc index 02100ac6eaad..ef51072cf6f5 100644 --- a/components/sync/service/brave_sync_service_impl_unittest.cc +++ b/components/sync/service/brave_sync_service_impl_unittest.cc @@ -7,8 +7,11 @@ #include "base/base64.h" #include "base/logging.h" +#include "base/memory/raw_ptr.h" #include "base/test/gtest_util.h" +#include "base/test/metrics/histogram_tester.h" #include "base/test/task_environment.h" +#include "brave/components/brave_sync/brave_sync_p3a.h" #include "brave/components/history/core/browser/sync/brave_history_delete_directives_model_type_controller.h" #include "brave/components/history/core/browser/sync/brave_history_model_type_controller.h" #include "brave/components/sync/service/brave_sync_service_impl.h" @@ -19,6 +22,7 @@ #include "components/os_crypt/sync/os_crypt_mocker.h" #include "components/sync/engine/nigori/key_derivation_params.h" #include "components/sync/engine/nigori/nigori.h" +#include "components/sync/model/type_entities_count.h" #include "components/sync/service/data_type_manager_impl.h" #include "components/sync/test/data_type_manager_mock.h" #include "components/sync/test/fake_data_type_controller.h" @@ -65,10 +69,13 @@ class SyncServiceImplDelegateMock : public SyncServiceImplDelegate { public: SyncServiceImplDelegateMock() = default; ~SyncServiceImplDelegateMock() override = default; - void SuspendDeviceObserverForOwnReset() override {} - void ResumeDeviceObserver() override {} - void SetLocalDeviceAppearedCallback( - base::OnceCallback local_device_appeared_callback) override {} + + MOCK_METHOD0(SuspendDeviceObserverForOwnReset, void()); + MOCK_METHOD0(ResumeDeviceObserver, void()); + MOCK_METHOD1(SetLocalDeviceAppearedCallback, + void(base::OnceCallback)); + MOCK_METHOD1(GetKnownToSyncHistoryCount, + void(base::OnceCallback)>)); }; class SyncServiceObserverMock : public SyncServiceObserver { @@ -110,9 +117,12 @@ class BraveSyncServiceImplTest : public testing::Test { ON_CALL(*sync_client, CreateDataTypeControllers(_)) .WillByDefault(Return(ByMove(std::move(controllers)))); + auto sync_service_delegate(std::make_unique()); + sync_service_delegate_ = sync_service_delegate.get(); + sync_service_impl_ = std::make_unique( sync_service_impl_bundle_.CreateBasicInitParams(std::move(sync_client)), - std::make_unique()); + std::move(sync_service_delegate)); } brave_sync::Prefs* brave_sync_prefs() { return &brave_sync_prefs_; } @@ -141,6 +151,7 @@ class BraveSyncServiceImplTest : public testing::Test { protected: content::BrowserTaskEnvironment task_environment_; + raw_ptr sync_service_delegate_; private: SyncServiceImplBundle sync_service_impl_bundle_; @@ -592,4 +603,48 @@ TEST_F(BraveSyncServiceImplTest, OnlyBookmarksAfterSetup) { OSCryptMocker::TearDown(); } +TEST_F(BraveSyncServiceImplTest, P3aForHistoryThroughDelegate) { + OSCryptMocker::SetUp(); + CreateSyncService(ModelTypeSet({BOOKMARKS, HISTORY})); + + brave_sync_service_impl()->Initialize(); + EXPECT_FALSE(engine()); + brave_sync_service_impl()->SetSyncCode(kValidSyncCode); + task_environment_.RunUntilIdle(); + + base::HistogramTester histogram_tester; + + std::vector counts; + syncer::TypeEntitiesCount bookmarks_count(syncer::BOOKMARKS); + bookmarks_count.entities = bookmarks_count.non_tombstone_entities = 1; + counts.push_back(bookmarks_count); + + brave_sync_service_impl()->OnGotEntityCounts(counts); + histogram_tester.ExpectBucketCount( + brave_sync::p3a::kSyncedObjectsCountHistogramNameV2, 0, 1); + + // Enable History and pretend we got its number from delegate. + // We need to have setup handle, otherwise + // |SyncUserSettingsImpl::SetSelectedTypes| and + // |SyncUserSettingsImpl::GetSelectedTypes| work wrong + auto sync_blocker = brave_sync_service_impl()->GetSetupInProgressHandle(); + auto selected_types = + brave_sync_service_impl()->GetUserSettings()->GetSelectedTypes(); + selected_types.Put(UserSelectableType::kHistory); + brave_sync_service_impl()->GetUserSettings()->SetSelectedTypes( + false, selected_types); + + ON_CALL(*sync_service_delegate_, GetKnownToSyncHistoryCount(_)) + .WillByDefault( + [](base::OnceCallback)> callback) { + std::move(callback).Run(std::pair(true, 10001)); + }); + + brave_sync_service_impl()->OnGotEntityCounts(counts); + histogram_tester.ExpectBucketCount( + brave_sync::p3a::kSyncedObjectsCountHistogramNameV2, 2, 1); + + OSCryptMocker::TearDown(); +} + } // namespace syncer diff --git a/components/sync/service/sync_service_impl_delegate.h b/components/sync/service/sync_service_impl_delegate.h index 937f160aedf3..a39b4d7b03a5 100644 --- a/components/sync/service/sync_service_impl_delegate.h +++ b/components/sync/service/sync_service_impl_delegate.h @@ -6,6 +6,8 @@ #ifndef BRAVE_COMPONENTS_SYNC_SERVICE_SYNC_SERVICE_IMPL_DELEGATE_H_ #define BRAVE_COMPONENTS_SYNC_SERVICE_SYNC_SERVICE_IMPL_DELEGATE_H_ +#include + #include "base/functional/callback_forward.h" #include "base/memory/raw_ptr.h" @@ -22,6 +24,9 @@ class SyncServiceImplDelegate { virtual void SetLocalDeviceAppearedCallback( base::OnceCallback local_device_appeared_callback) = 0; + virtual void GetKnownToSyncHistoryCount( + base::OnceCallback)> callback) = 0; + void set_profile_sync_service(BraveSyncServiceImpl* sync_service_impl) { sync_service_impl_ = sync_service_impl; } diff --git a/test/BUILD.gn b/test/BUILD.gn index fbe5c1c62bbf..b1fd72c30cdc 100644 --- a/test/BUILD.gn +++ b/test/BUILD.gn @@ -234,6 +234,7 @@ test("brave_unit_tests") { "//brave/components/de_amp/browser/test:unit_tests", "//brave/components/debounce/browser/test:unit_tests", "//brave/components/embedder_support:unit_tests", + "//brave/components/history/core/browser:unit_tests", "//brave/components/history/core/browser/sync:unit_tests", "//brave/components/ipfs/buildflags", "//brave/components/ipfs/test:brave_ipfs_unit_tests",