Skip to content

Commit

Permalink
Add share_url_template field to Manifest.
Browse files Browse the repository at this point in the history
Adds the share_url_template field to the Manifest object, for use with
the Web Share Target API.

For use in: codereview.chromium.org/2639463002/
BUG=668389

Review-Url: https://codereview.chromium.org/2637003002
Cr-Commit-Position: refs/heads/master@{#445322}
  • Loading branch information
cpyro authored and Commit bot committed Jan 23, 2017
1 parent 9aad739 commit 6e068f8
Show file tree
Hide file tree
Showing 7 changed files with 154 additions and 9 deletions.
5 changes: 5 additions & 0 deletions content/common/manifest_manager_messages.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ IPC_STRUCT_TRAITS_BEGIN(content::Manifest::Icon)
IPC_STRUCT_TRAITS_MEMBER(sizes)
IPC_STRUCT_TRAITS_END()

IPC_STRUCT_TRAITS_BEGIN(content::Manifest::ShareTarget)
IPC_STRUCT_TRAITS_MEMBER(url_template)
IPC_STRUCT_TRAITS_END()

IPC_STRUCT_TRAITS_BEGIN(content::Manifest::RelatedApplication)
IPC_STRUCT_TRAITS_MEMBER(platform)
IPC_STRUCT_TRAITS_MEMBER(url)
Expand All @@ -34,6 +38,7 @@ IPC_STRUCT_TRAITS_BEGIN(content::Manifest)
IPC_STRUCT_TRAITS_MEMBER(display)
IPC_STRUCT_TRAITS_MEMBER(orientation)
IPC_STRUCT_TRAITS_MEMBER(icons)
IPC_STRUCT_TRAITS_MEMBER(share_target)
IPC_STRUCT_TRAITS_MEMBER(related_applications)
IPC_STRUCT_TRAITS_MEMBER(prefer_related_applications)
IPC_STRUCT_TRAITS_MEMBER(theme_color)
Expand Down
20 changes: 11 additions & 9 deletions content/public/common/manifest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,35 +14,36 @@ const int64_t Manifest::kInvalidOrMissingColor =
static_cast<int64_t>(std::numeric_limits<int32_t>::max()) + 1;
const size_t Manifest::kMaxIPCStringLength = 4 * 1024;

Manifest::Icon::Icon() { }
Manifest::Icon::Icon() = default;

Manifest::Icon::Icon(const Icon& other) = default;

Manifest::Icon::~Icon() {
}
Manifest::Icon::~Icon() = default;

bool Manifest::Icon::operator==(const Manifest::Icon& other) const {
return src == other.src && type == other.type && sizes == other.sizes;
}

Manifest::RelatedApplication::RelatedApplication() {
}
Manifest::ShareTarget::ShareTarget() = default;

Manifest::RelatedApplication::~RelatedApplication() {
}
Manifest::ShareTarget::~ShareTarget() = default;

Manifest::RelatedApplication::RelatedApplication() = default;

Manifest::RelatedApplication::~RelatedApplication() = default;

Manifest::Manifest()
: display(blink::WebDisplayModeUndefined),
orientation(blink::WebScreenOrientationLockDefault),
prefer_related_applications(false),
theme_color(Manifest::kInvalidOrMissingColor),
background_color(Manifest::kInvalidOrMissingColor) {
share_target = base::nullopt;
}

Manifest::Manifest(const Manifest& other) = default;

Manifest::~Manifest() {
}
Manifest::~Manifest() = default;

bool Manifest::IsEmpty() const {
return name.is_null() &&
Expand All @@ -51,6 +52,7 @@ bool Manifest::IsEmpty() const {
display == blink::WebDisplayModeUndefined &&
orientation == blink::WebScreenOrientationLockDefault &&
icons.empty() &&
!share_target.has_value() &&
related_applications.empty() &&
!prefer_related_applications &&
theme_color == Manifest::kInvalidOrMissingColor &&
Expand Down
18 changes: 18 additions & 0 deletions content/public/common/manifest.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

#include <vector>

#include "base/optional.h"
#include "base/strings/nullable_string16.h"
#include "base/strings/string16.h"
#include "content/common/content_export.h"
Expand Down Expand Up @@ -58,6 +59,16 @@ struct CONTENT_EXPORT Manifest {
std::vector<IconPurpose> purpose;
};

// Structure representing how a Web Share target handles an incoming share.
struct CONTENT_EXPORT ShareTarget {
ShareTarget();
~ShareTarget();

// The URL template that contains placeholders to be replaced with shared
// data. Null if the parsing failed.
base::NullableString16 url_template;
};

// Structure representing a related application.
struct CONTENT_EXPORT RelatedApplication {
RelatedApplication();
Expand Down Expand Up @@ -107,6 +118,13 @@ struct CONTENT_EXPORT Manifest {
// icons inside the JSON array were invalid.
std::vector<Icon> icons;

// Null if parsing failed or the field was not present.
// TODO(constantina): This field is non-standard and part of a Chrome
// experiment. See:
// https://github.com/WICG/web-share-target/blob/master/docs/interface.md
// As such, this field should not be exposed to web contents.
base::Optional<ShareTarget> share_target;

// Empty if the parsing failed, the field was not present, empty or all the
// applications inside the array were invalid. The order of the array
// indicates the priority of the application to use.
Expand Down
22 changes: 22 additions & 0 deletions content/renderer/manifest/manifest_parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ void ManifestParser::Parse() {
manifest_.display = ParseDisplay(*dictionary);
manifest_.orientation = ParseOrientation(*dictionary);
manifest_.icons = ParseIcons(*dictionary);
manifest_.share_target = ParseShareTarget(*dictionary);
manifest_.related_applications = ParseRelatedApplications(*dictionary);
manifest_.prefer_related_applications =
ParsePreferRelatedApplications(*dictionary);
Expand Down Expand Up @@ -340,6 +341,27 @@ std::vector<Manifest::Icon> ManifestParser::ParseIcons(
return icons;
}

base::NullableString16 ManifestParser::ParseShareTargetURLTemplate(
const base::DictionaryValue& share_target) {
return ParseString(share_target, "url_template", Trim);
}

base::Optional<Manifest::ShareTarget> ManifestParser::ParseShareTarget(
const base::DictionaryValue& dictionary) {
if (!dictionary.HasKey("share_target"))
return base::nullopt;

Manifest::ShareTarget share_target;
const base::DictionaryValue* share_target_dict = nullptr;
dictionary.GetDictionary("share_target", &share_target_dict);
share_target.url_template = ParseShareTargetURLTemplate(*share_target_dict);

if (share_target.url_template.is_null()) {
return base::nullopt;
}
return base::Optional<Manifest::ShareTarget>(share_target);
}

base::NullableString16 ManifestParser::ParseRelatedApplicationPlatform(
const base::DictionaryValue& application) {
return ParseString(application, "platform", Trim);
Expand Down
15 changes: 15 additions & 0 deletions content/renderer/manifest/manifest_parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include <memory>

#include "base/macros.h"
#include "base/optional.h"
#include "base/strings/nullable_string16.h"
#include "base/strings/string_piece.h"
#include "content/common/content_export.h"
Expand Down Expand Up @@ -146,6 +147,20 @@ class CONTENT_EXPORT ManifestParser {
std::vector<Manifest::Icon> ParseIcons(
const base::DictionaryValue& dictionary);

// Parses the 'url_template' field of a Share Target, as defined in:
// https://github.com/WICG/web-share-target/blob/master/docs/interface.md
// Returns the parsed string if any, or a null string if the field was not
// present, or didn't contain a string.
base::NullableString16 ParseShareTargetURLTemplate(
const base::DictionaryValue& share_target);

// Parses the 'share_target' field of a Manifest, as defined in:
// https://github.com/WICG/web-share-target/blob/master/docs/interface.md
// Returns the parsed Web Share target. The returned Share Target is null if
// the field didn't exist, parsing failed, or it was empty.
base::Optional<Manifest::ShareTarget> ParseShareTarget(
const base::DictionaryValue& dictionary);

// Parses the 'platform' field of a related application, as defined in:
// https://w3c.github.io/manifest/#dfn-steps-for-processing-the-platform-member-of-an-application
// Returns the parsed string if any, a null string if the parsing failed.
Expand Down
81 changes: 81 additions & 0 deletions content/renderer/manifest/manifest_parser_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include <memory>

#include "base/macros.h"
#include "base/optional.h"
#include "base/strings/string_util.h"
#include "content/public/common/manifest.h"
#include "testing/gtest/include/gtest/gtest.h"
Expand Down Expand Up @@ -982,6 +983,86 @@ TEST_F(ManifestParserTest, IconPurposeParseRules) {
}
}

TEST_F(ManifestParserTest, ShareTargetParseRules) {
// Contains share_target field but no keys.
{
Manifest manifest = ParseManifest("{ \"share_target\": {} }");
EXPECT_FALSE(manifest.share_target.has_value());
EXPECT_TRUE(manifest.IsEmpty());
EXPECT_EQ(0u, GetErrorCount());
}

// Key in share_target that isn't valid.
{
Manifest manifest = ParseManifest(
"{ \"share_target\": {\"incorrect_key\": \"some_value\" } }");
ASSERT_FALSE(manifest.share_target.has_value());
EXPECT_TRUE(manifest.IsEmpty());
EXPECT_EQ(0u, GetErrorCount());
}
}

TEST_F(ManifestParserTest, ShareTargetUrlTemplateParseRules) {
// Contains share_target and url_template, but url_template is empty.
{
Manifest manifest =
ParseManifest("{ \"share_target\": { \"url_template\": \"\" } }");
ASSERT_TRUE(manifest.share_target.has_value());
EXPECT_TRUE(base::EqualsASCII(
manifest.share_target.value().url_template.string(), ""));
EXPECT_FALSE(manifest.IsEmpty());
EXPECT_EQ(0u, GetErrorCount());
}

// Don't parse if property isn't a string.
{
Manifest manifest =
ParseManifest("{ \"share_target\": { \"url_template\": {} } }");
EXPECT_FALSE(manifest.share_target.has_value());
EXPECT_TRUE(manifest.IsEmpty());
EXPECT_EQ(1u, GetErrorCount());
EXPECT_EQ("property 'url_template' ignored, type string expected.",
errors()[0]);
}

// Don't parse if property isn't a string.
{
Manifest manifest =
ParseManifest("{ \"share_target\": { \"url_template\": 42 } }");
EXPECT_FALSE(manifest.share_target.has_value());
EXPECT_TRUE(manifest.IsEmpty());
EXPECT_EQ(1u, GetErrorCount());
EXPECT_EQ("property 'url_template' ignored, type string expected.",
errors()[0]);
}

// Smoke test: Contains share_target and url_template, and url_template is
// valid template.
{
Manifest manifest = ParseManifest(
"{ \"share_target\": {\"url_template\": \"share/?title={title}\" } }");
ASSERT_TRUE(manifest.share_target.has_value());
EXPECT_TRUE(
base::EqualsASCII(manifest.share_target.value().url_template.string(),
"share/?title={title}"));
EXPECT_FALSE(manifest.IsEmpty());
EXPECT_EQ(0u, GetErrorCount());
}

// Smoke test: Contains share_target and url_template, and url_template is
// invalid template.
{
Manifest manifest = ParseManifest(
"{ \"share_target\": {\"url_template\": \"share/?title={title\" } }");
ASSERT_TRUE(manifest.share_target.has_value());
EXPECT_TRUE(
base::EqualsASCII(manifest.share_target.value().url_template.string(),
"share/?title={title"));
EXPECT_FALSE(manifest.IsEmpty());
EXPECT_EQ(0u, GetErrorCount());
}
}

TEST_F(ManifestParserTest, RelatedApplicationsParseRules) {
// If no application, empty list.
{
Expand Down
2 changes: 2 additions & 0 deletions content/renderer/manifest/manifest_uma_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ void ManifestUmaUtil::ParseSucceeded(const Manifest& manifest) {
UMA_HISTOGRAM_BOOLEAN("Manifest.HasProperty.orientation",
manifest.orientation != blink::WebScreenOrientationLockDefault);
UMA_HISTOGRAM_BOOLEAN("Manifest.HasProperty.icons", !manifest.icons.empty());
UMA_HISTOGRAM_BOOLEAN("Manifest.HasProperty.share_target",
manifest.share_target.has_value());
UMA_HISTOGRAM_BOOLEAN("Manifest.HasProperty.gcm_sender_id",
!manifest.gcm_sender_id.is_null());
}
Expand Down

0 comments on commit 6e068f8

Please sign in to comment.