Support OnDemandUpdate with multiple components at once. (#23169)

* Support OnDemandUpdate with multiple components at once. * Review fixes. * Mark two missed components as Brave.
brave · May 3, 2024 · 24303f6 · 24303f6
1 parent c745791
commit 24303f6
Show file tree

Hide file tree

Showing 32 changed files with 362 additions and 122 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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<std::string>& 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<std::string>& 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
@@ -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<std::string>& 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
@@ -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<std::string>& 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
@@ -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 <optional>
 
+#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<Configurator> 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,25 +84,36 @@ 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<std::string> 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<UpdateContext> 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
       // it. This is instead done by UpdateEngine, which calls us.
       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
@@ -13,6 +13,10 @@
 
 namespace update_client {
 
+bool CrxInstaller::IsBraveComponent() const {
+  return false;
+}
+
 scoped_refptr<UpdateClient> UpdateClientFactory(
     scoped_refptr<Configurator> 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
@@ -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<Configurator> config); \
   scoped_refptr<UpdateClient> 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
@@ -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",
+  ]
+}