Merge pull request #5847 from /issues/10271

ReadyCallback should be a repeating callback
brave · Jun 16, 2020 · a5ff39e · a5ff39e
2 parents 6b5cafb + 619f571
commit a5ff39e
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 16 deletions.
diff --git a/browser/component_updater/brave_component_installer.cc b/browser/component_updater/brave_component_installer.cc
@@ -124,17 +124,9 @@ void BraveComponentInstallerPolicy::ComponentReady(
     const base::Version& version,
     const base::FilePath& install_dir,
     std::unique_ptr<base::DictionaryValue> manifest) {
-  // It appears to be possible for the ComponentInstaller to call
-  // `ComponentReady` more than once. There is a call in
-  // ComponentInstaller::FinishRegistration and another one in
-  // ComponentInstaller::Install. So a call to Register followed by a call
-  // to Install could result in a crash here. See
-  // https://github.com/brave/brave-browser/issues/4624
-  if (!ready_callback_.is_null()) {
-    std::move(ready_callback_).Run(
-          install_dir,
-          GetManifestString(std::move(manifest), base64_public_key_));
-  }
+  ready_callback_.Run(
+      install_dir,
+      GetManifestString(std::move(manifest), base64_public_key_));
 }
 
 base::FilePath BraveComponentInstallerPolicy::GetRelativeInstallDir() const {

diff --git a/components/brave_component_updater/browser/brave_component.cc b/components/brave_component_updater/browser/brave_component.cc
@@ -8,6 +8,7 @@
 #include <utility>
 
 #include "base/bind.h"
+#include "base/logging.h"
 #include "base/sequenced_task_runner.h"
 
 namespace brave_component_updater {
@@ -22,6 +23,7 @@ BraveComponent::~BraveComponent() {
 void BraveComponent::Register(const std::string& component_name,
                               const std::string& component_id,
                               const std::string& component_base64_public_key) {
+  VLOG(2) << "register component: " << component_id;
   component_name_ = component_name;
   component_id_ = component_id;
   component_base64_public_key_ = component_base64_public_key;
@@ -31,24 +33,33 @@ void BraveComponent::Register(const std::string& component_name,
                      delegate_,
                      component_id);
   auto ready_callback =
-      base::BindOnce(&BraveComponent::OnComponentReady,
-                     weak_factory_.GetWeakPtr(),
-                     component_id);
+      base::BindRepeating(&BraveComponent::OnComponentReadyInternal,
+                          weak_factory_.GetWeakPtr(),
+                          component_id);
 
   delegate_->Register(component_name_,
                       component_base64_public_key_,
                       std::move(registered_callback),
-                      std::move(ready_callback));
+                      ready_callback);
 }
 
 bool BraveComponent::Unregister() {
+  VLOG(2) << "unregister component: " << component_id_;
   return delegate_->Unregister(component_id_);
 }
 
 scoped_refptr<base::SequencedTaskRunner> BraveComponent::GetTaskRunner() {
   return delegate_->GetTaskRunner();
 }
 
+void BraveComponent::OnComponentReadyInternal(
+    const std::string& component_id,
+    const base::FilePath& install_dir,
+    const std::string& manifest) {
+  VLOG(2) << "component ready: " << manifest;
+  OnComponentReady(component_id, install_dir, manifest);
+}
+
 void BraveComponent::OnComponentReady(
     const std::string& component_id,
     const base::FilePath& install_dir,
@@ -58,6 +69,7 @@ void BraveComponent::OnComponentReady(
 void BraveComponent::OnComponentRegistered(
     Delegate* delegate,
     const std::string& component_id) {
+  VLOG(2) << "component registered: " << component_id;
   delegate->OnDemandUpdate(component_id);
 }
 

diff --git a/components/brave_component_updater/browser/brave_component.h b/components/brave_component_updater/browser/brave_component.h
@@ -18,7 +18,7 @@ namespace brave_component_updater {
 
 class BraveComponent {
  public:
-  using ReadyCallback = base::OnceCallback<void(const base::FilePath&,
+  using ReadyCallback = base::RepeatingCallback<void(const base::FilePath&,
                                                 const std::string& manifest)>;
   class Delegate {
    public:
@@ -48,6 +48,9 @@ class BraveComponent {
  private:
   static void OnComponentRegistered(Delegate* delegate,
                                     const std::string& component_id);
+  void OnComponentReadyInternal(const std::string& component_id,
+                                const base::FilePath& install_dir,
+                                const std::string& manifest);
 
   std::string component_name_;
   std::string component_id_;