Commit 484d2441 authored by Miyoung Shin's avatar Miyoung Shin Committed by Commit Bot

Mark members as mandatory in manifest.mojom to match the web app manifest spec.

This is a follow-up CL for https://crrev.com/c/1597496 that updates
manifest.mojom to make the start_url, icons, related_applications,
splash_screen_url, and scope members non-optional in order to
follow the web app manifest spec
(https://www.w3.org/TR/appmanifest/#processing).

Bug: 704441
Change-Id: I92c6d8c220e05c9394d62a06ca4450191f1c552d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1633695
Commit-Queue: Miyoung Shin <myid.shin@igalia.com>
Reviewed-by: default avatarKentaro Hara <haraken@chromium.org>
Reviewed-by: default avatarDominick Ng <dominickn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#664499}
parent ab94a58b
......@@ -20,13 +20,13 @@ struct Manifest {
mojo_base.mojom.String16? short_name;
url.mojom.Url? start_url;
url.mojom.Url start_url;
DisplayMode display;
device.mojom.ScreenOrientationLockType orientation;
array<ManifestImageResource>? icons;
array<ManifestImageResource> icons;
// TODO(constantina): This field is non-standard and part of a Chrome
// experiment. See:
......@@ -40,7 +40,7 @@ struct Manifest {
// As such, this field should not be exposed to the drive-by web.
ManifestFileHandler? file_handler;
array<ManifestRelatedApplication>? related_applications;
array<ManifestRelatedApplication> related_applications;
// A boolean that is used as a hint for the user agent to say that related
// applications should be preferred over the web application. False if missing
......@@ -56,11 +56,11 @@ struct Manifest {
uint32 background_color;
// A URL of the HTML splash screen.
url.mojom.Url? splash_screen_url;
url.mojom.Url splash_screen_url;
mojo_base.mojom.String16? gcm_sender_id;
url.mojom.Url? scope;
url.mojom.Url scope;
};
// Structure representing an icon as per the Manifest specification, see:
......
......@@ -63,16 +63,13 @@ void InstalledAppController::OnGetManifestForRelatedApps(
const KURL& /*url*/,
mojom::blink::ManifestPtr manifest) {
Vector<mojom::blink::RelatedApplicationPtr> mojo_related_apps;
if (manifest->related_applications.has_value()) {
for (const auto& related_application : *manifest->related_applications) {
mojom::blink::RelatedApplicationPtr converted_application(
mojom::blink::RelatedApplication::New());
converted_application->platform = related_application->platform;
converted_application->id = related_application->id;
for (const auto& related_application : manifest->related_applications) {
auto application = mojom::blink::RelatedApplication::New();
application->platform = related_application->platform;
application->id = related_application->id;
if (related_application->url.has_value())
converted_application->url = related_application->url->GetString();
mojo_related_apps.push_back(std::move(converted_application));
}
application->url = related_application->url->GetString();
mojo_related_apps.push_back(std::move(application));
}
if (!provider_) {
......
......@@ -24,18 +24,6 @@
namespace blink {
namespace {
mojom::blink::ManifestPtr CreateEmptyManifest() {
auto manifest = mojom::blink::Manifest::New();
manifest->start_url = KURL();
manifest->splash_screen_url = KURL();
manifest->scope = KURL();
return manifest;
}
} // namespace
// static
const char ManifestManager::kSupplementName[] = "ManifestManager";
......@@ -78,8 +66,8 @@ void ManifestManager::RequestManifest(RequestManifestCallback callback) {
[](RequestManifestCallback callback, const KURL& manifest_url,
const mojom::blink::ManifestPtr& manifest,
const mojom::blink::ManifestDebugInfo* debug_info) {
std::move(callback).Run(manifest_url, manifest.is_null()
? CreateEmptyManifest()
std::move(callback).Run(
manifest_url, manifest.is_null() ? mojom::blink::Manifest::New()
: manifest->Clone());
},
std::move(callback)));
......
......@@ -218,15 +218,13 @@ KURL ManifestParser::ParseStartURL(const JSONObject* object) {
}
KURL ManifestParser::ParseScope(const JSONObject* object,
base::Optional<KURL>& start_url) {
const KURL& start_url) {
KURL scope = ParseURL(object, "scope", manifest_url_,
ParseURLOriginRestrictions::kSameOriginOnly);
// This will change to remove the |document_url_| fallback in the future.
// See https://github.com/w3c/manifest/issues/668.
const KURL& default_value = (!start_url.has_value() || start_url->IsEmpty())
? document_url_
: *start_url;
const KURL& default_value = start_url.IsEmpty() ? document_url_ : start_url;
DCHECK(default_value.IsValid());
if (scope.IsEmpty())
......
......@@ -97,7 +97,7 @@ class MODULES_EXPORT ManifestParser {
// https://w3c.github.io/manifest/#scope-member. Returns the parsed KURL if
// any, or start URL (falling back to document URL) without filename, path,
// and query if there is no defined scope or if the parsing failed.
KURL ParseScope(const JSONObject* object, base::Optional<KURL>& start_url);
KURL ParseScope(const JSONObject* object, const KURL& start_url);
// Parses the 'start_url' field of the manifest, as defined in:
// https://w3c.github.io/manifest/#dfn-steps-for-processing-the-start_url-member
......
......@@ -31,16 +31,14 @@ TypeConverter<blink::Manifest, blink::mojom::blink::ManifestPtr>::Convert(
base::NullableString16(blink::WebString(input->short_name).Utf16());
}
if (input->start_url.has_value())
output.start_url = *input->start_url;
if (!input->start_url.IsEmpty())
output.start_url = input->start_url;
output.display = input->display;
output.orientation = input->orientation;
if (input->icons.has_value()) {
for (auto& icon : *input->icons)
for (auto& icon : input->icons)
output.icons.push_back(icon.To<blink::Manifest::ImageResource>());
}
if (!input->share_target.is_null()) {
output.share_target =
......@@ -55,12 +53,10 @@ TypeConverter<blink::Manifest, blink::mojom::blink::ManifestPtr>::Convert(
output.file_handler = std::move(file_handler);
}
if (input->related_applications.has_value()) {
for (auto& related_application : *input->related_applications) {
for (auto& related_application : input->related_applications) {
output.related_applications.push_back(
related_application.To<blink::Manifest::RelatedApplication>());
}
}
output.prefer_related_applications = input->prefer_related_applications;
......@@ -70,16 +66,16 @@ TypeConverter<blink::Manifest, blink::mojom::blink::ManifestPtr>::Convert(
if (input->has_background_color)
output.background_color = input->background_color;
if (input->splash_screen_url.has_value())
output.splash_screen_url = *input->splash_screen_url;
if (!input->splash_screen_url.IsEmpty())
output.splash_screen_url = input->splash_screen_url;
if (!input->gcm_sender_id.IsEmpty()) {
output.gcm_sender_id =
base::NullableString16(blink::WebString(input->gcm_sender_id).Utf16());
}
if (input->scope.has_value())
output.scope = *input->scope;
if (!input->scope.IsEmpty())
output.scope = input->scope;
return output;
}
......
......@@ -41,14 +41,14 @@ void ManifestUmaUtil::ParseSucceeded(
UMA_HISTOGRAM_BOOLEAN("Manifest.HasProperty.short_name",
!manifest->short_name.IsEmpty());
UMA_HISTOGRAM_BOOLEAN("Manifest.HasProperty.start_url",
!manifest->start_url->IsEmpty());
!manifest->start_url.IsEmpty());
UMA_HISTOGRAM_BOOLEAN("Manifest.HasProperty.display",
manifest->display != kWebDisplayModeUndefined);
UMA_HISTOGRAM_BOOLEAN(
"Manifest.HasProperty.orientation",
manifest->orientation != kWebScreenOrientationLockDefault);
UMA_HISTOGRAM_BOOLEAN("Manifest.HasProperty.icons",
manifest->icons.has_value());
!manifest->icons.IsEmpty());
UMA_HISTOGRAM_BOOLEAN("Manifest.HasProperty.share_target",
manifest->share_target.get());
UMA_HISTOGRAM_BOOLEAN("Manifest.HasProperty.gcm_sender_id",
......
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