Skip to content

Commit

Permalink
[WebAPKRestore] Fix racing when removing old apps from sync
Browse files Browse the repository at this point in the history
Bug: 351351687
Change-Id: If8da62f7d6f39b5bfa705b0ece9ff55156e92d1e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5683385
Reviewed-by: Glenn Hartmann <hartmanng@chromium.org>
Commit-Queue: Ella Ge <eirage@chromium.org>
Auto-Submit: Ella Ge <eirage@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1324285}
  • Loading branch information
EiraGe authored and pull[bot] committed Jul 8, 2024
1 parent 43a15f6 commit 1217765
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 10 deletions.
23 changes: 15 additions & 8 deletions chrome/browser/android/webapk/webapk_sync_bridge.cc
Original file line number Diff line number Diff line change
Expand Up @@ -213,8 +213,8 @@ void WebApkSyncBridge::OnDatabaseOpened(

registry_ = std::move(registry);
std::move(callback).Run();
if (init_done_callback_) {
std::move(init_done_callback_).Run(/* initialized= */ true);
for (auto& task : init_done_callback_) {
std::move(task).Run(/* initialized= */ true);
}
}

Expand Down Expand Up @@ -317,7 +317,7 @@ void WebApkSyncBridge::RegisterDoneInitializingCallback(
return;
}

init_done_callback_ = std::move(init_done_callback);
init_done_callback_.push_back(std::move(init_done_callback));
}

void WebApkSyncBridge::MergeSyncDataForTesting(
Expand Down Expand Up @@ -431,7 +431,8 @@ void WebApkSyncBridge::OnWebApkUninstalled(const std::string& manifest_id) {
}

if (!AppWasUsedRecently(&app->sync_data())) {
DeleteAppsFromSync(std::vector<webapps::AppId>{app_id});
DeleteAppsFromSync(std::vector<webapps::AppId>{app_id},
database_.is_opened());
return;
}

Expand Down Expand Up @@ -536,7 +537,10 @@ void WebApkSyncBridge::RemoveOldWebAPKsFromSync(
app_ids.push_back(app_id);
}
}
DeleteAppsFromSync(app_ids);

RegisterDoneInitializingCallback(
base::BindOnce(&WebApkSyncBridge::DeleteAppsFromSync,
weak_ptr_factory_.GetWeakPtr(), app_ids));
}

void WebApkSyncBridge::AddOrModifyAppInSync(std::unique_ptr<WebApkProto> app,
Expand Down Expand Up @@ -568,11 +572,14 @@ void WebApkSyncBridge::AddOrModifyAppInSync(std::unique_ptr<WebApkProto> app,
}

void WebApkSyncBridge::DeleteAppsFromSync(
const std::vector<webapps::AppId>& app_ids) {
if (app_ids.size() > 0) {
RecordSyncedWebApkRemovalCountHistogram(app_ids.size());
const std::vector<webapps::AppId>& app_ids,
bool database_opened) {
if (app_ids.size() == 0 || !database_opened) {
return;
}

RecordSyncedWebApkRemovalCountHistogram(app_ids.size());

std::unique_ptr<syncer::MetadataChangeList> metadata_change_list =
syncer::ModelTypeStore::WriteBatch::CreateMetadataChangeList();
std::unique_ptr<RegistryUpdateData> registry_update =
Expand Down
5 changes: 3 additions & 2 deletions chrome/browser/android/webapk/webapk_sync_bridge.h
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,8 @@ class WebApkSyncBridge : public syncer::ModelTypeSyncBridge {
std::unique_ptr<RegistryUpdateData> update_data);

void AddOrModifyAppInSync(std::unique_ptr<WebApkProto> app, bool is_install);
void DeleteAppsFromSync(const std::vector<webapps::AppId>& app_ids);
void DeleteAppsFromSync(const std::vector<webapps::AppId>& app_ids,
bool database_opened);

void RecordSyncedWebApkAdditionHistogram(bool is_install,
bool already_exists_in_sync) const;
Expand All @@ -144,7 +145,7 @@ class WebApkSyncBridge : public syncer::ModelTypeSyncBridge {
Registry registry_;
std::unique_ptr<base::Clock> clock_;
std::unique_ptr<AbstractWebApkSpecificsFetcher> webapk_specifics_fetcher_;
base::OnceCallback<void(bool)> init_done_callback_;
std::vector<base::OnceCallback<void(bool)>> init_done_callback_;

base::WeakPtrFactory<WebApkSyncBridge> weak_ptr_factory_{this};
};
Expand Down

0 comments on commit 1217765

Please sign in to comment.