Add support for 'start_url' in Manifest.

BUG=366145

Review URL: https://codereview.chromium.org/577673004

Cr-Commit-Position: refs/heads/master@{#295147}

8 files changed, 145 insertions, 21 deletions

diff --git a/content/browser/manifest/manifest_manager_host.cc b/content/browser/manifest/manifest_manager_host.cc
index d1bf291..796d0a9 100644
--- a/content/browser/manifest/manifest_manager_host.cc
+++ b/content/browser/manifest/manifest_manager_host.cc
@@ -109,6 +109,8 @@ void ManifestManagerHost::OnRequestManifestResponse(
   manifest.short_name = base::NullableString16(
         manifest.short_name.string().substr(0, Manifest::kMaxIPCStringLength),
         manifest.short_name.is_null());
+  if (!manifest.start_url.is_empty() && !manifest.start_url.is_valid())
+    manifest.start_url = GURL();
 
   callback->Run(manifest);
   callbacks->Remove(request_id);
diff --git a/content/common/manifest_manager_messages.h b/content/common/manifest_manager_messages.h
index 380b8a4..a415a9f 100644
--- a/content/common/manifest_manager_messages.h
+++ b/content/common/manifest_manager_messages.h
@@ -17,6 +17,7 @@
 IPC_STRUCT_TRAITS_BEGIN(content::Manifest)
   IPC_STRUCT_TRAITS_MEMBER(name)
   IPC_STRUCT_TRAITS_MEMBER(short_name)
+  IPC_STRUCT_TRAITS_MEMBER(start_url)
 IPC_STRUCT_TRAITS_END()
 
 // The browser process requests for the manifest linked with the associated
diff --git a/content/public/common/manifest.h b/content/public/common/manifest.h
index 4ee0957..1b9b3c7 100644
--- a/content/public/common/manifest.h
+++ b/content/public/common/manifest.h
@@ -7,6 +7,7 @@
 
 #include "base/strings/nullable_string16.h"
 #include "content/common/content_export.h"
+#include "url/gurl.h"
 
 namespace content {
 
@@ -27,6 +28,9 @@ struct CONTENT_EXPORT Manifest {
   // Null if the parsing failed or the field was not present.
   base::NullableString16 short_name;
 
+  // Empty if the parsing failed or the field was not present.
+  GURL start_url;
+
   // Maximum length for all the strings inside the Manifest when it is sent over
   // IPC. The renderer process should truncate the strings before sending the
   // Manifest and the browser process must do the same when receiving it.
diff --git a/content/renderer/manifest/manifest_manager.cc b/content/renderer/manifest/manifest_manager.cc
index ae702b3..88ef6ac 100644
--- a/content/renderer/manifest/manifest_manager.cc
+++ b/content/renderer/manifest/manifest_manager.cc
@@ -103,10 +103,12 @@ void ManifestManager::FetchManifest() {
   // CSP rule, see http://crbug.com/409996.
   fetcher_->Start(render_frame()->GetWebFrame(),
                   base::Bind(&ManifestManager::OnManifestFetchComplete,
-                             base::Unretained(this)));
+                             base::Unretained(this),
+                             render_frame()->GetWebFrame()->document().url()));
 }
 
 void ManifestManager::OnManifestFetchComplete(
+    const GURL& document_url,
     const blink::WebURLResponse& response,
     const std::string& data) {
   if (response.isNull() && data.empty()) {
@@ -114,7 +116,7 @@ void ManifestManager::OnManifestFetchComplete(
     return;
   }
 
-  manifest_ = ManifestParser::Parse(data);
+  manifest_ = ManifestParser::Parse(data, response.url(), document_url);
 
   fetcher_.reset();
   ResolveCallbacks(ResolveStateSuccess);
diff --git a/content/renderer/manifest/manifest_manager.h b/content/renderer/manifest/manifest_manager.h
index dbb241b..3361138 100644
--- a/content/renderer/manifest/manifest_manager.h
+++ b/content/renderer/manifest/manifest_manager.h
@@ -12,6 +12,8 @@
 #include "content/public/common/manifest.h"
 #include "content/public/renderer/render_frame_observer.h"
 
+class GURL;
+
 namespace blink {
 class WebURLResponse;
 }
@@ -53,7 +55,8 @@ class ManifestManager : public RenderFrameObserver {
   void OnRequestManifestComplete(int request_id, const Manifest&);
 
   void FetchManifest();
-  void OnManifestFetchComplete(const blink::WebURLResponse& response,
+  void OnManifestFetchComplete(const GURL& document_url,
+                               const blink::WebURLResponse& response,
                                const std::string& data);
   void ResolveCallbacks(ResolveState state);
 
diff --git a/content/renderer/manifest/manifest_parser.cc b/content/renderer/manifest/manifest_parser.cc
index 3e08f8c..19b69a5 100644
--- a/content/renderer/manifest/manifest_parser.cc
+++ b/content/renderer/manifest/manifest_parser.cc
@@ -32,7 +32,6 @@ base::NullableString16 ParseName(const base::DictionaryValue& dictionary)  {
 
 // Parses the 'short_name' field of the manifest, as defined in:
 // http://w3c.github.io/manifest/#dfn-steps-for-processing-the-short-name-member
-// |short_name| is an out parameter that must not be null.
 // Returns the parsed string if any, a null string if the parsing failed.
 base::NullableString16 ParseShortName(
     const base::DictionaryValue& dictionary)  {
@@ -49,11 +48,40 @@ base::NullableString16 ParseShortName(
   return base::NullableString16(short_name, false);
 }
 
+// Parses the 'start_url' field of the manifest, as defined in:
+// http://w3c.github.io/manifest/#dfn-steps-for-processing-the-start_url-member
+// Returns the parsed GURL if any, an empty GURL if the parsing failed.
+GURL ParseStartURL(const base::DictionaryValue& dictionary,
+                   const GURL& manifest_url,
+                   const GURL& document_url) {
+  if (!dictionary.HasKey("start_url"))
+    return GURL();
+
+  base::string16 start_url_str;
+  if (!dictionary.GetString("start_url", &start_url_str)) {
+    // TODO(mlamouri): provide a custom message to the developer console.
+    return GURL();
+  }
+
+  GURL start_url = manifest_url.Resolve(start_url_str);
+  if (!start_url.is_valid())
+    return GURL();
+
+  if (start_url.GetOrigin() != document_url.GetOrigin()) {
+    // TODO(mlamouri): provide a custom message to the developer console.
+    return GURL();
+  }
+
+  return start_url;
+}
+
 } // anonymous namespace
 
 namespace content {
 
-Manifest ManifestParser::Parse(const base::StringPiece& json) {
+Manifest ManifestParser::Parse(const base::StringPiece& json,
+                               const GURL& manifest_url,
+                               const GURL& document_url) {
   scoped_ptr<base::Value> value(base::JSONReader::Read(json));
   if (!value) {
     // TODO(mlamouri): get the JSON parsing error and report it to the developer
@@ -77,6 +105,7 @@ Manifest ManifestParser::Parse(const base::StringPiece& json) {
 
   manifest.name = ParseName(*dictionary);
   manifest.short_name = ParseShortName(*dictionary);
+  manifest.start_url = ParseStartURL(*dictionary, manifest_url, document_url);
 
   return manifest;
 }
diff --git a/content/renderer/manifest/manifest_parser.h b/content/renderer/manifest/manifest_parser.h
index 8d49b98..4881279 100644
--- a/content/renderer/manifest/manifest_parser.h
+++ b/content/renderer/manifest/manifest_parser.h
@@ -8,6 +8,8 @@
 #include "base/strings/string_piece.h"
 #include "content/common/content_export.h"
 
+class GURL;
+
 namespace base {
 class DictionaryValue;
 }
@@ -21,7 +23,9 @@ struct Manifest;
 // http://w3c.github.io/manifest/#dfn-steps-for-processing-a-manifest
 class CONTENT_EXPORT ManifestParser {
  public:
-  static Manifest Parse(const base::StringPiece&);
+  static Manifest Parse(const base::StringPiece&,
+                        const GURL& manifest_url,
+                        const GURL& document_url);
 };
 
 } // namespace content
diff --git a/content/renderer/manifest/manifest_parser_unittest.cc b/content/renderer/manifest/manifest_parser_unittest.cc
index eb75919..0e500ae 100644
--- a/content/renderer/manifest/manifest_parser_unittest.cc
+++ b/content/renderer/manifest/manifest_parser_unittest.cc
@@ -10,75 +10,154 @@
 
 namespace content {
 
-TEST(ManifestParserTest, EmptyStringNull) {
-  Manifest manifest = ManifestParser::Parse("");
+class ManifestParserTest : public testing::Test  {
+ protected:
+  ManifestParserTest() {}
+  virtual ~ManifestParserTest() {}
+
+  Manifest ParseManifest(const base::StringPiece& json,
+                         const GURL& document_url = default_document_url,
+                         const GURL& manifest_url = default_manifest_url) {
+    return ManifestParser::Parse(json, document_url, manifest_url);
+  }
+
+  static const GURL default_document_url;
+  static const GURL default_manifest_url;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(ManifestParserTest);
+};
+
+const GURL ManifestParserTest::default_document_url(
+    "http://foo.com/index.html");
+const GURL ManifestParserTest::default_manifest_url(
+    "http://foo.com/manifest.json");
+
+TEST_F(ManifestParserTest, EmptyStringNull) {
+  Manifest manifest = ParseManifest("");
 
   // A parsing error is equivalent to an empty manifest.
   ASSERT_TRUE(manifest.IsEmpty());
   ASSERT_TRUE(manifest.name.is_null());
   ASSERT_TRUE(manifest.short_name.is_null());
+  ASSERT_TRUE(manifest.start_url.is_empty());
 }
 
-TEST(ManifestParserTest, ValidNoContentParses) {
-  Manifest manifest = ManifestParser::Parse("{}");
+TEST_F(ManifestParserTest, ValidNoContentParses) {
+  Manifest manifest = ParseManifest("{}");
 
   // Check that all the fields are null in that case.
   ASSERT_TRUE(manifest.IsEmpty());
   ASSERT_TRUE(manifest.name.is_null());
   ASSERT_TRUE(manifest.short_name.is_null());
+  ASSERT_TRUE(manifest.start_url.is_empty());
 }
 
-TEST(ManifestParserTest, NameParseRules) {
+TEST_F(ManifestParserTest, NameParseRules) {
   // Smoke test.
   {
-    Manifest manifest = ManifestParser::Parse("{ \"name\": \"foo\" }");
+    Manifest manifest = ParseManifest("{ \"name\": \"foo\" }");
     ASSERT_TRUE(EqualsASCII(manifest.name.string(), "foo"));
   }
 
   // Trim whitespaces.
   {
-    Manifest manifest = ManifestParser::Parse("{ \"name\": \"  foo  \" }");
+    Manifest manifest = ParseManifest("{ \"name\": \"  foo  \" }");
     ASSERT_TRUE(EqualsASCII(manifest.name.string(), "foo"));
   }
 
   // Don't parse if name isn't a string.
   {
-    Manifest manifest = ManifestParser::Parse("{ \"name\": {} }");
+    Manifest manifest = ParseManifest("{ \"name\": {} }");
     ASSERT_TRUE(manifest.name.is_null());
   }
 
   // Don't parse if name isn't a string.
   {
-    Manifest manifest = ManifestParser::Parse("{ \"name\": 42 }");
+    Manifest manifest = ParseManifest("{ \"name\": 42 }");
     ASSERT_TRUE(manifest.name.is_null());
   }
 }
 
-TEST(ManifestParserTest, ShortNameParseRules) {
+TEST_F(ManifestParserTest, ShortNameParseRules) {
   // Smoke test.
   {
-    Manifest manifest = ManifestParser::Parse("{ \"short_name\": \"foo\" }");
+    Manifest manifest = ParseManifest("{ \"short_name\": \"foo\" }");
     ASSERT_TRUE(EqualsASCII(manifest.short_name.string(), "foo"));
   }
 
   // Trim whitespaces.
   {
-    Manifest manifest =
-        ManifestParser::Parse("{ \"short_name\": \"  foo  \" }");
+    Manifest manifest = ParseManifest("{ \"short_name\": \"  foo  \" }");
     ASSERT_TRUE(EqualsASCII(manifest.short_name.string(), "foo"));
   }
 
   // Don't parse if name isn't a string.
   {
-    Manifest manifest = ManifestParser::Parse("{ \"short_name\": {} }");
+    Manifest manifest = ParseManifest("{ \"short_name\": {} }");
     ASSERT_TRUE(manifest.short_name.is_null());
   }
 
   // Don't parse if name isn't a string.
   {
-    Manifest manifest = ManifestParser::Parse("{ \"short_name\": 42 }");
+    Manifest manifest = ParseManifest("{ \"short_name\": 42 }");
     ASSERT_TRUE(manifest.short_name.is_null());
   }
 }
 
+TEST_F(ManifestParserTest, StartURLParseRules) {
+  // Smoke test.
+  {
+    Manifest manifest = ParseManifest("{ \"start_url\": \"land.html\" }");
+    ASSERT_EQ(manifest.start_url.spec(),
+              default_document_url.Resolve("land.html").spec());
+  }
+
+  // Whitespaces.
+  {
+    Manifest manifest = ParseManifest("{ \"start_url\": \"  land.html  \" }");
+    ASSERT_EQ(manifest.start_url.spec(),
+              default_document_url.Resolve("land.html").spec());
+  }
+
+  // Don't parse if property isn't a string.
+  {
+    Manifest manifest = ParseManifest("{ \"start_url\": {} }");
+    ASSERT_TRUE(manifest.start_url.is_empty());
+  }
+
+  // Don't parse if property isn't a string.
+  {
+    Manifest manifest = ParseManifest("{ \"start_url\": 42 }");
+    ASSERT_TRUE(manifest.start_url.is_empty());
+  }
+
+  // Absolute start_url, same origin with document.
+  {
+    Manifest manifest =
+        ParseManifest("{ \"start_url\": \"http://foo.com/land.html\" }",
+                      GURL("http://foo.com/manifest.json"),
+                      GURL("http://foo.com/index.html"));
+    ASSERT_EQ(manifest.start_url.spec(), "http://foo.com/land.html");
+  }
+
+  // Absolute start_url, cross origin with document.
+  {
+    Manifest manifest =
+        ParseManifest("{ \"start_url\": \"http://bar.com/land.html\" }",
+                      GURL("http://foo.com/manifest.json"),
+                      GURL("http://foo.com/index.html"));
+    ASSERT_TRUE(manifest.start_url.is_empty());
+  }
+
+  // Resolving has to happen based on the manifest_url.
+  {
+    Manifest manifest =
+        ParseManifest("{ \"start_url\": \"land.html\" }",
+                      GURL("http://foo.com/landing/manifest.json"),
+                      GURL("http://foo.com/index.html"));
+    ASSERT_EQ(manifest.start_url.spec(), "http://foo.com/landing/land.html");
+  }
+}
+
 } // namespace content