Commit aee1785a authored by Alexey Baskakov's avatar Alexey Baskakov Committed by Commit Bot

WebApp: Make sure that ManifestUpdateManager works in the provider ready scope.

Make sure that ManifestUpdateManager doesn't cause FinalizeUpdate while
web apps system is not ready.

Note: we should do
tasks_.clear();
in Shutdown to destroy all ManifestUpdateTasks.
That's how we unsubscribe tasks from WebAppUiManagerImpl:

void ManifestUpdateTask::UpdateAfterWindowsClose() {
...
  ui_manager_.NotifyOnAllAppWindowsClosed(
      app_id_,
      base::BindOnce(&ManifestUpdateTask::OnAllAppWindowsClosed,
      AsWeakPtr()));
}

In next CL: Plumb WebAppUiManagerImpl Startup/Shutdown.

Bug: 1084939
Change-Id: I25112df44af57606e38bcc6c12f12f67bd94c8b9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2212197
Commit-Queue: Alexey Baskakov <loyso@chromium.org>
Reviewed-by: default avatarAlan Cutter <alancutter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#771494}
parent 1b87679b
...@@ -36,17 +36,25 @@ void ManifestUpdateManager::SetSubsystems( ...@@ -36,17 +36,25 @@ void ManifestUpdateManager::SetSubsystems(
void ManifestUpdateManager::Start() { void ManifestUpdateManager::Start() {
registrar_observer_.Add(registrar_); registrar_observer_.Add(registrar_);
DCHECK(!started_);
started_ = true;
} }
void ManifestUpdateManager::Shutdown() { void ManifestUpdateManager::Shutdown() {
registrar_observer_.RemoveAll(); registrar_observer_.RemoveAll();
tasks_.clear();
started_ = false;
} }
void ManifestUpdateManager::MaybeUpdate(const GURL& url, void ManifestUpdateManager::MaybeUpdate(const GURL& url,
const AppId& app_id, const AppId& app_id,
content::WebContents* web_contents) { content::WebContents* web_contents) {
if (!base::FeatureList::IsEnabled(features::kDesktopPWAsLocalUpdating)) if (!started_ ||
!base::FeatureList::IsEnabled(features::kDesktopPWAsLocalUpdating)) {
return; return;
}
if (app_id.empty() || !registrar_->IsLocallyInstalled(app_id)) { if (app_id.empty() || !registrar_->IsLocallyInstalled(app_id)) {
NotifyResult(url, ManifestUpdateResult::kNoAppInScope); NotifyResult(url, ManifestUpdateResult::kNoAppInScope);
...@@ -82,6 +90,8 @@ void ManifestUpdateManager::MaybeUpdate(const GURL& url, ...@@ -82,6 +90,8 @@ void ManifestUpdateManager::MaybeUpdate(const GURL& url,
// AppRegistrarObserver: // AppRegistrarObserver:
void ManifestUpdateManager::OnWebAppUninstalled(const AppId& app_id) { void ManifestUpdateManager::OnWebAppUninstalled(const AppId& app_id) {
DCHECK(started_);
auto it = tasks_.find(app_id); auto it = tasks_.find(app_id);
if (it != tasks_.end()) { if (it != tasks_.end()) {
NotifyResult(it->second->url(), ManifestUpdateResult::kAppUninstalled); NotifyResult(it->second->url(), ManifestUpdateResult::kAppUninstalled);
......
...@@ -96,6 +96,7 @@ class ManifestUpdateManager final : public AppRegistrarObserver { ...@@ -96,6 +96,7 @@ class ManifestUpdateManager final : public AppRegistrarObserver {
base::Optional<base::Time> time_override_for_testing_; base::Optional<base::Time> time_override_for_testing_;
ResultCallback result_callback_for_testing_; ResultCallback result_callback_for_testing_;
bool started_ = false;
bool hang_update_checks_for_testing_ = false; bool hang_update_checks_for_testing_ = false;
}; };
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment