Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ReadyCallback should be a repeating callback (uplift to 1.11.x) #5866

Merged
merged 1 commit into from
Jun 24, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 3 additions & 11 deletions browser/component_updater/brave_component_installer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
20 changes: 16 additions & 4 deletions components/brave_component_updater/browser/brave_component.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <utility>

#include "base/bind.h"
#include "base/logging.h"
#include "base/sequenced_task_runner.h"

namespace brave_component_updater {
Expand All @@ -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;
Expand All @@ -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,
Expand All @@ -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);
}

Expand Down
5 changes: 4 additions & 1 deletion components/brave_component_updater/browser/brave_component.h
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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_;
Expand Down