Skip to content

Commit

Permalink
[APS] Update protofile.
Browse files Browse the repository at this point in the history 
This CL updates the protofile. The significant changes in this version are:
- Removal of platform
- Removal of Web Manifest ID
- Removal of Android extras' member variables

Bug: b/263438534
Change-Id: I01a083a510c4b7e40695638ad03654ffd47fd55a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4113058
Reviewed-by: Melissa Zhang <melzhang@chromium.org>
Commit-Queue: Jeevan Shikaram <jshikaram@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1088080}
  • Loading branch information
@jeevan-shikaram
jeevan-shikaram authored and Chromium LUCI CQ committed Jan 3, 2023
1 parent b0a28f3 commit 48b0a68
Show file tree
Hide file tree
Showing 7 changed files with 63 additions and 117 deletions.
28 changes: 13 additions & 15 deletions chrome/browser/apps/app_preload_service/app_preload_service_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,10 @@ const base::Value::Dict& GetStateManager(Profile* profile) {

void FillWebExtras(
apps::proto::AppProvisioningListAppsResponse_WebExtras* extras,
const std::string& manifest_id,
const std::string& manifest_url) {
extras->set_manifest_id(manifest_id);
const std::string& manifest_url,
const std::string& original_manifest_url) {
extras->set_manifest_url(manifest_url);
extras->set_original_manifest_url(original_manifest_url);
}

} // namespace
Expand Down Expand Up @@ -196,11 +196,11 @@ TEST_F(AppPreloadServiceTest, DISABLED_WebAppInstall) {
proto::AppProvisioningListAppsResponse response;
auto* app = response.add_apps_to_install();
app->set_name("Peanut Types");
app->set_platform(proto::AppProvisioningListAppsResponse::PLATFORM_WEB);
app->set_install_reason(
proto::AppProvisioningListAppsResponse::INSTALL_REASON_OEM);
FillWebExtras(app->mutable_web_extras(), "https://peanuttypes.com/app",
"https://meltingpot.googleusercontent.com/manifest.json");
FillWebExtras(app->mutable_web_extras(),
"https://meltingpot.googleusercontent.com/manifest.json",
"https://peanuttypes.com/app");

url_loader_factory_.AddResponse(
AppPreloadServerConnector::GetServerUrl().spec(),
Expand Down Expand Up @@ -228,11 +228,11 @@ TEST_F(AppPreloadServiceTest, IgnoreDefaultAppInstall) {
proto::AppProvisioningListAppsResponse response;
auto* app = response.add_apps_to_install();
app->set_name("Peanut Types");
app->set_platform(proto::AppProvisioningListAppsResponse::PLATFORM_WEB);
app->set_install_reason(
proto::AppProvisioningListAppsResponse::INSTALL_REASON_DEFAULT);
FillWebExtras(app->mutable_web_extras(), "https://peanuttypes.com/app",
"https://meltingpot.googleusercontent.com/manifest.json");
FillWebExtras(app->mutable_web_extras(),
"https://meltingpot.googleusercontent.com/manifest.json",
"https://peanuttypes.com/app");

url_loader_factory_.AddResponse(
AppPreloadServerConnector::GetServerUrl().spec(),
Expand All @@ -258,11 +258,8 @@ TEST_F(AppPreloadServiceTest, IgnoreAndroidAppInstall) {
proto::AppProvisioningListAppsResponse response;
auto* app = response.add_apps_to_install();
app->set_name("Peanut Types");
app->set_platform(proto::AppProvisioningListAppsResponse::PLATFORM_ANDROID);
app->set_install_reason(
proto::AppProvisioningListAppsResponse::INSTALL_REASON_OEM);
app->mutable_android_extras()->set_package_name(kPackageName);
app->mutable_android_extras()->set_activity_name(kActivityName);

url_loader_factory_.AddResponse(
AppPreloadServerConnector::GetServerUrl().spec(),
Expand All @@ -284,9 +281,11 @@ TEST_F(AppPreloadServiceTest, IgnoreAndroidAppInstall) {

// TODO(b/261632289): temporarily disabled while refactoring is in progress.
TEST_F(AppPreloadServiceTest, DISABLED_InstallOverUserApp) {
constexpr char kManifestId[] = "https://www.example.com/";
constexpr char kManifestId[] = "https://www.peanuttypes.app/";
constexpr char kManifestUrl[] =
"https://meltingpot.googleusercontent.com/manifest.json";
constexpr char kOriginalManifestUrl[] =
"https://peanuttypes.app/manifest.json";
constexpr char kUserAppName[] = "User Installed App";

auto app_id = web_app::test::InstallDummyWebApp(GetProfile(), kUserAppName,
Expand All @@ -296,10 +295,9 @@ TEST_F(AppPreloadServiceTest, DISABLED_InstallOverUserApp) {
auto* app = response.add_apps_to_install();

app->set_name("OEM Installed app");
app->set_platform(proto::AppProvisioningListAppsResponse::PLATFORM_WEB);
app->set_install_reason(
proto::AppProvisioningListAppsResponse::INSTALL_REASON_OEM);
FillWebExtras(app->mutable_web_extras(), kManifestId, kManifestUrl);
FillWebExtras(app->mutable_web_extras(), kManifestUrl, kOriginalManifestUrl);

url_loader_factory_.AddResponse(
AppPreloadServerConnector::GetServerUrl().spec(),
Expand Down
19 changes: 5 additions & 14 deletions chrome/browser/apps/app_preload_service/preload_app_definition.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,28 +12,20 @@ std::string PreloadAppDefinition::GetName() const {
return app_proto_.name();
}

// TODO(b/263437253): fix up once supporting libraries are in place.
AppType PreloadAppDefinition::GetPlatform() const {
switch (app_proto_.platform()) {
case proto::AppProvisioningListAppsResponse::PLATFORM_UNKNOWN:
return AppType::kUnknown;
case proto::AppProvisioningListAppsResponse::PLATFORM_WEB:
return AppType::kWeb;
case proto::AppProvisioningListAppsResponse::PLATFORM_ANDROID:
return AppType::kArc;
if (app_proto_.has_web_extras()) {
return AppType::kWeb;
}

return AppType::kUnknown;
}

bool PreloadAppDefinition::IsOemApp() const {
return app_proto_.install_reason() ==
proto::AppProvisioningListAppsResponse::INSTALL_REASON_OEM;
}

std::string PreloadAppDefinition::GetWebAppManifestId() const {
DCHECK_EQ(GetPlatform(), AppType::kWeb);

return app_proto_.web_extras().manifest_id();
}

GURL PreloadAppDefinition::GetWebAppManifestUrl() const {
DCHECK_EQ(GetPlatform(), AppType::kWeb);

Expand All @@ -54,7 +46,6 @@ std::ostream& operator<<(std::ostream& os, const PreloadAppDefinition& app) {

if (app.GetPlatform() == AppType::kWeb) {
os << "- Web Extras:" << std::endl;
os << " - Manifest ID: " << app.GetWebAppManifestId() << std::endl;
os << " - Manifest URL: " << app.GetWebAppManifestUrl() << std::endl;
os << " - Original Manifest URL: " << app.GetWebAppOriginalManifestUrl()
<< std::endl;
Expand Down
7 changes: 0 additions & 7 deletions chrome/browser/apps/app_preload_service/preload_app_definition.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,6 @@ class PreloadAppDefinition {
AppType GetPlatform() const;
bool IsOemApp() const;

// Returns the Web App manifest ID for the app, which is the canonical
// identifier for this app, as specified by
// https://www.w3.org/TR/appmanifest/#id-member. Does not attempt to validate
// the value returned. Must only be called if `GetPlatform()` returns
// `AppType::kWeb`.
std::string GetWebAppManifestId() const;

// Returns the Web App manifest URL for the app, which hosts the manifest of
// the app in a JSON format. The URL could point to a local file, or a web
// address. Does not attempt to validate the GURL. Must only be called if
Expand Down
28 changes: 5 additions & 23 deletions chrome/browser/apps/app_preload_service/preload_app_definition_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ namespace {
proto::AppProvisioningListAppsResponse_App CreateTestWebApp() {
proto::AppProvisioningListAppsResponse_App app;
app.set_name("Test app");
app.set_platform(proto::AppProvisioningListAppsResponse::PLATFORM_WEB);
app.mutable_web_extras()->set_manifest_url("https://example.com");
return app;
}
} // namespace
Expand Down Expand Up @@ -57,8 +57,8 @@ TEST_F(PreloadAppDefinitionTest, GetPlatformWhenNotSet) {
TEST_F(PreloadAppDefinitionTest, GetPlatform) {
proto::AppProvisioningListAppsResponse_App app;

app.set_platform(proto::AppProvisioningListAppsResponse_Platform::
AppProvisioningListAppsResponse_Platform_PLATFORM_WEB);
app.mutable_web_extras()->set_manifest_url("https://example.com");

auto app_def = PreloadAppDefinition(app);
ASSERT_EQ(app_def.GetPlatform(), AppType::kWeb);
}
Expand Down Expand Up @@ -90,25 +90,6 @@ TEST_F(PreloadAppDefinitionTest, IsNotOemApp) {
ASSERT_FALSE(app_def.IsOemApp());
}

TEST_F(PreloadAppDefinitionTest, GetWebAppManifestId) {
proto::AppProvisioningListAppsResponse_App app = CreateTestWebApp();
app.mutable_web_extras()->set_manifest_id(
"https://www.example.com/manifest_id/");

PreloadAppDefinition app_def(app);

ASSERT_EQ(app_def.GetWebAppManifestId(),
"https://www.example.com/manifest_id/");
}

TEST_F(PreloadAppDefinitionTest, GetWebAppManifestIdNotSpecified) {
proto::AppProvisioningListAppsResponse_App app = CreateTestWebApp();

PreloadAppDefinition app_def(app);

ASSERT_TRUE(app_def.GetWebAppManifestId().empty());
}

TEST_F(PreloadAppDefinitionTest, GetWebAppManifestUrlWebsite) {
proto::AppProvisioningListAppsResponse_App app = CreateTestWebApp();
app.mutable_web_extras()->set_manifest_url(
Expand Down Expand Up @@ -146,8 +127,9 @@ TEST_F(PreloadAppDefinitionTest, GetWebAppManifestUrlInvalidUrl) {
ASSERT_FALSE(app_def.GetWebAppManifestUrl().is_valid());
}

TEST_F(PreloadAppDefinitionTest, GetWebAppManifestUrlNotSpecified) {
TEST_F(PreloadAppDefinitionTest, GetWebAppManifestUrlEmpty) {
proto::AppProvisioningListAppsResponse_App app = CreateTestWebApp();
app.mutable_web_extras()->set_manifest_url("");

PreloadAppDefinition app_def(app);

Expand Down
68 changes: 35 additions & 33 deletions chrome/browser/apps/app_preload_service/proto/app_provisioning.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,25 @@ message AppProvisioningListAppsRequest {
USERTYPE_GUEST = 4;
}

enum Channel {
// Default for deserialization when an unexpected value is encountered.
// Indicates to the client that the server has a new platform and needs
// the proto file updated.
CHANNEL_UNKNOWN = 0;

// Canary channel.
CHANNEL_CANARY = 1;

// Developer channel.
CHANNEL_DEV = 2;

// Beta channel.
CHANNEL_BETA = 3;

// Stable channel.
CHANNEL_STABLE = 4;
}

// The board identifier for the device sending the request.
optional string board = 1;

Expand Down Expand Up @@ -64,6 +83,9 @@ message AppProvisioningListAppsRequest {

// Chrome OS platform version.
optional string platform = 2;

// The channel the device is using.
optional Channel channel = 3;
}

// The type of user account making this request.
Expand All @@ -74,19 +96,6 @@ message AppProvisioningListAppsResponse {
// A list of zero or more apps for APS to install.
repeated App apps_to_install = 1;

enum Platform {
// Default for deserialization when an unexpected value is encountered.
// Indicates to the client that the server has a new platform and needs
// the proto file updated.
PLATFORM_UNKNOWN = 0;

// A Web App.
PLATFORM_WEB = 1;

// An Android app managed by Play.
PLATFORM_ANDROID = 2;
}

enum InstallReason {
// Default for deserialization when an unexpected value is encountered.
// Indicates to the client that the server has a new reason and needs
Expand Down Expand Up @@ -116,49 +125,42 @@ message AppProvisioningListAppsResponse {
optional bool is_masking_allowed = 4;
}

// Every platform has its own [Platform]Extras message to store platform
// specific metadata.
// For Android-only metadata.
message AndroidExtras {
// |package_name| and |activity_name| uniquely identify each Android app.
optional string package_name = 1;
optional string activity_name = 2;
}
message AndroidExtras {}

// For Web-only metadata.
message WebExtras {
// |manifest_id| uniquely identifies a web app.
optional string manifest_id = 1;

// A URL to the web app's manifest in json format. This will always be from
// the host meltingpot.googleusercontent.com.
optional string manifest_url = 2;
optional string manifest_url = 1;

// The URL for the original source used to retrieve the contents of the
// manifest above. This is needed to resolve the file on the client-side.
optional string original_manifest_url = 3;
optional string original_manifest_url = 2;
}

message App {
// The primary key for the installer. This will always be of the format
// "platform:primary_key". For now we support "android:package_name" and
// "web:http://manifest/id".
optional string package_id = 1;

// The identifier for the App Group that Fondue used to derive this app
// instance in the response.
// Note: this may not be unique in the apps_to_install collection.
optional string app_group_uuid = 1;
optional string app_group_uuid = 2;

// The App's UTF-8 encoded name in the requested language (or next best).
optional string name = 2;
optional string name = 3;

// One or more Icons for this App for the requested language (or next best).
repeated Icon icons = 3;

// Specifies the platform this app uses to install. This will match the
// contents of the extras.
optional Platform platform = 4;
repeated Icon icons = 4;

// The reason why this app is in the list.
optional InstallReason install_reason = 5;

// Platform-specific information for installing this app.
// Every platform has its own [Platform]Extras message to store platform
// specific metadata.
oneof extras {
AndroidExtras android_extras = 6;
WebExtras web_extras = 7;
Expand Down
4 changes: 3 additions & 1 deletion chrome/browser/apps/app_preload_service/web_app_preload_installer.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,10 +126,12 @@ void WebAppPreloadInstaller::InstallApp(
weak_ptr_factory_.GetWeakPtr(), app, std::move(callback)));
}

// TODO(b/263437253): fix up once supporting libraries are in place.
std::string WebAppPreloadInstaller::GetAppId(
const PreloadAppDefinition& app) const {
// The app's "Web app manifest ID" is the equivalent of the unhashed app ID.
return web_app::GenerateAppIdFromUnhashed(app.GetWebAppManifestId());
// return web_app::GenerateAppIdFromUnhashed(app.GetWebAppManifestId());
return "";
}

void WebAppPreloadInstaller::InstallAppImpl(
Expand Down
Loading

0 comments on commit 48b0a68

Please sign in to comment.