Commit 2e140ce6 authored by Nigel Tao's avatar Nigel Tao Committed by Commit Bot

Don't conflate ARC++'s "ready" with "installed"

A previous commit mistakenly conflated "ready" (an ARC++ concept) with
"installed" (an App Service concept), or equivalently (in the App
Service model), "ready to run". As a consequence, with the App Service
enabled, ARC++ apps weren't shown in the app list UI for 5 or more
seconds after log-in, as it waiting for the ARC++ connection to become
"ready". This did not match the behavior with the App Service disabled.

The App Service doesn't actually have to worry about ARC++'s readiness.
For example, ArcApps::Launch (in App Service code) calls arc::LaunchApp
in (ARC++ code), which already handles the "!app_info->ready" case:
https://cs.chromium.org/chromium/src/chrome/browser/ui/app_list/arc/arc_app_utils.cc?q=app_info-%3Eready&l=277

BUG=826982

Change-Id: I79f8adc7a3c17dfa700e572494dc0a9e05b6f429
Reviewed-on: https://chromium-review.googlesource.com/c/1449431Reviewed-by: default avatarDominick Ng <dominickn@chromium.org>
Commit-Queue: Nigel Tao <nigeltao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#628545}
parent 6fbdadf3
......@@ -281,23 +281,19 @@ void ArcApps::OnConnectionReady() {
void ArcApps::OnAppRegistered(const std::string& app_id,
const ArcAppListPrefs::AppInfo& app_info) {
OnAppStatesChanged(app_id, app_info);
Publish(Convert(app_id, app_info));
}
void ArcApps::OnAppStatesChanged(const std::string& app_id,
const ArcAppListPrefs::AppInfo& app_info) {
apps::mojom::AppPtr app = apps::mojom::App::New();
app->app_type = apps::mojom::AppType::kArc;
app->app_id = app_id;
app->readiness = NewReadiness(app_info.ready);
Publish(std::move(app));
Publish(Convert(app_id, app_info));
}
void ArcApps::OnAppRemoved(const std::string& app_id) {
apps::mojom::AppPtr app = apps::mojom::App::New();
app->app_type = apps::mojom::AppType::kArc;
app->app_id = app_id;
app->readiness = NewReadiness(false);
app->readiness = apps::mojom::Readiness::kUninstalledByUser;
Publish(std::move(app));
}
......@@ -386,7 +382,9 @@ apps::mojom::AppPtr ArcApps::Convert(const std::string& app_id,
app->app_type = apps::mojom::AppType::kArc;
app->app_id = app_id;
app->readiness = NewReadiness(app_info.ready);
// TODO(crbug.com/826982): examine app_info.suspended, and possibly have a
// corresponding 'suspended' apps::mojom::Readiness enum value??
app->readiness = apps::mojom::Readiness::kReady;
app->name = app_info.name;
app->icon_key = NewIconKey(app_id);
......@@ -417,15 +415,6 @@ apps::mojom::IconKeyPtr ArcApps::NewIconKey(const std::string& app_id) {
return icon_key;
}
// static
apps::mojom::Readiness ArcApps::NewReadiness(bool ready) {
// TODO(crbug.com/826982): examine ArcAppListPrefs::AppInfo::suspended, and
// possibly have a corresponding 'suspended' apps::mojom::Readiness enum
// value.
return ready ? apps::mojom::Readiness::kReady
: apps::mojom::Readiness::kUninstalledByUser;
}
void ArcApps::Publish(apps::mojom::AppPtr app) {
subscribers_.ForAllPtrs([&app](apps::mojom::Subscriber* subscriber) {
std::vector<apps::mojom::AppPtr> apps;
......
......@@ -86,7 +86,6 @@ class ArcApps : public KeyedService,
apps::mojom::AppPtr Convert(const std::string& app_id,
const ArcAppListPrefs::AppInfo& app_info);
apps::mojom::IconKeyPtr NewIconKey(const std::string& app_id);
static apps::mojom::Readiness NewReadiness(bool ready);
void Publish(apps::mojom::AppPtr app);
mojo::Binding<apps::mojom::Publisher> binding_;
......
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