Skip to content

Commit

Permalink
Installation functionality for Crostini App search
Browse files Browse the repository at this point in the history
CL updates the Crostini Repository Search Results in the app launcher
to install Linux packages when clicked. This contains the relevant
app launcher and crostini manager changes. This feature is still behind
a flag and there are plans to incorporate an installation dialogue
into the installation process.

Bug: 921429
Change-Id: I17eeb6af488e56be2d9eb34edefb0194975839c8
Reviewed-on: https://chromium-review.googlesource.com/c/1444901
Commit-Queue: Daniel Ng <danielng@google.com>
Reviewed-by: Timothy Loh <timloh@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#628648}
  • Loading branch information
BipBopBoop authored and Commit Bot committed Feb 4, 2019
1 parent 58322e3 commit 4d5a709
Show file tree
Hide file tree
Showing 14 changed files with 390 additions and 10 deletions.
6 changes: 5 additions & 1 deletion chrome/app/chromeos_strings.grdp
Original file line number Diff line number Diff line change
Expand Up @@ -3507,9 +3507,13 @@
An error occurred during uninstallation. Please uninstall through the Terminal.
</message>

<message name="IDS_CROSTINI_REPOSITORY_SEARCH_RESULT_PLACEHOLDER_TEXT" translateable="false" desc="Placeholder text for surfaced Crostini Repository Search results in the app launcher">
<!-- Crostini Repository Search -->
<message name="IDS_CROSTINI_REPOSITORY_SEARCH_RESULT_TITLE_PLACEHOLDER_TEXT" translateable="false" desc="Placeholder title text for surfaced Crostini Repository Search results in the app launcher">
Install <ph name="LINUX_APP_NAME">$1<ex>GIMP</ex></ph>
</message>
<message name="IDS_CROSTINI_REPOSITORY_SEARCH_RESULT_DETAILS_PLACEHOLDER_TEXT" translateable="false" desc="Placeholder description details for surfaced Crostini Repository Search results in the app launcher">
Crostini Application
</message>

<!-- Time limit notification -->
<message name="IDS_SCREEN_TIME_NOTIFICATION_TITLE" desc="The title of the notification when screen usage limit reaches before locking the device.">
Expand Down
45 changes: 45 additions & 0 deletions chrome/browser/chromeos/crostini/crostini_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -474,6 +474,7 @@ void CrostiniManager::AddRunningVmForTesting(std::string vm_name) {
}

LinuxPackageInfo::LinuxPackageInfo() = default;
LinuxPackageInfo::LinuxPackageInfo(const LinuxPackageInfo&) = default;
LinuxPackageInfo::~LinuxPackageInfo() = default;

