Commit d9d2b1f6 authored by khmel's avatar khmel Committed by Commit bot

arc: Handle default app not availble case.

This CL handles the situation when default app is not available
after Arc starts. In this case, default app entry is removed from
the app launcher.

BUG=b/31934494
TEST=Extended unit tests
TEST=Manually on device.
     Normal flow: Start Arc by clicking default app icon. Spinning
     item appears in shelf for the starting app. Play Store starts,
     spiining item stil exists and installtion stars (view in
     notification). Once default app is installed, it automatically
     pops up and spinning item is replaced with normal icon.
     App is not available flow: Repeat steps above. Installation
     for default app does not appear and spinning item is removed
     from shelf soon after. Default app is also removed from the
     app launcher.
     Re-sign in. In both cases default app state is preserved.

Review-Url: https://codereview.chromium.org/2601323002
Cr-Commit-Position: refs/heads/master@{#442073}
parent 31803621
...@@ -50,6 +50,9 @@ const char kShouldSync[] = "should_sync"; ...@@ -50,6 +50,9 @@ const char kShouldSync[] = "should_sync";
const char kSystem[] = "system"; const char kSystem[] = "system";
const char kUninstalled[] = "uninstalled"; const char kUninstalled[] = "uninstalled";
constexpr base::TimeDelta kDetectDefaultAppAvailabilityTimeout =
base::TimeDelta::FromSeconds(15);
// Provider of write access to a dictionary storing ARC prefs. // Provider of write access to a dictionary storing ARC prefs.
class ScopedArcPrefUpdate : public DictionaryPrefUpdate { class ScopedArcPrefUpdate : public DictionaryPrefUpdate {
public: public:
...@@ -397,6 +400,10 @@ std::unique_ptr<ArcAppListPrefs::PackageInfo> ArcAppListPrefs::GetPackage( ...@@ -397,6 +400,10 @@ std::unique_ptr<ArcAppListPrefs::PackageInfo> ArcAppListPrefs::GetPackage(
!packages->GetDictionaryWithoutPathExpansion(package_name, &package)) !packages->GetDictionaryWithoutPathExpansion(package_name, &package))
return std::unique_ptr<PackageInfo>(); return std::unique_ptr<PackageInfo>();
bool uninstalled = false;
if (package->GetBoolean(kUninstalled, &uninstalled) && uninstalled)
return nullptr;
int32_t package_version = 0; int32_t package_version = 0;
int64_t last_backup_android_id = 0; int64_t last_backup_android_id = 0;
int64_t last_backup_time = 0; int64_t last_backup_time = 0;
...@@ -657,6 +664,13 @@ void ArcAppListPrefs::SetDefaltAppsReadyCallback(base::Closure callback) { ...@@ -657,6 +664,13 @@ void ArcAppListPrefs::SetDefaltAppsReadyCallback(base::Closure callback) {
default_apps_ready_callback_.Run(); default_apps_ready_callback_.Run();
} }
void ArcAppListPrefs::SimulateDefaultAppAvailabilityTimeoutForTesting() {
if (!detect_default_app_availability_timeout_.IsRunning())
return;
detect_default_app_availability_timeout_.Stop();
DetectDefaultAppAvailability();
}
void ArcAppListPrefs::OnInstanceReady() { void ArcAppListPrefs::OnInstanceReady() {
// Init() is also available at version 0. // Init() is also available at version 0.
arc::mojom::AppInstance* app_instance = arc::mojom::AppInstance* app_instance =
...@@ -678,6 +692,8 @@ void ArcAppListPrefs::OnInstanceReady() { ...@@ -678,6 +692,8 @@ void ArcAppListPrefs::OnInstanceReady() {
void ArcAppListPrefs::OnInstanceClosed() { void ArcAppListPrefs::OnInstanceClosed() {
DisableAllApps(); DisableAllApps();
installing_packages_count_ = 0; installing_packages_count_ = 0;
default_apps_installations_.clear();
detect_default_app_availability_timeout_.Stop();
binding_.Close(); binding_.Close();
if (sync_service_) { if (sync_service_) {
...@@ -905,10 +921,31 @@ void ArcAppListPrefs::OnAppListRefreshed( ...@@ -905,10 +921,31 @@ void ArcAppListPrefs::OnAppListRefreshed(
if (!is_initialized_) { if (!is_initialized_) {
is_initialized_ = true; is_initialized_ = true;
MaybeSetDefaultAppLoadingTimeout();
UMA_HISTOGRAM_COUNTS_1000("Arc.AppsInstalledAtStartup", ready_apps_.size()); UMA_HISTOGRAM_COUNTS_1000("Arc.AppsInstalledAtStartup", ready_apps_.size());
} }
} }
void ArcAppListPrefs::DetectDefaultAppAvailability() {
for (const auto& package : default_apps_.GetActivePackages()) {
// Check if already installed or installation in progress.
if (!GetPackage(package) && !default_apps_installations_.count(package))
HandlePackageRemoved(package);
}
}
void ArcAppListPrefs::MaybeSetDefaultAppLoadingTimeout() {
// Find at least one not installed default app package.
for (const auto& package : default_apps_.GetActivePackages()) {
if (!GetPackage(package)) {
detect_default_app_availability_timeout_.Start(FROM_HERE,
kDetectDefaultAppAvailabilityTimeout, this,
&ArcAppListPrefs::DetectDefaultAppAvailability);
break;
}
}
}
void ArcAppListPrefs::OnTaskOrientationLockRequested( void ArcAppListPrefs::OnTaskOrientationLockRequested(
int32_t task_id, int32_t task_id,
const arc::mojom::OrientationLock orientation_lock) { const arc::mojom::OrientationLock orientation_lock) {
...@@ -1007,13 +1044,18 @@ std::unordered_set<std::string> ArcAppListPrefs::GetAppsForPackage( ...@@ -1007,13 +1044,18 @@ std::unordered_set<std::string> ArcAppListPrefs::GetAppsForPackage(
return app_set; return app_set;
} }
void ArcAppListPrefs::OnPackageRemoved(const std::string& package_name) { void ArcAppListPrefs::HandlePackageRemoved(const std::string& package_name) {
const std::unordered_set<std::string> apps_to_remove = const std::unordered_set<std::string> apps_to_remove =
GetAppsForPackage(package_name); GetAppsForPackage(package_name);
for (const auto& app_id : apps_to_remove) for (const auto& app_id : apps_to_remove)
RemoveApp(app_id); RemoveApp(app_id);
RemovePackageFromPrefs(prefs_, package_name); RemovePackageFromPrefs(prefs_, package_name);
}
void ArcAppListPrefs::OnPackageRemoved(const std::string& package_name) {
HandlePackageRemoved(package_name);
for (auto& observer : observer_list_) for (auto& observer : observer_list_)
observer.OnPackageRemoved(package_name); observer.OnPackageRemoved(package_name);
} }
...@@ -1252,14 +1294,26 @@ void ArcAppListPrefs::OnIconInstalled(const std::string& app_id, ...@@ -1252,14 +1294,26 @@ void ArcAppListPrefs::OnIconInstalled(const std::string& app_id,
observer.OnAppIconUpdated(app_id, scale_factor); observer.OnAppIconUpdated(app_id, scale_factor);
} }
void ArcAppListPrefs::OnInstallationStarted() { void ArcAppListPrefs::OnInstallationStarted(
const base::Optional<std::string>& package_name) {
// Start new batch installation group if this is first installation. // Start new batch installation group if this is first installation.
if (!installing_packages_count_) if (!installing_packages_count_)
++current_batch_installation_revision_; ++current_batch_installation_revision_;
++installing_packages_count_; ++installing_packages_count_;
if (package_name.has_value() && default_apps_.HasPackage(*package_name))
default_apps_installations_.insert(*package_name);
} }
void ArcAppListPrefs::OnInstallationFinished() { void ArcAppListPrefs::OnInstallationFinished(
arc::mojom::InstallationResultPtr result) {
if (result && default_apps_.HasPackage(result->package_name)) {
default_apps_installations_.erase(result->package_name);
if (!result->success && !GetPackage(result->package_name))
HandlePackageRemoved(result->package_name);
}
if (!installing_packages_count_) { if (!installing_packages_count_) {
VLOG(2) << "Received unexpected installation finished event"; VLOG(2) << "Received unexpected installation finished event";
return; return;
......
...@@ -19,6 +19,7 @@ ...@@ -19,6 +19,7 @@
#include "base/observer_list.h" #include "base/observer_list.h"
#include "base/optional.h" #include "base/optional.h"
#include "base/time/time.h" #include "base/time/time.h"
#include "base/timer/timer.h"
#include "chrome/browser/chromeos/arc/arc_session_manager.h" #include "chrome/browser/chromeos/arc/arc_session_manager.h"
#include "chrome/browser/ui/app_list/arc/arc_default_app_list.h" #include "chrome/browser/ui/app_list/arc/arc_default_app_list.h"
#include "components/arc/common/app.mojom.h" #include "components/arc/common/app.mojom.h"
...@@ -244,6 +245,7 @@ class ArcAppListPrefs ...@@ -244,6 +245,7 @@ class ArcAppListPrefs
const std::string& package_name) const; const std::string& package_name) const;
void SetDefaltAppsReadyCallback(base::Closure callback); void SetDefaltAppsReadyCallback(base::Closure callback);
void SimulateDefaultAppAvailabilityTimeoutForTesting();
private: private:
friend class ChromeLauncherControllerImplTest; friend class ChromeLauncherControllerImplTest;
...@@ -288,9 +290,10 @@ class ArcAppListPrefs ...@@ -288,9 +290,10 @@ class ArcAppListPrefs
void OnTaskOrientationLockRequested( void OnTaskOrientationLockRequested(
int32_t task_id, int32_t task_id,
const arc::mojom::OrientationLock orientation_lock) override; const arc::mojom::OrientationLock orientation_lock) override;
void OnInstallationStarted(
void OnInstallationStarted() override; const base::Optional<std::string>& package_name) override;
void OnInstallationFinished() override; void OnInstallationFinished(
arc::mojom::InstallationResultPtr result) override;
void StartPrefs(); void StartPrefs();
...@@ -356,6 +359,16 @@ class ArcAppListPrefs ...@@ -356,6 +359,16 @@ class ArcAppListPrefs
// and it is not scheduled to install by sync. // and it is not scheduled to install by sync.
bool IsUnknownPackage(const std::string& package_name) const; bool IsUnknownPackage(const std::string& package_name) const;
// Detects that default apps either exist or installation session is started.
void DetectDefaultAppAvailability();
// Performs data clean up for removed package.
void HandlePackageRemoved(const std::string& package_name);
// Sets timeout to wait for default app installed or installation started if
// some default app is not available yet.
void MaybeSetDefaultAppLoadingTimeout();
Profile* const profile_; Profile* const profile_;
// Owned by the BrowserContext. // Owned by the BrowserContext.
...@@ -384,6 +397,13 @@ class ArcAppListPrefs ...@@ -384,6 +397,13 @@ class ArcAppListPrefs
bool apps_restored_ = false; bool apps_restored_ = false;
// True is Arc package list has been refreshed once. // True is Arc package list has been refreshed once.
bool package_list_initial_refreshed_ = false; bool package_list_initial_refreshed_ = false;
// Play Store does not have publicly available observers for default app
// installations. This timeout is for validating default app availability.
// Default apps should be either already installed or their installations
// should be started soon after initial app list refresh.
base::OneShotTimer detect_default_app_availability_timeout_;
// Set of currently installing default apps_.
std::unordered_set<std::string> default_apps_installations_;
arc::ArcPackageSyncableService* sync_service_ = nullptr; arc::ArcPackageSyncableService* sync_service_ = nullptr;
......
...@@ -1288,6 +1288,65 @@ TEST_F(ArcDefaulAppTest, DefaultApps) { ...@@ -1288,6 +1288,65 @@ TEST_F(ArcDefaulAppTest, DefaultApps) {
} }
} }
TEST_F(ArcDefaulAppTest, DefaultAppsNotAvailable) {
ArcAppListPrefs* prefs = ArcAppListPrefs::Get(profile_.get());
ASSERT_NE(nullptr, prefs);
ValidateHaveApps(fake_default_apps());
const std::vector<arc::mojom::AppInfo> empty_app_list;
app_instance()->RefreshAppList();
app_instance()->SendRefreshAppList(empty_app_list);
ValidateHaveApps(fake_default_apps());
prefs->SimulateDefaultAppAvailabilityTimeoutForTesting();
// No default app installation and already installed packages.
ValidateHaveApps(empty_app_list);
}
TEST_F(ArcDefaulAppTest, DefaultAppsInstallation) {
ArcAppListPrefs* prefs = ArcAppListPrefs::Get(profile_.get());
ASSERT_NE(nullptr, prefs);
const std::vector<arc::mojom::AppInfo> empty_app_list;
ValidateHaveApps(fake_default_apps());
app_instance()->RefreshAppList();
app_instance()->SendRefreshAppList(empty_app_list);
ValidateHaveApps(fake_default_apps());
// Notify that default installations have been started.
for (const auto& fake_app : fake_default_apps())
app_instance()->SendInstallationStarted(fake_app.package_name);
// Timeout does not affect default app availability because all installations
// for default apps have been started.
prefs->SimulateDefaultAppAvailabilityTimeoutForTesting();
ValidateHaveApps(fake_default_apps());
const arc::mojom::AppInfo& app_last = fake_default_apps().back();
std::vector<arc::mojom::AppInfo> available_apps = fake_default_apps();
available_apps.pop_back();
for (const auto& fake_app : available_apps)
app_instance()->SendInstallationFinished(fake_app.package_name, true);
// So far we have all default apps available because not all installations
// completed.
ValidateHaveApps(fake_default_apps());
// Last default app installation failed.
app_instance()->SendInstallationFinished(app_last.package_name, false);
// We should have all default apps except last.
ValidateHaveApps(available_apps);
}
TEST_F(ArcDefaulAppForManagedUserTest, DefaultAppsForManagedUser) { TEST_F(ArcDefaulAppForManagedUserTest, DefaultAppsForManagedUser) {
const ArcAppListPrefs* const prefs = ArcAppListPrefs::Get(profile_.get()); const ArcAppListPrefs* const prefs = ArcAppListPrefs::Get(profile_.get());
ASSERT_TRUE(prefs); ASSERT_TRUE(prefs);
......
...@@ -201,6 +201,15 @@ void ArcDefaultAppList::MaybeMarkPackageUninstalled( ...@@ -201,6 +201,15 @@ void ArcDefaultAppList::MaybeMarkPackageUninstalled(
it->second = uninstalled; it->second = uninstalled;
} }
std::unordered_set<std::string> ArcDefaultAppList::GetActivePackages() const {
std::unordered_set<std::string> result;
for (const auto& package_info : packages_) {
if (!package_info.second)
result.insert(package_info.first);
}
return result;
}
ArcDefaultAppList::AppInfo::AppInfo(const std::string& name, ArcDefaultAppList::AppInfo::AppInfo(const std::string& name,
const std::string& package_name, const std::string& package_name,
const std::string& activity, const std::string& activity,
......
...@@ -10,6 +10,7 @@ ...@@ -10,6 +10,7 @@
#include <map> #include <map>
#include <memory> #include <memory>
#include <string> #include <string>
#include <unordered_set>
#include "base/files/file_path.h" #include "base/files/file_path.h"
#include "base/macros.h" #include "base/macros.h"
...@@ -67,6 +68,9 @@ class ArcDefaultAppList { ...@@ -67,6 +68,9 @@ class ArcDefaultAppList {
void MaybeMarkPackageUninstalled(const std::string& package_name, void MaybeMarkPackageUninstalled(const std::string& package_name,
bool uninstalled); bool uninstalled);
// Returns set of packages which are marked not as uninstalled.
std::unordered_set<std::string> GetActivePackages() const;
const AppInfoMap& app_map() const { return apps_; } const AppInfoMap& app_map() const { return apps_; }
// Marks default apps as hidden for user, for example in case Arc is managed // Marks default apps as hidden for user, for example in case Arc is managed
...@@ -76,7 +80,7 @@ class ArcDefaultAppList { ...@@ -76,7 +80,7 @@ class ArcDefaultAppList {
private: private:
// Defines mapping package name to uninstalled state. // Defines mapping package name to uninstalled state.
using PacakageMap = std::map<std::string, bool>; using PackageMap = std::map<std::string, bool>;
// Called when default apps are read. // Called when default apps are read.
void OnAppsReady(std::unique_ptr<AppInfoMap> apps); void OnAppsReady(std::unique_ptr<AppInfoMap> apps);
...@@ -87,7 +91,7 @@ class ArcDefaultAppList { ...@@ -87,7 +91,7 @@ class ArcDefaultAppList {
bool hidden_ = true; bool hidden_ = true;
AppInfoMap apps_; AppInfoMap apps_;
PacakageMap packages_; PackageMap packages_;
base::WeakPtrFactory<ArcDefaultAppList> weak_ptr_factory_; base::WeakPtrFactory<ArcDefaultAppList> weak_ptr_factory_;
......
...@@ -227,13 +227,17 @@ class ArcAppLauncherBrowserTest : public ExtensionBrowserTest { ...@@ -227,13 +227,17 @@ class ArcAppLauncherBrowserTest : public ExtensionBrowserTest {
app_host()->OnPackageRemoved(package_name); app_host()->OnPackageRemoved(package_name);
} }
void SendInstallationStarted() { void SendInstallationStarted(const std::string& package_name) {
app_host()->OnInstallationStarted(); app_host()->OnInstallationStarted(package_name);
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
} }
void SendInstallationFinished() { void SendInstallationFinished(const std::string& package_name, bool success) {
app_host()->OnInstallationFinished(); arc::mojom::InstallationResult result;
result.package_name = package_name;
result.success = success;
app_host()->OnInstallationFinished(
arc::mojom::InstallationResultPtr(result.Clone()));
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
} }
...@@ -417,11 +421,11 @@ IN_PROC_BROWSER_TEST_F(ArcAppLauncherBrowserTest, AppListShown) { ...@@ -417,11 +421,11 @@ IN_PROC_BROWSER_TEST_F(ArcAppLauncherBrowserTest, AppListShown) {
EXPECT_FALSE(app_list_service->IsAppListVisible()); EXPECT_FALSE(app_list_service->IsAppListVisible());
SendInstallationStarted(); SendInstallationStarted(kTestAppPackage);
SendInstallationStarted(); SendInstallationStarted(kTestAppPackage2);
// New package is available. Show app list. // New package is available. Show app list.
SendInstallationFinished(); SendInstallationFinished(kTestAppPackage, true);
InstallTestApps(kTestAppPackage, false); InstallTestApps(kTestAppPackage, false);
SendPackageAdded(kTestAppPackage, true); SendPackageAdded(kTestAppPackage, true);
EXPECT_TRUE(app_list_service->IsAppListVisible()); EXPECT_TRUE(app_list_service->IsAppListVisible());
...@@ -435,14 +439,14 @@ IN_PROC_BROWSER_TEST_F(ArcAppLauncherBrowserTest, AppListShown) { ...@@ -435,14 +439,14 @@ IN_PROC_BROWSER_TEST_F(ArcAppLauncherBrowserTest, AppListShown) {
// Install next package from batch. Next new package is available. // Install next package from batch. Next new package is available.
// Don't show app list. // Don't show app list.
SendInstallationFinished(); SendInstallationFinished(kTestAppPackage2, true);
InstallTestApps(kTestAppPackage2, false); InstallTestApps(kTestAppPackage2, false);
SendPackageAdded(kTestAppPackage2, true); SendPackageAdded(kTestAppPackage2, true);
EXPECT_FALSE(app_list_service->IsAppListVisible()); EXPECT_FALSE(app_list_service->IsAppListVisible());
// Run next installation batch. App list should be shown again. // Run next installation batch. App list should be shown again.
SendInstallationStarted(); SendInstallationStarted(kTestAppPackage3);
SendInstallationFinished(); SendInstallationFinished(kTestAppPackage3, true);
InstallTestApps(kTestAppPackage3, false); InstallTestApps(kTestAppPackage3, false);
SendPackageAdded(kTestAppPackage3, true); SendPackageAdded(kTestAppPackage3, true);
EXPECT_TRUE(app_list_service->IsAppListVisible()); EXPECT_TRUE(app_list_service->IsAppListVisible());
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
// //
// Next MinVersion: 17 // Next MinVersion: 18
module arc.mojom; module arc.mojom;
...@@ -31,6 +31,12 @@ enum OrientationLock { ...@@ -31,6 +31,12 @@ enum OrientationLock {
LANDSCAPE_SECONDARY = 7, LANDSCAPE_SECONDARY = 7,
}; };
// Describes installation result.
struct InstallationResult {
string package_name;
bool success; // true if app was installed successfully.
};
// Describes ARC app. // Describes ARC app.
struct AppInfo { struct AppInfo {
string name; string name;
...@@ -129,11 +135,15 @@ interface AppHost { ...@@ -129,11 +135,15 @@ interface AppHost {
// Notifies that an application shortcut needs to be created. // Notifies that an application shortcut needs to be created.
[MinVersion=9] OnInstallShortcut@11(ShortcutInfo shortcut); [MinVersion=9] OnInstallShortcut@11(ShortcutInfo shortcut);
// Notifies that Play Store installation has been started. // Notifies that Play Store installation has been started. |package_name|
[MinVersion=16] OnInstallationStarted@14(); // specifies installation package
[MinVersion=16] OnInstallationStarted@14(
[MinVersion=17] string? package_name@0);
// Notifies that Play Store installation is finished. // Notifies that Play Store installation is finished. |result| contains
[MinVersion=16] OnInstallationFinished@15(); // details of installation result.
[MinVersion=16] OnInstallationFinished@15(
[MinVersion=17] InstallationResult? result@1);
// Notifies that task requested orientation lock. // Notifies that task requested orientation lock.
[MinVersion=12] OnTaskOrientationLockRequested@12(int32 task_id, [MinVersion=12] OnTaskOrientationLockRequested@12(int32 task_id,
......
...@@ -191,6 +191,19 @@ void FakeAppInstance::SendPackageUninstalled(const std::string& package_name) { ...@@ -191,6 +191,19 @@ void FakeAppInstance::SendPackageUninstalled(const std::string& package_name) {
app_host_->OnPackageRemoved(package_name); app_host_->OnPackageRemoved(package_name);
} }
void FakeAppInstance::SendInstallationStarted(const std::string& package_name) {
app_host_->OnInstallationStarted(package_name);
}
void FakeAppInstance::SendInstallationFinished(const std::string& package_name,
bool success) {
mojom::InstallationResult result;
result.package_name = package_name;
result.success = success;
app_host_->OnInstallationFinished(
mojom::InstallationResultPtr(result.Clone()));
}
void FakeAppInstance::CanHandleResolution( void FakeAppInstance::CanHandleResolution(
const std::string& package_name, const std::string& package_name,
const std::string& activity, const std::string& activity,
......
...@@ -135,6 +135,10 @@ class FakeAppInstance : public mojom::AppInstance { ...@@ -135,6 +135,10 @@ class FakeAppInstance : public mojom::AppInstance {
void SendPackageAdded(const mojom::ArcPackageInfo& package); void SendPackageAdded(const mojom::ArcPackageInfo& package);
void SendPackageUninstalled(const std::string& pacakge_name); void SendPackageUninstalled(const std::string& pacakge_name);
void SendInstallationStarted(const std::string& package_name);
void SendInstallationFinished(const std::string& package_name,
bool success);
int refresh_app_list_count() const { return refresh_app_list_count_; } int refresh_app_list_count() const { return refresh_app_list_count_; }
const std::vector<std::unique_ptr<Request>>& launch_requests() const { const std::vector<std::unique_ptr<Request>>& launch_requests() const {
......
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