-
Notifications
You must be signed in to change notification settings - Fork 964
Support OnDemandUpdate with multiple components at once. #23169
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
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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)); | ||
goodov marked this conversation as resolved.
Show resolved
Hide resolved
|
||
auto update_complete_callback = base::BindOnce( | ||
&CrxUpdateService::OnUpdateComplete, base::Unretained(this), | ||
goodov marked this conversation as resolved.
Show resolved
Hide resolved
|
||
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 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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_ |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does it work well if There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
it should. This |
||
} | ||
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_); | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this function will be removed in a follow up PR. It's left for now to not overcomplicate the PR.