Skip to content

Commit

Permalink
[Shortcut] Add shortcut struct on crosapi mojom.
Browse files Browse the repository at this point in the history
This CL adds a shortcut struct on crosapi mojom and adds the mojom
traits. This CL also deprecates the old unused shortcut struct from
crosapi interface to avoid confusion.

crosapi design: go/cros-shortcut-lacros

BUG=b/304661502

Change-Id: If79224f451f047f255f8daac0e79d7f2598b8ea4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4969978
Commit-Queue: Maggie Cai <mxcai@chromium.org>
Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1216782}
  • Loading branch information
Maggie Cai authored and Chromium LUCI CQ committed Oct 30, 2023
1 parent a8b2c48 commit 2b454a8
Show file tree
Hide file tree
Showing 5 changed files with 153 additions and 18 deletions.
8 changes: 8 additions & 0 deletions chromeos/crosapi/mojom/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,12 @@ mojom("mojom") {
mojom = "crosapi.mojom.ResourceScaleFactor"
cpp = "::ui::ResourceScaleFactor"
},
{
mojom = "crosapi.mojom.AppShortcut"
cpp = "::apps::ShortcutPtr"
move_only = true
nullable_is_same_type = true
},
]
traits_headers = [
"//chromeos/crosapi/mojom/app_service_types_mojom_traits.h",
Expand All @@ -289,6 +295,7 @@ mojom("mojom") {
"//chromeos/crosapi/mojom/web_app_types_mojom_traits.h",
"//components/services/app_service/public/cpp/app_launch_util.h",
"//components/services/app_service/public/cpp/app_types.h",
"//components/services/app_service/public/cpp/shortcut/shortcut.h",
"//components/services/app_service/public/cpp/capability_access.h",
"//components/services/app_service/public/cpp/icon_types.h",
"//components/services/app_service/public/cpp/preferred_app.h",
Expand All @@ -310,6 +317,7 @@ mojom("mojom") {
"//chromeos/components/certificate_provider:certificate_provider",
"//components/policy/core/common:policy_namespace",
"//components/services/app_service/public/cpp:app_types",
"//components/services/app_service/public/cpp/shortcut",
"//components/tab_groups",
"//components/webapps/browser:constants",
"//ui/base:ui_data_pack",
Expand Down
40 changes: 32 additions & 8 deletions chromeos/crosapi/mojom/app_service_types.mojom
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ struct App {
[MinVersion=18, RenamedFrom="shortcuts"]
// DEPRECATED, will be replaced by specific interfaces to update shortcuts.
// This struct is not used in production yet, so it's safe to deprecate.
array<Shortcut>? deprecated_shortcuts@27;
array<REMOVED_01>? deprecated_shortcuts@27;

[MinVersion=19]
OptionalBool is_platform_app@28;
Expand Down Expand Up @@ -507,17 +507,41 @@ struct PreferredApp {
string app_id;
};

// Represents an app shortcut.
// Information about an app shortcut.

// This struct is used to transport App Shortcut data between lacros-chrome and
// ash-chrome. It is intended to be the minimal subset of apps::Shortcut
// required for this purpose.
//
// See components/services/app_service/public/cpp/shortcut/shortcut.h for
// details for the structs in this file.
[Stable]
struct Shortcut {
// Represents a particular shortcut in an app. Needs to be unique within an
// app as calls will be made using both app_id and shortcut_id.
string shortcut_id@0;
struct AppShortcut {
// 'host_app_id' and 'local_id' should not be changeable after creation.
// The host app of the shortcut.
string host_app_id@0;

// The locally unique identifier for the shortcut within an app. This id would
// be used to launch the shortcut or load shortcut icon from the app.
string local_id@1;

// Name of the shortcut.
string? name@2;

// Represents what icon should be loaded for this shortcut, icon key will
// change if the icon has been updated from the publisher.
IconKey? icon_key@3;
};

// DEPRECATED
[Stable, RenamedFrom="crosapi.mojom.Shortcut"]
struct REMOVED_01 {
// DEPRECATED
string shortcut_id@0;

// DEPRECATED
string name@1;

// "Position" of a shortcut, which is a non-negative, sequential
// value. If position is 0, no position was specified.
// DEPRECATED
uint8 position@2;
};
60 changes: 51 additions & 9 deletions chromeos/crosapi/mojom/app_service_types_mojom_traits.cc
Original file line number Diff line number Diff line change
Expand Up @@ -33,21 +33,21 @@ absl::optional<bool> ConvertMojomOptionalBoolToOptionalBool(
}
}

apps::IconKeyPtr ConvertOptionalIconKeyToIconKeyPtr(
const absl::optional<apps::IconKey>& icon_key) {
if (!icon_key.has_value()) {
return nullptr;
}
return icon_key->Clone();
}

} // namespace

namespace mojo {

apps::IconKeyPtr StructTraits<crosapi::mojom::AppDataView,
apps::AppPtr>::icon_key(const apps::AppPtr& r) {
if (!r->icon_key.has_value()) {
return nullptr;
}

auto icon_key = std::make_unique<apps::IconKey>(
r->icon_key.value().timeline, r->icon_key.value().resource_id,
r->icon_key.value().icon_effects);
icon_key->raw_icon_updated = r->icon_key.value().raw_icon_updated;
return icon_key;
return ConvertOptionalIconKeyToIconKeyPtr(r->icon_key);
}

// static
Expand Down Expand Up @@ -1203,4 +1203,46 @@ bool StructTraits<crosapi::mojom::PreferredAppChangesDataView,
return true;
}

apps::IconKeyPtr
StructTraits<crosapi::mojom::AppShortcutDataView, apps::ShortcutPtr>::icon_key(
const apps::ShortcutPtr& r) {
return ConvertOptionalIconKeyToIconKeyPtr(r->icon_key);
}

bool StructTraits<crosapi::mojom::AppShortcutDataView, apps::ShortcutPtr>::Read(
crosapi::mojom::AppShortcutDataView data,
apps::ShortcutPtr* out) {
std::string host_app_id;
if (!data.ReadHostAppId(&host_app_id)) {
return false;
}

std::string local_id;
if (!data.ReadLocalId(&local_id)) {
return false;
}

absl::optional<std::string> name;
if (!data.ReadName(&name)) {
return false;
}

apps::IconKeyPtr icon_key;
if (!data.ReadIconKey(&icon_key)) {
return false;
}

auto shortcut = std::make_unique<apps::Shortcut>(host_app_id, local_id);
shortcut->name = name;
// Currently all shortcuts are User created, will add this field on crosapi
// when we support developer created shortcuts.
shortcut->shortcut_source = apps::ShortcutSource::kUser;
if (icon_key) {
shortcut->icon_key = std::move(*icon_key);
}

*out = std::move(shortcut);
return true;
}

} // namespace mojo
23 changes: 22 additions & 1 deletion chromeos/crosapi/mojom/app_service_types_mojom_traits.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "components/services/app_service/public/cpp/intent_filter.h"
#include "components/services/app_service/public/cpp/permission.h"
#include "components/services/app_service/public/cpp/preferred_app.h"
#include "components/services/app_service/public/cpp/shortcut/shortcut.h"
#include "third_party/abseil-cpp/absl/types/optional.h"
#include "third_party/abseil-cpp/absl/types/variant.h"

Expand Down Expand Up @@ -112,7 +113,7 @@ struct StructTraits<crosapi::mojom::AppDataView, apps::AppPtr> {
static crosapi::mojom::OptionalBool handles_intents(const apps::AppPtr& r);

// This method is required for Ash-Lacros backwards compatibility.
static std::vector<crosapi::mojom::ShortcutPtr> deprecated_shortcuts(
static std::vector<crosapi::mojom::REMOVED_01Ptr> deprecated_shortcuts(
const apps::AppPtr& r) {
return {};
}
Expand Down Expand Up @@ -390,6 +391,26 @@ struct StructTraits<crosapi::mojom::PreferredAppChangesDataView,
apps::PreferredAppChangesPtr* out);
};

template <>
struct StructTraits<crosapi::mojom::AppShortcutDataView, apps::ShortcutPtr> {
static const std::string& host_app_id(const apps::ShortcutPtr& r) {
return r->host_app_id;
}

static const std::string& local_id(const apps::ShortcutPtr& r) {
return r->local_id;
}

static const absl::optional<std::string>& name(const apps::ShortcutPtr& r) {
return r->name;
}

static apps::IconKeyPtr icon_key(const apps::ShortcutPtr& r);

static bool Read(crosapi::mojom::AppShortcutDataView data,
apps::ShortcutPtr* out);
};

} // namespace mojo

#endif // CHROMEOS_CROSAPI_MOJOM_APP_SERVICE_TYPES_MOJOM_TRAITS_H_
40 changes: 40 additions & 0 deletions chromeos/crosapi/mojom/app_service_types_mojom_traits_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1200,3 +1200,43 @@ TEST(AppServiceTypesMojomTraitsTest, RoundTripCapabilityAccess) {
EXPECT_TRUE(output->microphone.value_or(false));
}
}

