From 24303f67d5711acc6984ac490ae3c63b66804df6 Mon Sep 17 00:00:00 2001 From: goodov <5928869+goodov@users.noreply.github.com> Date: Sat, 4 May 2024 02:53:32 +0700 Subject: [PATCH] Support OnDemandUpdate with multiple components at once. (#23169) * Support OnDemandUpdate with multiple components at once. * Review fixes. * Mark two missed components as Brave. --- browser/brave_browser_main_parts.cc | 15 +++-- browser/brave_browser_main_parts.h | 1 + browser/brave_browser_process_impl.cc | 5 -- .../component_updater_utils.cc | 11 +--- .../component_updater/component_installer.cc | 8 +++ .../component_updater/component_installer.h | 8 ++- .../component_updater_service.cc | 48 ++++++++++++++++ .../component_updater_service.h | 18 +++--- .../component_updater_service_internal.h | 20 +++++++ .../update_client/update_checker.cc | 57 ++++++++++++++++--- .../components/update_client/update_client.cc | 4 ++ .../components/update_client/update_client.h | 5 ++ .../brave_component_updater/browser/BUILD.gn | 16 ++++++ .../browser/brave_component_installer.cc | 4 ++ .../browser/brave_component_installer.h | 1 + .../browser/brave_on_demand_updater.cc | 31 ++++++++-- .../browser/brave_on_demand_updater.h | 19 +++++-- .../browser/mock_on_demand_updater.cc | 22 +++++++ .../browser/mock_on_demand_updater.h | 42 ++++++++++++++ .../ad_block_component_filters_provider.cc | 17 ------ .../ad_block_component_filters_provider.h | 6 +- .../browser/ad_block_component_installer.cc | 5 ++ .../ad_block_component_service_manager.cc | 28 +++++---- components/brave_wallet/browser/test/BUILD.gn | 1 + .../browser/wallet_data_files_installer.cc | 5 ++ .../wallet_data_files_installer_unittest.cc | 50 ++++++++-------- ...p_background_images_component_installer.cc | 5 ++ .../media_detector_component_installer.cc | 5 ++ .../browser/core/psst_component_installer.cc | 5 ++ ios/app/brave_core_main.mm | 8 +-- ios/browser/component_updater/BUILD.gn | 3 +- .../component_updater_utils.mm | 11 +--- 32 files changed, 362 insertions(+), 122 deletions(-) create mode 100644 chromium_src/components/component_updater/component_updater_service.cc create mode 100644 chromium_src/components/component_updater/component_updater_service_internal.h create mode 100644 components/brave_component_updater/browser/mock_on_demand_updater.cc create mode 100644 components/brave_component_updater/browser/mock_on_demand_updater.h diff --git a/browser/brave_browser_main_parts.cc b/browser/brave_browser_main_parts.cc index 43e710418eb0..7080e04dcdbf 100644 --- a/browser/brave_browser_main_parts.cc +++ b/browser/brave_browser_main_parts.cc @@ -11,6 +11,7 @@ #include "base/command_line.h" #include "brave/browser/browsing_data/brave_clear_browsing_data.h" #include "brave/browser/ethereum_remote_client/buildflags/buildflags.h" +#include "brave/components/brave_component_updater/browser/brave_on_demand_updater.h" #include "brave/components/brave_rewards/common/rewards_flags.h" #include "brave/components/brave_rewards/common/rewards_util.h" #include "brave/components/brave_sync/features.h" @@ -20,8 +21,10 @@ #include "brave/components/tor/buildflags/buildflags.h" #include "brave/components/translate/core/common/brave_translate_features.h" #include "build/build_config.h" +#include "chrome/browser/browser_process.h" #include "chrome/browser/profiles/profile.h" #include "chrome/common/chrome_features.h" +#include "components/component_updater/component_updater_service.h" #include "components/prefs/pref_service.h" #include "components/sync/base/command_line_switches.h" #include "components/translate/core/browser/translate_language_list.h" @@ -60,16 +63,20 @@ #include "brave/browser/android/preferences/features.h" #endif -#if BUILDFLAG(ENABLE_TOR) || !BUILDFLAG(IS_ANDROID) -#include "chrome/browser/browser_process.h" -#endif - #if BUILDFLAG(ETHEREUM_REMOTE_CLIENT_ENABLED) && BUILDFLAG(ENABLE_EXTENSIONS) #include "brave/browser/extensions/brave_component_loader.h" #include "chrome/browser/extensions/extension_service.h" #include "extensions/browser/extension_system.h" #endif +int BraveBrowserMainParts::PreMainMessageLoopRun() { + brave_component_updater::BraveOnDemandUpdater::GetInstance() + ->RegisterOnDemandUpdater( + &g_browser_process->component_updater()->GetOnDemandUpdater()); + + return ChromeBrowserMainParts::PreMainMessageLoopRun(); +} + void BraveBrowserMainParts::PreBrowserStart() { #if BUILDFLAG(ENABLE_SPEEDREADER) // Register() must be called after the SerializedNavigationDriver is diff --git a/browser/brave_browser_main_parts.h b/browser/brave_browser_main_parts.h index e829af39f56a..5dd10bbfcdc7 100644 --- a/browser/brave_browser_main_parts.h +++ b/browser/brave_browser_main_parts.h @@ -16,6 +16,7 @@ class BraveBrowserMainParts : public ChromeBrowserMainParts { BraveBrowserMainParts& operator=(const BraveBrowserMainParts&) = delete; ~BraveBrowserMainParts() override = default; + int PreMainMessageLoopRun() override; void PreBrowserStart() override; void PostBrowserStart() override; void PreShutdown() override; diff --git a/browser/brave_browser_process_impl.cc b/browser/brave_browser_process_impl.cc index 2d1c32fe27fa..04bda7d72fdf 100644 --- a/browser/brave_browser_process_impl.cc +++ b/browser/brave_browser_process_impl.cc @@ -25,7 +25,6 @@ #include "brave/common/brave_channel_info.h" #include "brave/components/brave_ads/browser/component_updater/resource_component.h" #include "brave/components/brave_component_updater/browser/brave_component_updater_delegate.h" -#include "brave/components/brave_component_updater/browser/brave_on_demand_updater.h" #include "brave/components/brave_component_updater/browser/local_data_files_service.h" #include "brave/components/brave_referrals/browser/brave_referrals_service.h" #include "brave/components/brave_shields/content/browser/ad_block_service.h" @@ -53,7 +52,6 @@ #include "chrome/common/channel_info.h" #include "chrome/common/chrome_paths.h" #include "chrome/common/pref_names.h" -#include "components/component_updater/component_updater_service.h" #include "components/component_updater/timer_update_scheduler.h" #include "content/public/browser/browser_thread.h" #include "content/public/browser/child_process_security_policy.h" @@ -153,9 +151,6 @@ void BraveBrowserProcessImpl::Init() { content::ChildProcessSecurityPolicy::GetInstance()->RegisterWebSafeScheme( ipfs::kIPNSScheme); #endif - brave_component_updater::BraveOnDemandUpdater::GetInstance() - ->RegisterOnDemandUpdateCallback( - base::BindRepeating(&component_updater::BraveOnDemandUpdate)); UpdateBraveDarkMode(); pref_change_registrar_.Add( kBraveDarkMode, diff --git a/chromium_src/chrome/browser/component_updater/component_updater_utils.cc b/chromium_src/chrome/browser/component_updater/component_updater_utils.cc index 477ac2adf8cb..6202e9f38804 100644 --- a/chromium_src/chrome/browser/component_updater/component_updater_utils.cc +++ b/chromium_src/chrome/browser/component_updater/component_updater_utils.cc @@ -5,18 +5,13 @@ #include "src/chrome/browser/component_updater/component_updater_utils.cc" -#include "base/functional/callback.h" -#include "chrome/browser/browser_process.h" -#include "components/component_updater/component_updater_service.h" +#include "brave/components/brave_component_updater/browser/brave_on_demand_updater.h" namespace component_updater { void BraveOnDemandUpdate(const std::string& component_id) { - component_updater::ComponentUpdateService* cus = - g_browser_process->component_updater(); - cus->GetOnDemandUpdater().OnDemandUpdate( - component_id, component_updater::OnDemandUpdater::Priority::FOREGROUND, - component_updater::Callback()); + brave_component_updater::BraveOnDemandUpdater::GetInstance()->OnDemandUpdate( + component_id); } } // namespace component_updater diff --git a/chromium_src/components/component_updater/component_installer.cc b/chromium_src/components/component_updater/component_installer.cc index 5d23cad87831..969a82ac4865 100644 --- a/chromium_src/components/component_updater/component_installer.cc +++ b/chromium_src/components/component_updater/component_installer.cc @@ -15,6 +15,10 @@ namespace component_updater { +bool ComponentInstallerPolicy::IsBraveComponent() const { + return false; +} + void ComponentInstaller::Register(ComponentUpdateService* cus, base::OnceClosure callback) { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); @@ -59,4 +63,8 @@ void ComponentInstaller::Register( registered_version, max_previous_product_version); } +bool ComponentInstaller::IsBraveComponent() const { + return installer_policy_->IsBraveComponent(); +} + } // namespace component_updater diff --git a/chromium_src/components/component_updater/component_installer.h b/chromium_src/components/component_updater/component_installer.h index e87030b937ee..169e1f269f15 100644 --- a/chromium_src/components/component_updater/component_installer.h +++ b/chromium_src/components/component_updater/component_installer.h @@ -6,8 +6,6 @@ #ifndef BRAVE_CHROMIUM_SRC_COMPONENTS_COMPONENT_UPDATER_COMPONENT_INSTALLER_H_ #define BRAVE_CHROMIUM_SRC_COMPONENTS_COMPONENT_UPDATER_COMPONENT_INSTALLER_H_ -// Prevent CrxInstaller::OnUpdateError from being redefined by the below -// #define. #include "components/update_client/update_client.h" // We can't redefine Register() here because that would change two methods at @@ -22,10 +20,16 @@ const base::Version& registered_version = base::Version(kNullVersion), \ const base::Version& max_previous_product_version = \ base::Version(kNullVersion)); \ + bool IsBraveComponent() const override; \ void OnUpdateError +#define AllowUpdatesOnMeteredConnections \ + IsBraveComponent() const; \ + virtual bool AllowUpdatesOnMeteredConnections + #include "src/components/component_updater/component_installer.h" // IWYU pragma: export #undef OnUpdateError +#undef AllowUpdatesOnMeteredConnections #endif // BRAVE_CHROMIUM_SRC_COMPONENTS_COMPONENT_UPDATER_COMPONENT_INSTALLER_H_ diff --git a/chromium_src/components/component_updater/component_updater_service.cc b/chromium_src/components/component_updater/component_updater_service.cc new file mode 100644 index 000000000000..03bae8c33b93 --- /dev/null +++ b/chromium_src/components/component_updater/component_updater_service.cc @@ -0,0 +1,48 @@ +/* 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 "components/component_updater/component_updater_service.h" + +#include "base/notreached.h" +#include "components/component_updater/component_updater_service_internal.h" + +#include "src/components/component_updater/component_updater_service.cc" + +namespace component_updater { + +void CrxUpdateService::OnDemandUpdate(const std::vector& ids, + Priority priority, + Callback callback) { + DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); + + for (const auto& id : ids) { + if (!GetComponent(id)) { + if (callback) { + base::SequencedTaskRunner::GetCurrentDefault()->PostTask( + FROM_HERE, base::BindOnce(std::move(callback), + update_client::Error::INVALID_ARGUMENT)); + } + return; + } + } + + auto crx_data_callback = base::BindOnce(&CrxUpdateService::GetCrxComponents, + base::Unretained(this)); + auto update_complete_callback = base::BindOnce( + &CrxUpdateService::OnUpdateComplete, base::Unretained(this), + std::move(callback), base::TimeTicks::Now()); + + update_client_->Update(ids, std::move(crx_data_callback), {}, + priority == Priority::FOREGROUND, + std::move(update_complete_callback)); +} + +void OnDemandUpdater::OnDemandUpdate(const std::vector& ids, + Priority priority, + Callback callback) { + NOTREACHED_NORETURN(); +} + +} // namespace component_updater diff --git a/chromium_src/components/component_updater/component_updater_service.h b/chromium_src/components/component_updater/component_updater_service.h index c2dd91b28572..4fd31f49b329 100644 --- a/chromium_src/components/component_updater/component_updater_service.h +++ b/chromium_src/components/component_updater/component_updater_service.h @@ -14,21 +14,25 @@ class BraveComponentUpdaterAndroid; } } // namespace chrome -namespace brave_shields { -class AdBlockComponentFiltersProvider; +namespace brave_component_updater { +class BraveOnDemandUpdater; } #define BRAVE_COMPONENT_UPDATER_SERVICE_H_ \ friend class ::IPFSDOMHandler; \ friend class ::chrome::android::BraveComponentUpdaterAndroid; -#define BRAVE_COMPONENT_UPDATER_SERVICE_H_ON_DEMAND_UPDATER \ - private: \ - friend void BraveOnDemandUpdate(const std::string&); \ - friend class brave_shields::AdBlockComponentFiltersProvider; \ - \ +#define BRAVE_COMPONENT_UPDATER_SERVICE_H_ON_DEMAND_UPDATER \ + private: \ + friend class brave_component_updater::BraveOnDemandUpdater; \ + \ + virtual void OnDemandUpdate(const std::vector& ids, \ + Priority priority, Callback callback); \ + \ public: + #include "src/components/component_updater/component_updater_service.h" // IWYU pragma: export + #undef BRAVE_COMPONENT_UPDATER_SERVICE_H_ON_DEMAND_UPDATER #undef BRAVE_COMPONENT_UPDATER_SERVICE_H_ diff --git a/chromium_src/components/component_updater/component_updater_service_internal.h b/chromium_src/components/component_updater/component_updater_service_internal.h new file mode 100644 index 000000000000..b4cc35639fda --- /dev/null +++ b/chromium_src/components/component_updater/component_updater_service_internal.h @@ -0,0 +1,20 @@ +/* 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_COMPONENT_UPDATER_COMPONENT_UPDATER_SERVICE_INTERNAL_H_ +#define BRAVE_CHROMIUM_SRC_COMPONENTS_COMPONENT_UPDATER_COMPONENT_UPDATER_SERVICE_INTERNAL_H_ + +#include "components/component_updater/component_updater_service.h" + +#define OnDemandUpdate \ + OnDemandUpdate(const std::vector& ids, Priority priority, \ + Callback callback) override; \ + void OnDemandUpdate + +#include "src/components/component_updater/component_updater_service_internal.h" // IWYU pragma: export + +#undef OnDemandUpdate + +#endif // BRAVE_CHROMIUM_SRC_COMPONENTS_COMPONENT_UPDATER_COMPONENT_UPDATER_SERVICE_INTERNAL_H_ diff --git a/chromium_src/components/update_client/update_checker.cc b/chromium_src/components/update_client/update_checker.cc index 00d7527f7eab..313966769b74 100644 --- a/chromium_src/components/update_client/update_checker.cc +++ b/chromium_src/components/update_client/update_checker.cc @@ -3,8 +3,13 @@ * 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/. */ +#include "components/update_client/update_checker.h" + #include +#include "base/ranges/algorithm.h" +#include "components/update_client/update_client.h" + #include "src/components/update_client/update_checker.cc" #if BUILDFLAG(WIDEVINE_ARM64_DLL_FIX) @@ -14,6 +19,19 @@ namespace update_client { +namespace { + +bool IsBraveComponent(const Component* component) { + CHECK(component); + const auto& crx_component = component->crx_component(); + if (!crx_component || !crx_component->installer) { + return false; + } + return crx_component->installer->IsBraveComponent(); +} + +} // namespace + SequentialUpdateChecker::SequentialUpdateChecker( scoped_refptr config, PersistedData* metadata) @@ -39,6 +57,16 @@ void SequentialUpdateChecker::CheckForUpdates( additional_attributes_ = additional_attributes; update_check_callback_ = std::move(update_check_callback); + // Partition IDs for batch update checks. The order of + // `components_to_check_for_updates` doesn't matter to the caller, as + // post-update mapping is done via an Id->Component map, making this + // rearrangement safe. + base::ranges::stable_partition( + update_context_->components_to_check_for_updates, + [&](const std::string& id) { + return IsBraveComponent(update_context_->components[id].get()); + }); + for (const auto& id : update_context_->components_to_check_for_updates) { remaining_ids_.push_back(id); } @@ -56,12 +84,20 @@ void SequentialUpdateChecker::CheckNext( DCHECK(!remaining_ids_.empty()); DCHECK(update_context_); - const auto id = remaining_ids_.front(); - remaining_ids_.pop_front(); + // Support multiple checks in a single call, but only if they are all Brave. + std::vector ids; + for (auto id_it = remaining_ids_.begin(); id_it != remaining_ids_.end();) { + const auto& component = update_context_->components[*id_it]; + if (!ids.empty() && !IsBraveComponent(component.get())) { + break; + } + ids.push_back(*id_it); + id_it = remaining_ids_.erase(id_it); + } scoped_refptr context = new UpdateContext( update_context_->config, update_context_->crx_cache_, - update_context_->is_foreground, update_context_->is_install, {id}, + update_context_->is_foreground, update_context_->is_install, ids, update_context_->crx_state_change_callback, update_context_->notify_observers_callback, // We don't pass a context callback here because UpdateChecker doesn't use @@ -69,12 +105,15 @@ void SequentialUpdateChecker::CheckNext( base::DoNothing(), update_context_->persisted_data, /*is_update_check_only=*/false); - auto& component = context->components[id]; - auto& crx_component = update_context_->components[id]->crx_component(); - component->set_crx_component(*crx_component); - component->set_previous_version(crx_component->version); - component->set_previous_fp(crx_component->fingerprint); - context->components_to_check_for_updates.push_back(id); + DCHECK(!ids.empty()); + for (const auto& id : ids) { + auto& component = context->components[id]; + auto& crx_component = update_context_->components[id]->crx_component(); + component->set_crx_component(*crx_component); + component->set_previous_version(crx_component->version); + component->set_previous_fp(crx_component->fingerprint); + context->components_to_check_for_updates.push_back(id); + } update_checker_ = UpdateChecker::Create(config_, metadata_); diff --git a/chromium_src/components/update_client/update_client.cc b/chromium_src/components/update_client/update_client.cc index 2ac05bf54a4e..80c65f9d9a56 100644 --- a/chromium_src/components/update_client/update_client.cc +++ b/chromium_src/components/update_client/update_client.cc @@ -13,6 +13,10 @@ namespace update_client { +bool CrxInstaller::IsBraveComponent() const { + return false; +} + scoped_refptr UpdateClientFactory( scoped_refptr config) { VLOG(3) << "Brave UpdateClientFactory called"; diff --git a/chromium_src/components/update_client/update_client.h b/chromium_src/components/update_client/update_client.h index 0b4c06fd8b3a..07f142a30a29 100644 --- a/chromium_src/components/update_client/update_client.h +++ b/chromium_src/components/update_client/update_client.h @@ -6,12 +6,17 @@ #ifndef BRAVE_CHROMIUM_SRC_COMPONENTS_UPDATE_CLIENT_UPDATE_CLIENT_H_ #define BRAVE_CHROMIUM_SRC_COMPONENTS_UPDATE_CLIENT_UPDATE_CLIENT_H_ +#define GetInstalledFile(...) \ + IsBraveComponent() const; \ + virtual bool GetInstalledFile(__VA_ARGS__) + #define UpdateClientFactory \ UpdateClientFactory_ChromiumImpl(scoped_refptr config); \ scoped_refptr UpdateClientFactory #include "src/components/update_client/update_client.h" // IWYU pragma: export +#undef GetInstalledFile #undef UpdateClientFactory #endif // BRAVE_CHROMIUM_SRC_COMPONENTS_UPDATE_CLIENT_UPDATE_CLIENT_H_ diff --git a/components/brave_component_updater/browser/BUILD.gn b/components/brave_component_updater/browser/BUILD.gn index ad3849d4e38c..21d693049875 100644 --- a/components/brave_component_updater/browser/BUILD.gn +++ b/components/brave_component_updater/browser/BUILD.gn @@ -35,3 +35,19 @@ static_library("browser") { "//crypto", ] } + +source_set("test_support") { + testonly = true + + sources = [ + "mock_on_demand_updater.cc", + "mock_on_demand_updater.h", + ] + + deps = [ + ":browser", + "//base", + "//components/component_updater", + "//testing/gmock", + ] +} diff --git a/components/brave_component_updater/browser/brave_component_installer.cc b/components/brave_component_updater/browser/brave_component_installer.cc index 949d2b3912b8..d3f423e909e8 100644 --- a/components/brave_component_updater/browser/brave_component_installer.cc +++ b/components/brave_component_updater/browser/brave_component_installer.cc @@ -139,6 +139,10 @@ BraveComponentInstallerPolicy::GetInstallerAttributes() const { return update_client::InstallerAttributes(); } +bool BraveComponentInstallerPolicy::IsBraveComponent() const { + return true; +} + void RegisterComponent(component_updater::ComponentUpdateService* cus, const std::string& name, const std::string& base64_public_key, diff --git a/components/brave_component_updater/browser/brave_component_installer.h b/components/brave_component_updater/browser/brave_component_installer.h index 304c0685dc14..2c5ba862130b 100644 --- a/components/brave_component_updater/browser/brave_component_installer.h +++ b/components/brave_component_updater/browser/brave_component_installer.h @@ -51,6 +51,7 @@ class BraveComponentInstallerPolicy void GetHash(std::vector* hash) const override; std::string GetName() const override; update_client::InstallerAttributes GetInstallerAttributes() const override; + bool IsBraveComponent() const override; std::string name_; std::string base64_public_key_; diff --git a/components/brave_component_updater/browser/brave_on_demand_updater.cc b/components/brave_component_updater/browser/brave_on_demand_updater.cc index 5f7ad419033e..7bb7ec061fe2 100644 --- a/components/brave_component_updater/browser/brave_on_demand_updater.cc +++ b/components/brave_component_updater/browser/brave_on_demand_updater.cc @@ -6,7 +6,10 @@ #include "brave/components/brave_component_updater/browser/brave_on_demand_updater.h" #include +#include +#include "base/check_is_test.h" +#include "base/functional/callback.h" // IWYU pragma: keep #include "base/no_destructor.h" namespace brave_component_updater { @@ -20,14 +23,34 @@ BraveOnDemandUpdater::BraveOnDemandUpdater() = default; BraveOnDemandUpdater::~BraveOnDemandUpdater() = default; +component_updater::OnDemandUpdater* +BraveOnDemandUpdater::RegisterOnDemandUpdater( + component_updater::OnDemandUpdater* on_demand_updater) { + if (!on_demand_updater) { + CHECK_IS_TEST(); + } + return std::exchange(on_demand_updater_, on_demand_updater); +} + void BraveOnDemandUpdater::OnDemandUpdate(const std::string& id) { - DCHECK(!on_demand_update_callback_.is_null()); - on_demand_update_callback_.Run(id); + OnDemandUpdate(id, component_updater::OnDemandUpdater::Priority::FOREGROUND, + component_updater::Callback()); } -void BraveOnDemandUpdater::RegisterOnDemandUpdateCallback(Callback callback) { - on_demand_update_callback_ = callback; +void BraveOnDemandUpdater::OnDemandUpdate( + const std::string& id, + component_updater::OnDemandUpdater::Priority priority, + component_updater::Callback callback) { + CHECK(on_demand_updater_); + on_demand_updater_->OnDemandUpdate(id, priority, std::move(callback)); } +void BraveOnDemandUpdater::OnDemandUpdate( + const std::vector& ids, + component_updater::OnDemandUpdater::Priority priority, + component_updater::Callback callback) { + CHECK(on_demand_updater_); + on_demand_updater_->OnDemandUpdate(ids, priority, std::move(callback)); +} } // namespace brave_component_updater diff --git a/components/brave_component_updater/browser/brave_on_demand_updater.h b/components/brave_component_updater/browser/brave_on_demand_updater.h index ca53d7004296..72af093229cb 100644 --- a/components/brave_component_updater/browser/brave_on_demand_updater.h +++ b/components/brave_component_updater/browser/brave_on_demand_updater.h @@ -7,8 +7,9 @@ #define BRAVE_COMPONENTS_BRAVE_COMPONENT_UPDATER_BROWSER_BRAVE_ON_DEMAND_UPDATER_H_ #include +#include -#include "base/functional/callback.h" +#include "components/component_updater/component_updater_service.h" namespace base { template @@ -19,21 +20,29 @@ namespace brave_component_updater { class BraveOnDemandUpdater { public: - using Callback = base::RepeatingCallback; static BraveOnDemandUpdater* GetInstance(); BraveOnDemandUpdater(const BraveOnDemandUpdater&) = delete; BraveOnDemandUpdater& operator=(const BraveOnDemandUpdater&) = delete; - ~BraveOnDemandUpdater(); + + component_updater::OnDemandUpdater* RegisterOnDemandUpdater( + component_updater::OnDemandUpdater* on_demand_updater); + void OnDemandUpdate(const std::string& id); - void RegisterOnDemandUpdateCallback(Callback callback); + void OnDemandUpdate(const std::string& id, + component_updater::OnDemandUpdater::Priority priority, + component_updater::Callback callback); + void OnDemandUpdate(const std::vector& ids, + component_updater::OnDemandUpdater::Priority priority, + component_updater::Callback callback); private: friend base::NoDestructor; BraveOnDemandUpdater(); + ~BraveOnDemandUpdater(); - Callback on_demand_update_callback_; + raw_ptr on_demand_updater_ = nullptr; }; } // namespace brave_component_updater diff --git a/components/brave_component_updater/browser/mock_on_demand_updater.cc b/components/brave_component_updater/browser/mock_on_demand_updater.cc new file mode 100644 index 000000000000..09db116716f1 --- /dev/null +++ b/components/brave_component_updater/browser/mock_on_demand_updater.cc @@ -0,0 +1,22 @@ +/* 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 "brave/components/brave_component_updater/browser/mock_on_demand_updater.h" + +#include "brave/components/brave_component_updater/browser/brave_on_demand_updater.h" + +namespace brave_component_updater { + +MockOnDemandUpdater::MockOnDemandUpdater() { + prev_on_demand_updater_ = + BraveOnDemandUpdater::GetInstance()->RegisterOnDemandUpdater(this); +} + +MockOnDemandUpdater::~MockOnDemandUpdater() { + BraveOnDemandUpdater::GetInstance()->RegisterOnDemandUpdater( + prev_on_demand_updater_); +} + +} // namespace brave_component_updater diff --git a/components/brave_component_updater/browser/mock_on_demand_updater.h b/components/brave_component_updater/browser/mock_on_demand_updater.h new file mode 100644 index 000000000000..1ff1f02271cb --- /dev/null +++ b/components/brave_component_updater/browser/mock_on_demand_updater.h @@ -0,0 +1,42 @@ +/* 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_COMPONENTS_BRAVE_COMPONENT_UPDATER_BROWSER_MOCK_ON_DEMAND_UPDATER_H_ +#define BRAVE_COMPONENTS_BRAVE_COMPONENT_UPDATER_BROWSER_MOCK_ON_DEMAND_UPDATER_H_ + +#include +#include + +#include "base/functional/callback.h" +#include "components/component_updater/component_updater_service.h" +#include "testing/gmock/include/gmock/gmock.h" + +namespace brave_component_updater { + +class MockOnDemandUpdater : public component_updater::OnDemandUpdater { + public: + MockOnDemandUpdater(); + ~MockOnDemandUpdater() override; + + MOCK_METHOD(void, + OnDemandUpdate, + (const std::string& id, + component_updater::OnDemandUpdater::Priority priority, + component_updater::Callback callback), + (override)); + MOCK_METHOD(void, + OnDemandUpdate, + (const std::vector& ids, + component_updater::OnDemandUpdater::Priority priority, + component_updater::Callback callback), + (override)); + + private: + raw_ptr prev_on_demand_updater_ = nullptr; +}; + +} // namespace brave_component_updater + +#endif // BRAVE_COMPONENTS_BRAVE_COMPONENT_UPDATER_BROWSER_MOCK_ON_DEMAND_UPDATER_H_ diff --git a/components/brave_shields/core/browser/ad_block_component_filters_provider.cc b/components/brave_shields/core/browser/ad_block_component_filters_provider.cc index 47475aaf6072..b7ec5ef594be 100644 --- a/components/brave_shields/core/browser/ad_block_component_filters_provider.cc +++ b/components/brave_shields/core/browser/ad_block_component_filters_provider.cc @@ -125,21 +125,4 @@ void AdBlockComponentFiltersProvider::LoadFilterSet( base::BindOnce(&OnReadDATFileData, std::move(cb), permission_mask_)); } -void AdBlockComponentFiltersProvider::UpdateComponent( - base::OnceCallback callback) { - if (!component_updater_service_) { - std::move(callback).Run(false); - return; - } - - auto on_updated = [](decltype(callback) cb, update_client::Error error) { - std::move(cb).Run(error == update_client::Error::NONE || - error == update_client::Error::UPDATE_IN_PROGRESS); - }; - - component_updater_service_->GetOnDemandUpdater().OnDemandUpdate( - component_id_, component_updater::OnDemandUpdater::Priority::FOREGROUND, - base::BindOnce(on_updated, std::move(callback))); -} - } // namespace brave_shields diff --git a/components/brave_shields/core/browser/ad_block_component_filters_provider.h b/components/brave_shields/core/browser/ad_block_component_filters_provider.h index bea848a65e2f..3741ce626ac5 100644 --- a/components/brave_shields/core/browser/ad_block_component_filters_provider.h +++ b/components/brave_shields/core/browser/ad_block_component_filters_provider.h @@ -51,14 +51,12 @@ class AdBlockComponentFiltersProvider : public AdBlockFiltersProvider { AdBlockComponentFiltersProvider& operator=( const AdBlockComponentFiltersProvider&) = delete; + const std::string& component_id() const { return component_id_; } + void LoadFilterSet( base::OnceCallback*)>)>) override; - // Updates the component, and when complete executes the specified callback - // with a value indicating success or failure. - void UpdateComponent(base::OnceCallback callback); - // Remove the component. This will force it to be redownloaded next time it // is registered. void UnregisterComponent(); diff --git a/components/brave_shields/core/browser/ad_block_component_installer.cc b/components/brave_shields/core/browser/ad_block_component_installer.cc index 0b1f2c885e65..eaac96e59ede 100644 --- a/components/brave_shields/core/browser/ad_block_component_installer.cc +++ b/components/brave_shields/core/browser/ad_block_component_installer.cc @@ -82,6 +82,7 @@ class AdBlockComponentInstallerPolicy void GetHash(std::vector* hash) const override; std::string GetName() const override; update_client::InstallerAttributes GetInstallerAttributes() const override; + bool IsBraveComponent() const override; private: const std::string component_id_; @@ -155,6 +156,10 @@ AdBlockComponentInstallerPolicy::GetInstallerAttributes() const { return update_client::InstallerAttributes(); } +bool AdBlockComponentInstallerPolicy::IsBraveComponent() const { + return true; +} + void OnRegistered(const std::string& component_id) { BraveOnDemandUpdater::GetInstance()->OnDemandUpdate(component_id); } diff --git a/components/brave_shields/core/browser/ad_block_component_service_manager.cc b/components/brave_shields/core/browser/ad_block_component_service_manager.cc index 0cc11d2639f2..8a61c3a73fa5 100644 --- a/components/brave_shields/core/browser/ad_block_component_service_manager.cc +++ b/components/brave_shields/core/browser/ad_block_component_service_manager.cc @@ -9,12 +9,12 @@ #include #include -#include "base/barrier_callback.h" #include "base/feature_list.h" #include "base/memory/raw_ref.h" #include "base/metrics/histogram_macros.h" #include "base/ranges/algorithm.h" #include "base/values.h" +#include "brave/components/brave_component_updater/browser/brave_on_demand_updater.h" #include "brave/components/brave_shields/core/browser/ad_block_component_filters_provider.h" #include "brave/components/brave_shields/core/browser/ad_block_filters_provider_manager.h" #include "brave/components/brave_shields/core/browser/ad_block_list_p3a.h" @@ -22,6 +22,7 @@ #include "brave/components/brave_shields/core/common/brave_shield_constants.h" #include "brave/components/brave_shields/core/common/features.h" #include "brave/components/brave_shields/core/common/pref_names.h" +#include "components/component_updater/component_updater_service.h" #include "components/prefs/pref_service.h" #include "components/prefs/scoped_user_pref_update.h" @@ -278,22 +279,19 @@ void AdBlockComponentServiceManager::UpdateFilterLists( return; } - // This callback will be executed by our barrier callback once all filter - // list components have been updated. - auto on_all_updated = [](decltype(callback) cb, std::vector results) { - std::move(cb).Run( - base::ranges::all_of(results, [](bool result) { return result; })); - }; + std::vector component_ids; + for (const auto& [key, provider] : component_filters_providers_) { + component_ids.push_back(provider->component_id()); + } - // This barrier callback maintains a "completed count". When it has been - // called the expected number of times, it will execute `on_all_updated`. - auto barrier_callback = base::BarrierCallback( - component_filters_providers_.size(), - base::BindOnce(on_all_updated, std::move(callback))); + auto on_updated = [](decltype(callback) cb, update_client::Error error) { + std::move(cb).Run(error == update_client::Error::NONE || + error == update_client::Error::UPDATE_IN_PROGRESS); + }; - for (auto& [key, provider] : component_filters_providers_) { - provider->UpdateComponent(barrier_callback); - } + brave_component_updater::BraveOnDemandUpdater::GetInstance()->OnDemandUpdate( + component_ids, component_updater::OnDemandUpdater::Priority::FOREGROUND, + base::BindOnce(on_updated, std::move(callback))); } void AdBlockComponentServiceManager::SetFilterListCatalog( diff --git a/components/brave_wallet/browser/test/BUILD.gn b/components/brave_wallet/browser/test/BUILD.gn index 60d7094e9020..e6708893a7f1 100644 --- a/components/brave_wallet/browser/test/BUILD.gn +++ b/components/brave_wallet/browser/test/BUILD.gn @@ -114,6 +114,7 @@ source_set("brave_wallet_unit_tests") { ":test_support", "//base/test:test_support", "//brave/components/brave_component_updater/browser", + "//brave/components/brave_component_updater/browser:test_support", "//brave/components/brave_wallet/browser", "//brave/components/brave_wallet/browser:constants", "//brave/components/brave_wallet/browser:generated_bitcoin_rpc_responses", diff --git a/components/brave_wallet/browser/wallet_data_files_installer.cc b/components/brave_wallet/browser/wallet_data_files_installer.cc index 2f70087028a3..6232874c67e7 100644 --- a/components/brave_wallet/browser/wallet_data_files_installer.cc +++ b/components/brave_wallet/browser/wallet_data_files_installer.cc @@ -71,6 +71,7 @@ class WalletDataFilesInstallerPolicy void GetHash(std::vector* hash) const override; std::string GetName() const override; update_client::InstallerAttributes GetInstallerAttributes() const override; + bool IsBraveComponent() const override; WalletDataFilesInstallerPolicy(const WalletDataFilesInstallerPolicy&) = delete; @@ -132,6 +133,10 @@ WalletDataFilesInstallerPolicy::GetInstallerAttributes() const { return update_client::InstallerAttributes(); } +bool WalletDataFilesInstallerPolicy::IsBraveComponent() const { + return true; +} + std::optional GetLastInstalledWalletVersion() { return last_installed_wallet_version; } diff --git a/components/brave_wallet/browser/wallet_data_files_installer_unittest.cc b/components/brave_wallet/browser/wallet_data_files_installer_unittest.cc index 166562fa9f7f..65755e44aedc 100644 --- a/components/brave_wallet/browser/wallet_data_files_installer_unittest.cc +++ b/components/brave_wallet/browser/wallet_data_files_installer_unittest.cc @@ -15,7 +15,7 @@ #include "base/test/scoped_feature_list.h" #include "base/test/task_environment.h" #include "base/time/time.h" -#include "brave/components/brave_component_updater/browser/brave_on_demand_updater.h" +#include "brave/components/brave_component_updater/browser/mock_on_demand_updater.h" #include "brave/components/brave_wallet/browser/blockchain_registry.h" #include "brave/components/brave_wallet/browser/brave_wallet_prefs.h" #include "brave/components/brave_wallet/browser/brave_wallet_service.h" @@ -32,12 +32,15 @@ #include "services/data_decoder/public/cpp/test_support/in_process_data_decoder.h" #include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h" #include "services/network/test/test_url_loader_factory.h" +#include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" #define FPL(x) FILE_PATH_LITERAL(x) namespace brave_wallet { +namespace { + constexpr char kComponentId[] = "bbckkcdiepaecefgfnibemejliemjnio"; class MockWalletDataFilesInstallerDelegateImpl @@ -75,6 +78,8 @@ class MockBraveWalletServiceDelegateImpl : public BraveWalletServiceDelegate { } }; +} // namespace + class WalletDataFilesInstallerUnitTest : public testing::Test { public: WalletDataFilesInstallerUnitTest() @@ -189,35 +194,27 @@ class WalletDataFilesInstallerUnitTest : public testing::Test { BlockchainRegistry* registry() { return BlockchainRegistry::GetInstance(); } void SetOnDemandUpdateCallbackWithComponentReady(const base::FilePath& path) { - brave_component_updater::BraveOnDemandUpdater::GetInstance() - ->RegisterOnDemandUpdateCallback( - base::BindLambdaForTesting([path, this](const std::string& id) { - if (id != kComponentId) { - return; - } - + EXPECT_CALL(on_demand_updater_, + OnDemandUpdate(kComponentId, testing::_, testing::_)) + .WillOnce( + [path, this](const std::string& id, + component_updater::OnDemandUpdater::Priority priority, + component_updater::Callback callback) { // Unblock CreateWallet once the component is registered. installer().OnComponentReady(path); - })); - } - - void SetOnDemandUpdateCallbackWithEmptyCallback() { - brave_component_updater::BraveOnDemandUpdater::GetInstance() - ->RegisterOnDemandUpdateCallback(base::DoNothing()); + }); } void SetOnDemandUpdateCallbackWithComponentUpdateError() { - brave_component_updater::BraveOnDemandUpdater::GetInstance() - ->RegisterOnDemandUpdateCallback( - base::BindLambdaForTesting([&](const std::string& id) { - if (id != kComponentId) { - return; - } - - installer().OnEvent(update_client::UpdateClient::Observer:: - Events::COMPONENT_UPDATE_ERROR, - kComponentId); - })); + EXPECT_CALL(on_demand_updater_, + OnDemandUpdate(kComponentId, testing::_, testing::_)) + .WillOnce([this](const std::string& id, + component_updater::OnDemandUpdater::Priority priority, + component_updater::Callback callback) { + installer().OnEvent(update_client::UpdateClient::Observer::Events:: + COMPONENT_UPDATE_ERROR, + kComponentId); + }); } protected: @@ -228,6 +225,7 @@ class WalletDataFilesInstallerUnitTest : public testing::Test { private: base::test::TaskEnvironment task_environment_; + brave_component_updater::MockOnDemandUpdater on_demand_updater_; sync_preferences::TestingPrefServiceSyncable prefs_; sync_preferences::TestingPrefServiceSyncable local_state_; network::TestURLLoaderFactory url_loader_factory_; @@ -254,8 +252,6 @@ TEST_F(WalletDataFilesInstallerUnitTest, EXPECT_CALL(*updater(), RegisterComponent(testing::_)) .Times(1) .WillOnce(testing::Return(true)); - brave_component_updater::BraveOnDemandUpdater::GetInstance() - ->RegisterOnDemandUpdateCallback(base::DoNothing()); // Mimic created wallets. local_state()->SetTime(kBraveWalletLastUnlockTime, base::Time::Now()); installer().MaybeRegisterWalletDataFilesComponent(updater(), local_state()); diff --git a/components/ntp_background_images/browser/ntp_background_images_component_installer.cc b/components/ntp_background_images/browser/ntp_background_images_component_installer.cc index 3d230d667969..bd9db1cd464b 100644 --- a/components/ntp_background_images/browser/ntp_background_images_component_installer.cc +++ b/components/ntp_background_images/browser/ntp_background_images_component_installer.cc @@ -66,6 +66,7 @@ class NTPBackgroundImagesComponentInstallerPolicy void GetHash(std::vector* hash) const override; std::string GetName() const override; update_client::InstallerAttributes GetInstallerAttributes() const override; + bool IsBraveComponent() const override; private: const std::string component_id_; @@ -143,6 +144,10 @@ NTPBackgroundImagesComponentInstallerPolicy::GetInstallerAttributes() const { return update_client::InstallerAttributes(); } +bool NTPBackgroundImagesComponentInstallerPolicy::IsBraveComponent() const { + return true; +} + void OnRegistered(const std::string& component_id) { BraveOnDemandUpdater::GetInstance()->OnDemandUpdate(component_id); } diff --git a/components/playlist/browser/media_detector_component_installer.cc b/components/playlist/browser/media_detector_component_installer.cc index b401f0df5e9e..dd31f4f167ff 100644 --- a/components/playlist/browser/media_detector_component_installer.cc +++ b/components/playlist/browser/media_detector_component_installer.cc @@ -53,6 +53,7 @@ class MediaDetectorComponentInstallerPolicy void GetHash(std::vector* hash) const override; std::string GetName() const override; update_client::InstallerAttributes GetInstallerAttributes() const override; + bool IsBraveComponent() const override; private: static constexpr size_t kHashSize = 32; @@ -136,6 +137,10 @@ MediaDetectorComponentInstallerPolicy::GetInstallerAttributes() const { return update_client::InstallerAttributes(); } +bool MediaDetectorComponentInstallerPolicy::IsBraveComponent() const { + return true; +} + void OnRegisteredToComponentUpdateService() { BraveOnDemandUpdater::GetInstance()->OnDemandUpdate(kComponentID); } diff --git a/components/psst/browser/core/psst_component_installer.cc b/components/psst/browser/core/psst_component_installer.cc index 0275a0323389..0ebc2d844fb7 100644 --- a/components/psst/browser/core/psst_component_installer.cc +++ b/components/psst/browser/core/psst_component_installer.cc @@ -78,6 +78,7 @@ class PsstComponentInstallerPolicy void GetHash(std::vector* hash) const override; std::string GetName() const override; update_client::InstallerAttributes GetInstallerAttributes() const override; + bool IsBraveComponent() const override; private: const std::string component_id_; @@ -140,6 +141,10 @@ PsstComponentInstallerPolicy::GetInstallerAttributes() const { return update_client::InstallerAttributes(); } +bool PsstComponentInstallerPolicy::IsBraveComponent() const { + return true; +} + void RegisterPsstComponent( component_updater::ComponentUpdateService* cus, base::OnceCallback callback) { diff --git a/ios/app/brave_core_main.mm b/ios/app/brave_core_main.mm index ab1c60fc4ed4..92ed8c7a8e54 100644 --- a/ios/app/brave_core_main.mm +++ b/ios/app/brave_core_main.mm @@ -243,9 +243,11 @@ - (instancetype)initWithUserAgent:(NSString*)userAgent component_updater::ComponentUpdateService* cus = GetApplicationContext()->GetComponentUpdateService(); DCHECK(cus); + brave_component_updater::BraveOnDemandUpdater::GetInstance() + ->RegisterOnDemandUpdater(&cus->GetOnDemandUpdater()); + [self registerComponentsForUpdate:cus]; _adblockService = [[AdblockService alloc] initWithComponentUpdater:cus]; - [self registerComponentsForUpdate:cus]; _backgroundImagesService = [[NTPBackgroundImagesService alloc] initWithBackgroundImagesService: @@ -335,10 +337,6 @@ - (void)scheduleLowPriorityStartupTasks { - (void)registerComponentsForUpdate: (component_updater::ComponentUpdateService*)cus { - brave_component_updater::BraveOnDemandUpdater::GetInstance() - ->RegisterOnDemandUpdateCallback( - base::BindRepeating(&component_updater::BraveOnDemandUpdate)); - RegisterSafetyTipsComponent(cus); brave_wallet::WalletDataFilesInstaller::GetInstance() .MaybeRegisterWalletDataFilesComponent( diff --git a/ios/browser/component_updater/BUILD.gn b/ios/browser/component_updater/BUILD.gn index 6ee977e4a324..322df07304cf 100644 --- a/ios/browser/component_updater/BUILD.gn +++ b/ios/browser/component_updater/BUILD.gn @@ -10,7 +10,6 @@ source_set("component_updater") { ] deps = [ "//base", - "//components/component_updater", - "//ios/chrome/browser/shared/model/application_context", + "//brave/components/brave_component_updater/browser", ] } diff --git a/ios/browser/component_updater/component_updater_utils.mm b/ios/browser/component_updater/component_updater_utils.mm index 8a4b40fe0a11..7151d8ed7be7 100644 --- a/ios/browser/component_updater/component_updater_utils.mm +++ b/ios/browser/component_updater/component_updater_utils.mm @@ -5,18 +5,13 @@ #include "brave/ios/browser/component_updater/component_updater_utils.h" -#include "base/functional/callback.h" -#include "components/component_updater/component_updater_service.h" -#include "ios/chrome/browser/shared/model/application_context/application_context.h" +#include "brave/components/brave_component_updater/browser/brave_on_demand_updater.h" namespace component_updater { void BraveOnDemandUpdate(const std::string& component_id) { - component_updater::ComponentUpdateService* cus = - GetApplicationContext()->GetComponentUpdateService(); - cus->GetOnDemandUpdater().OnDemandUpdate( - component_id, component_updater::OnDemandUpdater::Priority::FOREGROUND, - component_updater::Callback()); + brave_component_updater::BraveOnDemandUpdater::GetInstance()->OnDemandUpdate( + component_id); } } // namespace component_updater