ContainerInfo::ContainerInfo(std::string container_name,
Expand Down Expand Up @@ -1036,6 +1037,23 @@ void CrostiniManager::GetLinuxPackageInfo(
weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
}

void CrostiniManager::GetLinuxPackageInfoFromApt(
const std::string& vm_name,
const std::string& container_name,
const std::string& package_name,
GetLinuxPackageInfoCallback callback) {
vm_tools::cicerone::LinuxPackageInfoFromAptRequest request;
request.set_owner_id(owner_id_);
request.set_vm_name(vm_name);
request.set_container_name(container_name);
request.set_package_name(package_name);

GetCiceroneClient()->GetLinuxPackageInfoFromApt(
std::move(request),
base::BindOnce(&CrostiniManager::OnGetLinuxPackageInfo,
weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
}

void CrostiniManager::InstallLinuxPackage(
std::string vm_name,
std::string container_name,
Expand All @@ -1062,6 +1080,32 @@ void CrostiniManager::InstallLinuxPackage(
weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
}

void CrostiniManager::InstallLinuxPackageFromApt(
const std::string& vm_name,
const std::string& container_name,
const std::string& package_id,
InstallLinuxPackageCallback callback) {
if (!GetCiceroneClient()->IsInstallLinuxPackageProgressSignalConnected()) {
// Technically we could still start the install, but we wouldn't be able to
// detect when the install completes, successfully or otherwise.
LOG(ERROR)
<< "Attempted to install package when progress signal not connected.";
std::move(callback).Run(CrostiniResult::INSTALL_LINUX_PACKAGE_FAILED);
return;
}

vm_tools::cicerone::InstallLinuxPackageFromAptRequest request;
request.set_owner_id(owner_id_);
request.set_vm_name(vm_name);
request.set_container_name(container_name);
request.set_package_id(package_id);

GetCiceroneClient()->InstallLinuxPackageFromApt(
std::move(request),
base::BindOnce(&CrostiniManager::OnInstallLinuxPackage,
weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
}

void CrostiniManager::UninstallPackageOwningFile(
std::string vm_name,
std::string container_name,
Expand Down Expand Up @@ -2096,6 +2140,7 @@ void CrostiniManager::OnGetLinuxPackageInfo(
}

result.success = true;
result.package_id = response.package_id();
result.name = split[0];
result.version = split[1];
result.description = response.description();
Expand Down
23 changes: 22 additions & 1 deletion chrome/browser/chromeos/crostini/crostini_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ struct Icon {

struct LinuxPackageInfo {
LinuxPackageInfo();
LinuxPackageInfo(const LinuxPackageInfo&);
~LinuxPackageInfo();

bool success;
Expand All @@ -117,6 +118,8 @@ struct LinuxPackageInfo {
std::string failure_reason;

// The remaining fields are only set when success is true.
// package_id is given as "name;version;arch;data".
std::string package_id;
std::string name;
std::string version;
std::string summary;
Expand Down Expand Up @@ -364,6 +367,13 @@ class CrostiniManager : public KeyedService,
std::string package_path,
GetLinuxPackageInfoCallback callback);

// Asynchronously retrieve information about a Linux Package in the APT
// repository. This uses a package_name to identify a package.
void GetLinuxPackageInfoFromApt(const std::string& vm_name,
const std::string& container_name,
const std::string& package_name,
GetLinuxPackageInfoCallback callback);

// Begin installation of a Linux Package inside the container. If the
// installation is successfully started, further updates will be sent to
// added LinuxPackageOperationProgressObservers.
Expand All @@ -372,6 +382,16 @@ class CrostiniManager : public KeyedService,
std::string package_path,
InstallLinuxPackageCallback callback);

// Begin installation of a Linux Package inside the container. If the
// installation is successfully started, further updates will be sent to
// added LinuxPackageOperationProgressObservers. Uses a package_id, given
// by "package_name;version;arch;data", to identify the package to install
// from the APT repository.
void InstallLinuxPackageFromApt(const std::string& vm_name,
const std::string& container_name,
const std::string& package_id,
InstallLinuxPackageCallback callback);

// Begin uninstallation of a Linux Package inside the container. The package
// is identified by its associated .desktop file's ID; we don't use package_id
// to avoid problems with stale package_ids (such as after upgrades). If the
Expand Down Expand Up @@ -609,7 +629,8 @@ class CrostiniManager : public KeyedService,
GetContainerAppIconsCallback callback,
base::Optional<vm_tools::cicerone::ContainerAppIconResponse> reply);

// Callback for CrostiniManager::GetLinuxPackageInfo.
// Callback for CrostiniManager::GetLinuxPackageInfo and
// CrostiniManager::GetLinuxPackageInfoFromApt.
void OnGetLinuxPackageInfo(
GetLinuxPackageInfoCallback callback,
base::Optional<vm_tools::cicerone::LinuxPackageInfoResponse> reply);
Expand Down
159 changes: 159 additions & 0 deletions chrome/browser/chromeos/crostini/crostini_manager_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,15 @@
#include "content/public/test/test_browser_thread_bundle.h"
#include "device/usb/public/cpp/fake_usb_device_manager.h"
#include "storage/browser/fileapi/external_mount_points.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"

namespace crostini {

namespace {
const char kVmName[] = "vm_name";
const char kContainerName[] = "container_name";
const char kPackageID[] = "package;1;;";
constexpr int64_t kDiskSizeBytes = 4ll * 1024 * 1024 * 1024; // 4 GiB
} // namespace

Expand Down Expand Up @@ -162,6 +164,27 @@ class CrostiniManagerTest : public testing::Test {
std::move(closure).Run();
}

void SearchAppCallback(base::OnceClosure closure,
const std::vector<std::string>& expected_result,
const std::vector<std::string>& result) {
EXPECT_THAT(result, testing::ContainerEq(expected_result));
std::move(closure).Run();
}

void GetLinuxPackageInfoFromAptCallback(
base::OnceClosure closure,
const LinuxPackageInfo& expected_result,
const LinuxPackageInfo& result) {
EXPECT_EQ(result.success, expected_result.success);
EXPECT_EQ(result.failure_reason, expected_result.failure_reason);
EXPECT_EQ(result.package_id, expected_result.package_id);
EXPECT_EQ(result.name, expected_result.name);
EXPECT_EQ(result.version, expected_result.version);
EXPECT_EQ(result.summary, expected_result.summary);
EXPECT_EQ(result.description, expected_result.description);
std::move(closure).Run();
}

CrostiniManagerTest()
: test_browser_thread_bundle_(
content::TestBrowserThreadBundle::REAL_IO_THREAD) {
Expand Down Expand Up @@ -995,4 +1018,140 @@ TEST_F(CrostiniManagerRestartTest, IsContainerRunningFalseIfVmNotStarted) {
EXPECT_FALSE(crostini_manager()->GetContainerInfo(kVmName, kContainerName));
}

TEST_F(CrostiniManagerTest, InstallLinuxPackageFromAptSignalNotConnectedError) {
fake_cicerone_client_->set_install_linux_package_progress_signal_connected(
false);
crostini_manager()->InstallLinuxPackageFromApt(
kVmName, kContainerName, kPackageID,
base::BindOnce(&CrostiniManagerTest::InstallLinuxPackageCallback,
base::Unretained(this), run_loop()->QuitClosure(),
CrostiniResult::INSTALL_LINUX_PACKAGE_FAILED));
run_loop()->Run();
}

TEST_F(CrostiniManagerTest, InstallLinuxPackageFromAptSignalSuccess) {
vm_tools::cicerone::InstallLinuxPackageResponse response;
response.set_status(vm_tools::cicerone::InstallLinuxPackageResponse::STARTED);
fake_cicerone_client_->set_install_linux_package_response(response);
crostini_manager()->InstallLinuxPackageFromApt(
kVmName, kContainerName, kPackageID,
base::BindOnce(&CrostiniManagerTest::InstallLinuxPackageCallback,
base::Unretained(this), run_loop()->QuitClosure(),
CrostiniResult::SUCCESS));
run_loop()->Run();
}

TEST_F(CrostiniManagerTest, InstallLinuxPackageFromAptSignalFailure) {
vm_tools::cicerone::InstallLinuxPackageResponse response;
response.set_status(vm_tools::cicerone::InstallLinuxPackageResponse::FAILED);
response.set_failure_reason(
"Unit tests can't install Linux package from apt!");
fake_cicerone_client_->set_install_linux_package_response(response);
crostini_manager()->InstallLinuxPackageFromApt(
kVmName, kContainerName, kPackageID,
base::BindOnce(&CrostiniManagerTest::InstallLinuxPackageCallback,
base::Unretained(this), run_loop()->QuitClosure(),
CrostiniResult::INSTALL_LINUX_PACKAGE_FAILED));
run_loop()->Run();
}

TEST_F(CrostiniManagerTest, InstallLinuxPackageFromAptSignalOperationBlocked) {
vm_tools::cicerone::InstallLinuxPackageResponse response;
response.set_status(
vm_tools::cicerone::InstallLinuxPackageResponse::INSTALL_ALREADY_ACTIVE);
fake_cicerone_client_->set_install_linux_package_response(response);
crostini_manager()->InstallLinuxPackageFromApt(
kVmName, kContainerName, kPackageID,
base::BindOnce(&CrostiniManagerTest::InstallLinuxPackageCallback,
base::Unretained(this), run_loop()->QuitClosure(),
CrostiniResult::BLOCKING_OPERATION_ALREADY_ACTIVE));
run_loop()->Run();
}

TEST_F(CrostiniManagerTest, SearchAppSuccess) {
vm_tools::cicerone::AppSearchResponse response;
vm_tools::cicerone::AppSearchResponse::AppSearchResult* app =
response.add_packages();
app->set_package_name("fake app");
app = response.add_packages();
app->set_package_name("fake app1");
app = response.add_packages();
app->set_package_name("fake app2");
fake_cicerone_client_->set_search_app_response(response);
std::vector<std::string> expected = {"fake app", "fake app1", "fake app2"};
crostini_manager()->SearchApp(
kVmName, kContainerName, "fake ap",
base::BindOnce(&CrostiniManagerTest::SearchAppCallback,
base::Unretained(this), run_loop()->QuitClosure(),
expected));
run_loop()->Run();
}

TEST_F(CrostiniManagerTest, SearchAppNoResults) {
vm_tools::cicerone::AppSearchResponse response;
fake_cicerone_client_->set_search_app_response(response);
std::vector<std::string> expected = {};
crostini_manager()->SearchApp(
kVmName, kContainerName, "fake ap",
base::BindOnce(&CrostiniManagerTest::SearchAppCallback,
base::Unretained(this), run_loop()->QuitClosure(),
expected));
run_loop()->Run();
}

TEST_F(CrostiniManagerTest, GetLinuxPackageInfoFromAptFailedToGetInfo) {
const char kFailMessage[] = "Failed to get package info.";
vm_tools::cicerone::LinuxPackageInfoResponse response;
response.set_success(false);
response.set_failure_reason(kFailMessage);
fake_cicerone_client_->set_linux_package_info_response(response);
LinuxPackageInfo expected;
expected.success = false;
expected.failure_reason = kFailMessage;
crostini_manager()->GetLinuxPackageInfoFromApt(
kVmName, kContainerName, "fake app",
base::BindOnce(&CrostiniManagerTest::GetLinuxPackageInfoFromAptCallback,
base::Unretained(this), run_loop()->QuitClosure(),
expected));
run_loop()->Run();
}

TEST_F(CrostiniManagerTest, GetLinuxPackageInfoFromAptInvalidID) {
vm_tools::cicerone::LinuxPackageInfoResponse response;
response.set_success(true);
response.set_package_id("Bad;;id;");
fake_cicerone_client_->set_linux_package_info_response(response);
LinuxPackageInfo expected;
expected.success = false;
expected.failure_reason = "Linux package info contained invalid package id.";
crostini_manager()->GetLinuxPackageInfoFromApt(
kVmName, kContainerName, "fake app",
base::BindOnce(&CrostiniManagerTest::GetLinuxPackageInfoFromAptCallback,
base::Unretained(this), run_loop()->QuitClosure(),
expected));
run_loop()->Run();
}

TEST_F(CrostiniManagerTest, GetLinuxPackageInfoFromAptSuccess) {
vm_tools::cicerone::LinuxPackageInfoResponse response;
response.set_success(true);
response.set_package_id("good;1.1;id;");
response.set_summary("A summary");
response.set_description("A description");
fake_cicerone_client_->set_linux_package_info_response(response);
LinuxPackageInfo expected;
expected.success = true;
expected.package_id = "good;1.1;id;";
expected.name = "good";
expected.version = "1.1";
expected.summary = "A summary";
expected.description = "A description";
crostini_manager()->GetLinuxPackageInfoFromApt(
kVmName, kContainerName, "fake app",
base::BindOnce(&CrostiniManagerTest::GetLinuxPackageInfoFromAptCallback,
base::Unretained(this), run_loop()->QuitClosure(),
expected));
run_loop()->Run();
}

} // namespace crostini
12 changes: 12 additions & 0 deletions chrome/browser/chromeos/crostini/crostini_package_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,18 @@ void CrostiniPackageService::InstallLinuxPackage(
std::move(callback)));
}

void CrostiniPackageService::InstallLinuxPackageFromApt(
const std::string& vm_name,
const std::string& container_name,
const std::string& package_id,
CrostiniManager::InstallLinuxPackageCallback callback) {
CrostiniManager::GetForProfile(profile_)->InstallLinuxPackageFromApt(
vm_name, container_name, package_id,
base::BindOnce(&CrostiniPackageService::OnInstallLinuxPackage,
weak_ptr_factory_.GetWeakPtr(), vm_name, container_name,
std::move(callback)));
}

void CrostiniPackageService::OnInstallLinuxPackageProgress(
const std::string& vm_name,
const std::string& container_name,
Expand Down
8 changes: 8 additions & 0 deletions chrome/browser/chromeos/crostini/crostini_package_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,14 @@ class CrostiniPackageService : public KeyedService,
const std::string& package_path,
CrostiniManager::InstallLinuxPackageCallback callback);

// Install a Linux package via a package_id. If successfully started, a
// system notification will be used to display further updates.
void InstallLinuxPackageFromApt(
const std::string& vm_name,
const std::string& container_name,
const std::string& package_id,
CrostiniManager::InstallLinuxPackageCallback callback);

// LinuxPackageOperationProgressObserver:
void OnInstallLinuxPackageProgress(const std::string& vm_name,
const std::string& container_name,
Expand Down
1 change: 1 addition & 0 deletions chrome/browser/chromeos/crostini/crostini_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "base/timer/timer.h"
#include "chrome/browser/chromeos/crostini/crostini_app_launch_observer.h"
#include "chrome/browser/chromeos/crostini/crostini_manager.h"
#include "chrome/browser/chromeos/crostini/crostini_package_service.h"
#include "chrome/browser/chromeos/crostini/crostini_pref_names.h"
#include "chrome/browser/chromeos/crostini/crostini_registry_service.h"
#include "chrome/browser/chromeos/crostini/crostini_registry_service_factory.h"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ void CrostiniRepositorySearchProvider::OnStart(
new_results.reserve(app_names.size());
for (auto& app_name : app_names) {
new_results.emplace_back(
std::make_unique<CrostiniRepositorySearchResult>(app_name));
std::make_unique<CrostiniRepositorySearchResult>(profile_, app_name));
// Todo(https://crbug.com/921429): Improve relevance logic, this will likely
// be implemented in garcon then piped to Chrome
new_results.back()->set_relevance(static_cast<float>(query_.size()) /
Expand Down
Loading

0 comments on commit 4d5a709

Please sign in to comment.