Saving and reading home tab icons to proto
Icons for the home tab within the tab strip specified in the manifest are saved (without filtering) through proto. Icons are saved as ImageResource, keeping the same shape it is set as in the manifest.
Change-Id: I2cce28fc171e2fc238cf2bc8fdc4205a2b6a0fd2
Bug: 1381377
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4136334
Reviewed-by: Peter Beverloo <[email protected]>
Reviewed-by: Eric Willigers <[email protected]>
Commit-Queue: Eric Willigers <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1094699}
diff --git a/chrome/browser/web_applications/BUILD.gn b/chrome/browser/web_applications/BUILD.gn
index d7dffea..74b4b758 100644
--- a/chrome/browser/web_applications/BUILD.gn
+++ b/chrome/browser/web_applications/BUILD.gn
@@ -387,6 +387,7 @@
"//components/webapps/common:mojo_bindings",
"//components/webapps/services/web_app_origin_association:lib",
"//components/webapps/services/web_app_origin_association:service",
+ "//content/browser/background_fetch:background_fetch_proto",
"//content/public/browser",
"//mojo/core/embedder",
"//mojo/core/embedder:features",
diff --git a/chrome/browser/web_applications/DEPS b/chrome/browser/web_applications/DEPS
index 89357b54..242c13f 100644
--- a/chrome/browser/web_applications/DEPS
+++ b/chrome/browser/web_applications/DEPS
@@ -4,6 +4,7 @@
"+components/services/storage/indexed_db/locks",
"+components/web_package",
"+components/webapps/services/web_app_origin_association",
+ "+content/browser/background_fetch",
"+mojo/core/embedder",
"+mojo/public/cpp/bindings",
"+services/network/public/cpp",
diff --git a/chrome/browser/web_applications/proto/BUILD.gn b/chrome/browser/web_applications/proto/BUILD.gn
index 3fbfadd..88c14676 100644
--- a/chrome/browser/web_applications/proto/BUILD.gn
+++ b/chrome/browser/web_applications/proto/BUILD.gn
@@ -16,6 +16,7 @@
]
link_deps = [
"//chrome/browser/ash/system_web_apps/types:proto",
+ "//content/browser/background_fetch:background_fetch_proto",
"//components/sync/protocol",
]
}
diff --git a/chrome/browser/web_applications/proto/web_app_tab_strip.proto b/chrome/browser/web_applications/proto/web_app_tab_strip.proto
index 0d5c5f4e..55d20bf 100644
--- a/chrome/browser/web_applications/proto/web_app_tab_strip.proto
+++ b/chrome/browser/web_applications/proto/web_app_tab_strip.proto
@@ -4,10 +4,16 @@
syntax = "proto2";
+import "content/browser/background_fetch/background_fetch.proto";
+
option optimize_for = LITE_RUNTIME;
package web_app.proto;
+message HomeTabParams {
+ repeated content.proto.ImageResource icons = 1;
+}
+
message NewTabButtonParams {
optional string url = 1;
}
@@ -28,6 +34,6 @@
oneof home_tab {
Visibility home_tab_visibility = 3;
- // TODO(crbug.com/897314): Add HomeTabParams containing icons.
+ HomeTabParams home_tab_params = 4;
}
}
diff --git a/chrome/browser/web_applications/test/web_app_test_utils.cc b/chrome/browser/web_applications/test/web_app_test_utils.cc
index 96f45ee..8eb901f 100644
--- a/chrome/browser/web_applications/test/web_app_test_utils.cc
+++ b/chrome/browser/web_applications/test/web_app_test_utils.cc
@@ -310,6 +310,56 @@
return results;
}
+std::vector<blink::Manifest::ImageResource> CreateRandomHomeTabIcons(
+ RandomHelper& random) {
+ std::vector<blink::Manifest::ImageResource> icons;
+
+ for (int i = random.next_uint(4) + 1; i >= 0; --i) {
+ blink::Manifest::ImageResource icon;
+
+ int mime_type = random.next_uint(3);
+ switch (mime_type) {
+ case 0:
+ icon.src = GURL("https://example.com/image" + base::NumberToString(i) +
+ ".png");
+ icon.type = base::UTF8ToUTF16(std::string("image/png"));
+ break;
+ case 1:
+ icon.src = GURL("https://example.com/image" + base::NumberToString(i) +
+ ".svg");
+ icon.type = base::UTF8ToUTF16(std::string("image/svg+xml"));
+ break;
+ case 2:
+ icon.src = GURL("https://example.com/image" + base::NumberToString(i) +
+ ".webp");
+ icon.type = base::UTF8ToUTF16(std::string("image/webp"));
+ break;
+ }
+
+ // Icon sizes can be non square
+ std::vector<gfx::Size> sizes;
+ for (int j = random.next_uint(3) + 1; j > 0; --j) {
+ sizes.emplace_back(j * random.next_uint(200), j * random.next_uint(200));
+ }
+ icon.sizes = std::move(sizes);
+
+ std::vector<blink::mojom::ManifestImageResource_Purpose> purposes = {
+ blink::mojom::ManifestImageResource_Purpose::ANY,
+ blink::mojom::ManifestImageResource_Purpose::MASKABLE,
+ blink::mojom::ManifestImageResource_Purpose::MONOCHROME};
+
+ std::vector<blink::mojom::ManifestImageResource_Purpose> purpose;
+
+ for (int j = random.next_uint(purposes.size()); j >= 0; --j) {
+ unsigned index = random.next_uint(purposes.size());
+ purpose.push_back(purposes[index]);
+ purposes.erase(purposes.begin() + index);
+ }
+ icon.purpose = std::move(purpose);
+ icons.push_back(std::move(icon));
+ }
+ return icons;
+}
} // namespace
std::string GetExternalPrefMigrationTestName(
@@ -627,8 +677,18 @@
if (random.next_bool()) {
blink::Manifest::TabStrip tab_strip;
- tab_strip.home_tab =
- random.next_enum<blink::mojom::TabStripMemberVisibility>();
+
+ if (random.next_bool()) {
+ blink::Manifest::HomeTabParams home_tab_params;
+ if (random.next_bool()) {
+ home_tab_params.icons = CreateRandomHomeTabIcons(random);
+ }
+ tab_strip.home_tab = std::move(home_tab_params);
+ } else {
+ tab_strip.home_tab =
+ random.next_enum<blink::mojom::TabStripMemberVisibility>();
+ }
+
if (random.next_bool()) {
blink::Manifest::NewTabButtonParams new_tab_button_params;
if (random.next_bool()) {
diff --git a/chrome/browser/web_applications/web_app.cc b/chrome/browser/web_applications/web_app.cc
index 5505f8a..10da49f 100644
--- a/chrome/browser/web_applications/web_app.cc
+++ b/chrome/browser/web_applications/web_app.cc
@@ -155,6 +155,30 @@
return base::Value(std::move(debug_dict));
}
+base::Value ImageResourceDebugValue(
+ const blink::Manifest::ImageResource& icon) {
+ const char* const kPurposeStrings[] = {"Any", "Monochrome", "Maskable"};
+
+ base::Value root(base::Value::Type::DICT);
+ root.SetStringKey("src", icon.src.spec());
+ root.SetStringKey("type", icon.type);
+
+ base::Value sizes_json(base::Value::Type::LIST);
+ for (const auto& size : icon.sizes) {
+ std::string size_formatted = base::NumberToString(size.width()) + "x" +
+ base::NumberToString(size.height());
+ sizes_json.Append(base::Value(size_formatted));
+ }
+ root.SetKey("sizes", std::move(sizes_json));
+
+ base::Value purpose_json(base::Value::Type::LIST);
+ for (const auto& purpose : icon.purpose) {
+ purpose_json.Append(kPurposeStrings[static_cast<int>(purpose)]);
+ }
+ root.SetKey("purpose", std::move(purpose_json));
+ return root;
+}
+
} // namespace
WebApp::WebApp(const AppId& app_id)
@@ -956,8 +980,18 @@
"home_tab", base::StreamableToString(absl::get<TabStrip::Visibility>(
tab_strip_.value().home_tab)));
} else {
- tab_strip_json.Set("home_tab", base::Value::Dict());
- // TODO(crbug.com/897314): Add debug info for home tab icons.
+ base::Value::Dict home_tab_json;
+ base::Value icons_json(base::Value::Type::LIST);
+ absl::optional<std::vector<blink::Manifest::ImageResource>> icons =
+ absl::get<blink::Manifest::HomeTabParams>(tab_strip_.value().home_tab)
+ .icons;
+
+ for (auto& icon : *icons) {
+ icons_json.Append(ImageResourceDebugValue(icon));
+ }
+
+ home_tab_json.Set("icons", std::move(icons_json));
+ tab_strip_json.Set("home_tab", std::move(home_tab_json));
}
root.Set("tab_strip", std::move(tab_strip_json));
} else {
diff --git a/chrome/browser/web_applications/web_app_database.cc b/chrome/browser/web_applications/web_app_database.cc
index 9d816aa..48b78bb 100644
--- a/chrome/browser/web_applications/web_app_database.cc
+++ b/chrome/browser/web_applications/web_app_database.cc
@@ -738,6 +738,15 @@
if (absl::holds_alternative<TabStrip::Visibility>(tab_strip.home_tab)) {
mutable_tab_strip->set_home_tab_visibility(TabStripVisibilityToProto(
absl::get<TabStrip::Visibility>(tab_strip.home_tab)));
+ } else {
+ auto* mutable_home_tab_params =
+ mutable_tab_strip->mutable_home_tab_params();
+ absl::optional<std::vector<blink::Manifest::ImageResource>> icons =
+ absl::get<blink::Manifest::HomeTabParams>(tab_strip.home_tab).icons;
+ for (const auto& image_resource : *icons) {
+ *(mutable_home_tab_params->add_icons()) =
+ AppImageResourceToProto(image_resource);
+ }
}
if (absl::holds_alternative<TabStrip::Visibility>(
@@ -1375,7 +1384,14 @@
tab_strip.home_tab = ProtoToTabStripVisibility(
local_data.tab_strip().home_tab_visibility());
} else {
- tab_strip.home_tab = blink::Manifest::HomeTabParams();
+ absl::optional<std::vector<blink::Manifest::ImageResource>> icons =
+ ParseAppImageResource(
+ "WebApp", local_data.tab_strip().home_tab_params().icons());
+ blink::Manifest::HomeTabParams home_tab_params;
+ if (!icons->empty()) {
+ home_tab_params.icons = std::move(*icons);
+ }
+ tab_strip.home_tab = std::move(home_tab_params);
}
if (local_data.tab_strip().has_new_tab_button_visibility()) {
diff --git a/chrome/browser/web_applications/web_app_database_unittest.cc b/chrome/browser/web_applications/web_app_database_unittest.cc
index 02a295f..21d406ad 100644
--- a/chrome/browser/web_applications/web_app_database_unittest.cc
+++ b/chrome/browser/web_applications/web_app_database_unittest.cc
@@ -458,6 +458,7 @@
EXPECT_FALSE(app_copy->manifest_id().has_value());
EXPECT_FALSE(app_copy->IsStorageIsolated());
EXPECT_TRUE(app_copy->permissions_policy().empty());
+ EXPECT_FALSE(app_copy->tab_strip());
}
TEST_F(WebAppDatabaseTest, WebAppWithManyIcons) {
diff --git a/chrome/browser/web_applications/web_app_proto_utils.cc b/chrome/browser/web_applications/web_app_proto_utils.cc
index 6bb92a2..24a1feb 100644
--- a/chrome/browser/web_applications/web_app_proto_utils.cc
+++ b/chrome/browser/web_applications/web_app_proto_utils.cc
@@ -3,10 +3,13 @@
// found in the LICENSE file.
#include "chrome/browser/web_applications/web_app_proto_utils.h"
+#include "base/strings/utf_string_conversions.h"
#include "chrome/browser/web_applications/mojom/user_display_mode.mojom.h"
#include "chrome/browser/web_applications/user_display_mode.h"
#include "components/services/app_service/public/cpp/icon_info.h"
+#include "third_party/blink/public/common/manifest/manifest.h"
#include "third_party/blink/public/mojom/manifest/manifest.mojom.h"
+#include "ui/gfx/geometry/size.h"
namespace web_app {
@@ -40,6 +43,19 @@
}
}
+content::proto::ImageResource_Purpose
+ManifestImageResourcePurposeToImageResoucePurposeProto(
+ blink::mojom::ManifestImageResource_Purpose purpose) {
+ switch (purpose) {
+ case blink::mojom::ManifestImageResource_Purpose::ANY:
+ return content::proto::ImageResource_Purpose_ANY;
+ case blink::mojom::ManifestImageResource_Purpose::MONOCHROME:
+ return content::proto::ImageResource_Purpose_MONOCHROME;
+ case blink::mojom::ManifestImageResource_Purpose::MASKABLE:
+ return content::proto::ImageResource_Purpose_MASKABLE;
+ }
+}
+
} // namespace
absl::optional<std::vector<apps::IconInfo>> ParseAppIconInfos(
@@ -80,6 +96,67 @@
return manifest_icons;
}
+absl::optional<std::vector<blink::Manifest::ImageResource>>
+ParseAppImageResource(const char* container_name_for_logging,
+ const RepeatedImageResourceProto& manifest_icons_proto) {
+ std::vector<blink::Manifest::ImageResource> manifest_icons;
+ for (const content::proto::ImageResource& image_resource_proto :
+ manifest_icons_proto) {
+ blink::Manifest::ImageResource image_resource;
+
+ if (!image_resource_proto.has_src()) {
+ DLOG(ERROR) << container_name_for_logging
+ << " ImageResource has missing url";
+ return absl::nullopt;
+ }
+ image_resource.src = GURL(image_resource_proto.src());
+
+ if (!image_resource.src.is_valid()) {
+ DLOG(ERROR) << container_name_for_logging
+ << " ImageResource has invalid url: "
+ << image_resource.src.possibly_invalid_spec();
+ return absl::nullopt;
+ }
+
+ if (image_resource_proto.has_type()) {
+ image_resource.type = base::ASCIIToUTF16(image_resource_proto.type());
+ }
+
+ if (!image_resource_proto.sizes().empty()) {
+ std::vector<gfx::Size> sizes;
+ for (const auto& size_proto : image_resource_proto.sizes()) {
+ sizes.emplace_back(size_proto.width(), size_proto.height());
+ }
+ image_resource.sizes = std::move(sizes);
+ }
+
+ std::vector<blink::mojom::ManifestImageResource_Purpose> purpose;
+ if (!image_resource_proto.purpose().empty()) {
+ for (const auto& purpose_proto : image_resource_proto.purpose()) {
+ switch (purpose_proto) {
+ case content::proto::ImageResource_Purpose_ANY:
+ purpose.push_back(blink::mojom::ManifestImageResource_Purpose::ANY);
+ break;
+ case content::proto::ImageResource_Purpose_MASKABLE:
+ purpose.push_back(
+ blink::mojom::ManifestImageResource_Purpose::MASKABLE);
+ break;
+ case content::proto::ImageResource_Purpose_MONOCHROME:
+ purpose.push_back(
+ blink::mojom::ManifestImageResource_Purpose::MONOCHROME);
+ break;
+ }
+ }
+ } else {
+ purpose.push_back(blink::mojom::ManifestImageResource_Purpose::ANY);
+ }
+ image_resource.purpose = std::move(purpose);
+ manifest_icons.push_back(std::move(image_resource));
+ }
+
+ return manifest_icons;
+}
+
sync_pb::WebAppSpecifics WebAppToSyncProto(const WebApp& app) {
DCHECK(!app.start_url().is_empty());
DCHECK(app.start_url().is_valid());
@@ -120,6 +197,31 @@
return icon_info_proto;
}
+content::proto::ImageResource AppImageResourceToProto(
+ const blink::Manifest::ImageResource& image_resource) {
+ content::proto::ImageResource image_resource_proto;
+ DCHECK(!image_resource.src.is_empty());
+
+ image_resource_proto.set_src(image_resource.src.spec());
+
+ if (!image_resource.type.empty()) {
+ image_resource_proto.set_type(base::UTF16ToASCII(image_resource.type));
+ }
+
+ for (const auto& size : image_resource.sizes) {
+ content::proto::ImageResource::Size size_proto;
+ size_proto.set_width(size.width());
+ size_proto.set_height(size.height());
+ *(image_resource_proto.add_sizes()) = size_proto;
+ }
+
+ for (const auto& purpose : image_resource.purpose) {
+ image_resource_proto.add_purpose(
+ ManifestImageResourcePurposeToImageResoucePurposeProto(purpose));
+ }
+ return image_resource_proto;
+}
+
absl::optional<WebApp::SyncFallbackData> ParseSyncFallbackDataStruct(
const sync_pb::WebAppSpecifics& sync_proto) {
WebApp::SyncFallbackData parsed_sync_fallback_data;
diff --git a/chrome/browser/web_applications/web_app_proto_utils.h b/chrome/browser/web_applications/web_app_proto_utils.h
index 1a480b22..a07227e 100644
--- a/chrome/browser/web_applications/web_app_proto_utils.h
+++ b/chrome/browser/web_applications/web_app_proto_utils.h
@@ -10,6 +10,7 @@
#include "chrome/browser/web_applications/proto/web_app.pb.h"
#include "chrome/browser/web_applications/web_app.h"
#include "components/sync/protocol/web_app_specifics.pb.h"
+#include "content/browser/background_fetch/background_fetch.pb.h"
#include "third_party/abseil-cpp/absl/types/optional.h"
namespace apps {
@@ -23,16 +24,27 @@
using RepeatedIconInfosProto =
const ::google::protobuf::RepeatedPtrField<::sync_pb::WebAppIconInfo>;
+using RepeatedImageResourceProto =
+ const ::google::protobuf::RepeatedPtrField<content::proto::ImageResource>;
+
absl::optional<std::vector<apps::IconInfo>> ParseAppIconInfos(
const char* container_name_for_logging,
const RepeatedIconInfosProto& manifest_icons_proto);
+absl::optional<std::vector<blink::Manifest::ImageResource>>
+ParseAppImageResource(const char* container_name_for_logging,
+ const RepeatedImageResourceProto& manifest_icons_proto);
+
// Use the given |app| to populate a |WebAppSpecifics| sync proto.
sync_pb::WebAppSpecifics WebAppToSyncProto(const WebApp& app);
// Use the given |icon_info| to populate a |WebAppIconInfo| sync proto.
sync_pb::WebAppIconInfo AppIconInfoToSyncProto(const apps::IconInfo& icon_info);
+// Use the given |image_resource| to populate a |ImageResource| proto.
+content::proto::ImageResource AppImageResourceToProto(
+ const blink::Manifest::ImageResource& image_resource);
+
absl::optional<WebApp::SyncFallbackData> ParseSyncFallbackDataStruct(
const sync_pb::WebAppSpecifics& sync_proto);
diff --git a/content/browser/background_fetch/background_fetch.proto b/content/browser/background_fetch/background_fetch.proto
index 7218586e..51583c5 100644
--- a/content/browser/background_fetch/background_fetch.proto
+++ b/content/browser/background_fetch/background_fetch.proto
@@ -45,6 +45,34 @@
optional BackgroundFetchFailureReason failure_reason = 8;
}
+// TODO(crbug.com/1381377): Move ImageResource to generic place.
+// Currently also being used within chrome/browser/web_applications
+// https://w3c.github.io/manifest/#dom-imageresource
+//
+// Next Tag: 5
+message ImageResource {
+ optional string src = 1;
+
+ // Next Tag: 3
+ message Size {
+ optional int32 width = 1;
+ optional int32 height = 2;
+ }
+
+ repeated Size sizes = 2;
+ optional bytes type = 3;
+
+ // https://w3c.github.io/manifest/#purpose-member
+ enum Purpose {
+ ANY = 1;
+ MONOCHROME = 2;
+ MASKABLE = 3;
+ }
+
+ // blink::mojom::ManifestImageResource_Purpose enum.
+ repeated Purpose purpose = 4;
+}
+
// Developer provided options.
// https://wicg.github.io/background-fetch/#background-fetch-manager
//
@@ -54,32 +82,6 @@
// and the most recent value is stored in BackgroundFetchUIOptions.
optional string title = 1;
- // https://w3c.github.io/manifest/#dom-imageresource
- //
- // Next Tag: 5
- message ImageResource {
- optional string src = 1;
-
- // Next Tag: 3
- message Size {
- optional int32 width = 1;
- optional int32 height = 2;
- }
-
- repeated Size sizes = 2;
- optional bytes type = 3;
-
- // https://w3c.github.io/manifest/#purpose-member
- enum Purpose {
- ANY = 1;
- MONOCHROME = 2;
- MASKABLE = 3;
- }
-
- // blink::mojom::ManifestImageResource_Purpose enum.
- repeated Purpose purpose = 4;
- }
-
repeated ImageResource icons = 2;
optional uint64 download_total = 3;
diff --git a/content/browser/background_fetch/storage/create_metadata_task.cc b/content/browser/background_fetch/storage/create_metadata_task.cc
index 37bac1b2..b9c2165 100644
--- a/content/browser/background_fetch/storage/create_metadata_task.cc
+++ b/content/browser/background_fetch/storage/create_metadata_task.cc
@@ -128,6 +128,18 @@
this}; // Keep as last.
};
+proto::ImageResource_Purpose
+ManifestImageResourcePurposeToImageResoucePurposeProto(
+ blink::mojom::ManifestImageResource_Purpose purpose) {
+ switch (purpose) {
+ case blink::mojom::ManifestImageResource_Purpose::ANY:
+ return proto::ImageResource_Purpose_ANY;
+ case blink::mojom::ManifestImageResource_Purpose::MONOCHROME:
+ return proto::ImageResource_Purpose_MONOCHROME;
+ case blink::mojom::ManifestImageResource_Purpose::MASKABLE:
+ return proto::ImageResource_Purpose_MASKABLE;
+ }
+}
} // namespace
CreateMetadataTask::CreateMetadataTask(
@@ -266,20 +278,8 @@
image_resource_proto->set_type(base::UTF16ToASCII(icon.type));
for (const auto& purpose : icon.purpose) {
- switch (purpose) {
- case blink::mojom::ManifestImageResource_Purpose::ANY:
- image_resource_proto->add_purpose(
- proto::BackgroundFetchOptions_ImageResource_Purpose_ANY);
- break;
- case blink::mojom::ManifestImageResource_Purpose::MONOCHROME:
- image_resource_proto->add_purpose(
- proto::BackgroundFetchOptions_ImageResource_Purpose_MONOCHROME);
- break;
- case blink::mojom::ManifestImageResource_Purpose::MASKABLE:
- image_resource_proto->add_purpose(
- proto::BackgroundFetchOptions_ImageResource_Purpose_MASKABLE);
- break;
- }
+ image_resource_proto->add_purpose(
+ ManifestImageResourcePurposeToImageResoucePurposeProto(purpose));
}
}
diff --git a/content/browser/background_fetch/storage/get_initialization_data_task.cc b/content/browser/background_fetch/storage/get_initialization_data_task.cc
index 22444605..c497a8f 100644
--- a/content/browser/background_fetch/storage/get_initialization_data_task.cc
+++ b/content/browser/background_fetch/storage/get_initialization_data_task.cc
@@ -371,11 +371,11 @@
ir.purpose.reserve(icon.purpose_size());
for (auto purpose : icon.purpose()) {
switch (purpose) {
- case proto::BackgroundFetchOptions_ImageResource_Purpose_ANY:
+ case proto::ImageResource_Purpose_ANY:
ir.purpose.push_back(
blink::mojom::ManifestImageResource_Purpose::ANY);
break;
- case proto::BackgroundFetchOptions_ImageResource_Purpose_MONOCHROME:
+ case proto::ImageResource_Purpose_MONOCHROME:
ir.purpose.push_back(
blink::mojom::ManifestImageResource_Purpose::MONOCHROME);
break;