// Test that every field in apps::Shortcut in correctly converted.
TEST(AppServiceTypesMojomTraitsTest, ShortcutRoundTrip) {
auto input = std::make_unique<apps::Shortcut>("host_app_id", "local_id");
input->name = "lacros test name";
input->icon_key = apps::IconKey(
/*timeline=*/1, apps::IconKey::kInvalidResourceId, /*icon_effects=*/2);
input->icon_key->raw_icon_updated = true;

apps::ShortcutPtr output;
ASSERT_TRUE(mojo::test::SerializeAndDeserialize<crosapi::mojom::AppShortcut>(
input, output));

EXPECT_EQ(output->host_app_id, "host_app_id");
EXPECT_EQ(output->local_id, "local_id");
EXPECT_EQ(output->shortcut_id,
apps::GenerateShortcutId("host_app_id", "local_id"));
EXPECT_EQ(output->name, "lacros test name");
EXPECT_EQ(output->shortcut_source, apps::ShortcutSource::kUser);

EXPECT_EQ(output->icon_key->timeline, 1U);
EXPECT_EQ(output->icon_key->icon_effects, 2U);
EXPECT_TRUE(output->icon_key->raw_icon_updated);
}

// Test that serialization and deserialization works with optional fields that
// doesn't fill up.
TEST(AppServiceTypesMojomTraitsTest, ShortcutRoundTripNoOptional) {
auto input = std::make_unique<apps::Shortcut>("host_app_id", "local_id");

apps::ShortcutPtr output;
ASSERT_TRUE(mojo::test::SerializeAndDeserialize<crosapi::mojom::AppShortcut>(
input, output));

EXPECT_EQ(output->host_app_id, "host_app_id");
EXPECT_EQ(output->local_id, "local_id");
EXPECT_EQ(output->shortcut_id,
apps::GenerateShortcutId("host_app_id", "local_id"));
EXPECT_EQ(output->shortcut_source, apps::ShortcutSource::kUser);
}

0 comments on commit 2b454a8

Please sign in to comment.