Skip to content

Commit

Permalink
Run populate on the original sequence.
Browse files Browse the repository at this point in the history
The CL makes sure that populating the plus address blocklist data
happens from the UI thread.

Bug: 324556906
Change-Id: I51e2cb27bba8b33c3b9cada2c37ddb63e30bea46
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5785893
Commit-Queue: Norge Vizcay <vizcay@google.com>
Reviewed-by: Joshua Pawlicki <waffles@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1340947}
  • Loading branch information
norgevz authored and Chromium LUCI CQ committed Aug 13, 2024
1 parent d05d394 commit 7d90a8a
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "components/component_updater/installer_policies/plus_address_blocklist_component_installer.h"

#include <memory>
#include <optional>
#include <vector>

#include "base/feature_list.h"
Expand Down Expand Up @@ -33,15 +34,24 @@ base::FilePath GetInstalledPath(const base::FilePath& base) {
return base.Append(kPlusAddressBlocklistBinaryPbFileName);
}

void LoadPlusAddressBlocklistFromDisk(const base::FilePath& pb_path) {
std::optional<std::string> LoadPlusAddressBlocklistFromDisk(
const base::FilePath& pb_path) {
std::string binary_pb;
if (!base::ReadFileToString(pb_path, &binary_pb)) {
DVLOG(1) << "Failed reading from " << pb_path.value();
return;
return std::nullopt;
}

return binary_pb;
}

void PopulatePlusAddressBlocklistData(const base::FilePath& pb_path,
std::optional<std::string> binary_pb) {
if (!binary_pb) {
return;
}
bool parsing_result = plus_addresses::PlusAddressBlocklistData::GetInstance()
.PopulateDataFromComponent(binary_pb);
.PopulateDataFromComponent(std::move(*binary_pb));
if (!parsing_result) {
DVLOG(1) << "Failed parsing proto " << pb_path.value();
return;
Expand Down Expand Up @@ -80,10 +90,12 @@ void PlusAddressBlocklistInstallerPolicy::ComponentReady(
base::Value::Dict /* manifest */) {
VLOG(1) << "Component ready, version " << version.GetString() << " in "
<< install_dir.value();
base::ThreadPool::PostTask(
const base::FilePath pb_path = GetInstalledPath(install_dir);

base::ThreadPool::PostTaskAndReplyWithResult(
FROM_HERE, {base::MayBlock(), base::TaskPriority::BEST_EFFORT},
base::BindOnce(&LoadPlusAddressBlocklistFromDisk,
GetInstalledPath(install_dir)));
base::BindOnce(&LoadPlusAddressBlocklistFromDisk, pb_path),
base::BindOnce(&PopulatePlusAddressBlocklistData, pb_path));
}

// Called during startup and installation before ComponentReady().
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,4 +88,20 @@ TEST_F(PlusAddressBlocklistInstallerPolicyTest,
RunUntilIdle();
}

TEST_F(PlusAddressBlocklistInstallerPolicyTest, LoadFileWithData) {
plus_addresses::CompactPlusAddressBlockedFacets blocked_facets;
blocked_facets.set_exclusion_pattern("foo");
blocked_facets.set_exception_pattern("bar");
ASSERT_TRUE(
CreateTestPlusAddressBlocklist(blocked_facets.SerializeAsString()));
ASSERT_NO_FATAL_FAILURE(LoadPlusAddressBlocklist());

const plus_addresses::PlusAddressBlocklistData& blocklist_data =
plus_addresses::PlusAddressBlocklistData::GetInstance();
ASSERT_TRUE(blocklist_data.GetExclusionPattern());
ASSERT_TRUE(blocklist_data.GetExceptionPattern());
EXPECT_EQ(blocklist_data.GetExceptionPattern()->pattern(), "bar");
EXPECT_EQ(blocklist_data.GetExclusionPattern()->pattern(), "foo");
}

} // namespace component_updater

0 comments on commit 7d90a8a

Please sign in to comment.