WebApp: Add GURL scope and theme_color WebApp data members.

We parse them from manifest during installability check.

Bug: 891172
Change-Id: I891dd48f29e6f43386c028a14f92c08db630da5e
Reviewed-on: https://chromium-review.googlesource.com/c/1325580
Commit-Queue: Alexey Baskakov <loyso@chromium.org>
Reviewed-by: Alan Cutter <alancutter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606395}

chrome/browser/web_applications/proto/web_app.proto

@@ -13,8 +13,10 @@ package web_app;
 // crbug.com/902214.
 message WebAppProto {
   // app_id is the client tag for sync system.
-  optional string app_id = 1;
+  required string app_id = 1;
   optional string name = 2;
   optional string description = 3;
-  optional string launch_url = 4;
+  required string launch_url = 4;
+  optional string scope = 5;
+  optional uint32 theme_color = 6;
 }

chrome/browser/web_applications/web_app.cc

@@ -2,10 +2,12 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <ios>
 #include <ostream>
 
 #include "base/logging.h"
 #include "chrome/browser/web_applications/web_app.h"
+#include "ui/gfx/color_utils.h"
 
 namespace web_app {
 
@@ -26,10 +28,26 @@ void WebApp::SetLaunchUrl(const GURL& launch_url) {
   launch_url_ = launch_url;
 }
 
+void WebApp::SetScope(const GURL& scope) {
+  DCHECK(scope.is_empty() || scope.is_valid());
+  scope_ = scope;
+}
+
+void WebApp::SetThemeColor(base::Optional<SkColor> theme_color) {
+  theme_color_ = theme_color;
+}
+
 std::ostream& operator<<(std::ostream& out, const WebApp& app) {
+  const std::string theme_color =
+      app.theme_color()
+          ? color_utils::SkColorToRgbaString(app.theme_color().value())
+          : "none";
+
   return out << "app_id: " << app.app_id() << std::endl
              << "  name: " << app.name() << std::endl
              << "  launch_url: " << app.launch_url() << std::endl
+             << "  scope: " << app.scope() << std::endl
+             << "  theme_color: " << theme_color << std::endl
              << "  description: " << app.description();
 }
 

chrome/browser/web_applications/web_app.h

@@ -9,7 +9,9 @@
 #include <string>
 
 #include "base/macros.h"
+#include "base/optional.h"
 #include "chrome/browser/web_applications/components/web_app_helpers.h"
+#include "third_party/skia/include/core/SkColor.h"
 #include "url/gurl.h"
 
 namespace web_app {
@@ -24,10 +26,14 @@ class WebApp {
   const std::string& name() const { return name_; }
   const std::string& description() const { return description_; }
   const GURL& launch_url() const { return launch_url_; }
+  const GURL& scope() const { return scope_; }
+  const base::Optional<SkColor>& theme_color() const { return theme_color_; }
 
   void SetName(const std::string& name);
   void SetDescription(const std::string& description);
   void SetLaunchUrl(const GURL& launch_url);
+  void SetScope(const GURL& scope);
+  void SetThemeColor(base::Optional<SkColor> theme_color);
 
  private:
   const AppId app_id_;
@@ -35,6 +41,10 @@ class WebApp {
   std::string name_;
   std::string description_;
   GURL launch_url_;
+  // TODO(loyso): Implement IsValid() function that verifies that the launch_url
+  // is within the scope.
+  GURL scope_;
+  base::Optional<SkColor> theme_color_;
 
   DISALLOW_COPY_AND_ASSIGN(WebApp);
 };

chrome/browser/web_applications/web_app_database.cc

@@ -67,6 +67,10 @@ std::unique_ptr<WebAppProto> WebAppDatabase::CreateWebAppProto(
   proto->set_name(web_app.name());
   proto->set_description(web_app.description());
   proto->set_launch_url(web_app.launch_url().spec());
+  if (!web_app.scope().is_empty())
+    proto->set_scope(web_app.scope().spec());
+  if (web_app.theme_color())
+    proto->set_theme_color(web_app.theme_color().value());
 
   return proto;
 }
@@ -86,6 +90,19 @@ std::unique_ptr<WebApp> WebAppDatabase::CreateWebApp(const WebAppProto& proto) {
   web_app->SetName(proto.name());
   web_app->SetDescription(proto.description());
 
+  if (proto.has_scope()) {
+    GURL scope(proto.scope());
+    if (scope.is_empty() || !scope.is_valid()) {
+      LOG(ERROR) << "WebApp proto scope parse error: "
+                 << scope.possibly_invalid_spec();
+      return nullptr;
+    }
+    web_app->SetScope(scope);
+  }
+
+  if (proto.has_theme_color())
+    web_app->SetThemeColor(proto.theme_color());
+
   return web_app;
 }
 

chrome/browser/web_applications/web_app_database_unittest.cc

@@ -27,9 +27,11 @@ namespace {
 
 bool operator==(const WebApp& web_app, const WebApp& web_app2) {
   return std::tie(web_app.app_id(), web_app.name(), web_app.launch_url(),
-                  web_app.description()) ==
+                  web_app.description(), web_app.scope(),
+                  web_app.theme_color()) ==
          std::tie(web_app2.app_id(), web_app2.name(), web_app2.launch_url(),
-                  web_app2.description());
+                  web_app2.description(), web_app2.scope(),
+                  web_app2.theme_color());
 }
 
 bool operator!=(const WebApp& web_app, const WebApp& web_app2) {
@@ -74,12 +76,16 @@ class WebAppDatabaseTest : public testing::Test {
     const AppId app_id = GenerateAppIdFromURL(GURL(launch_url));
     const std::string name = "Name" + base::IntToString(suffix);
     const std::string description = "Description" + base::IntToString(suffix);
+    const std::string scope = base_url + "/scope" + base::IntToString(suffix);
+    const base::Optional<SkColor> theme_color = suffix;
 
     auto app = std::make_unique<WebApp>(app_id);
 
     app->SetName(name);
     app->SetDescription(description);
     app->SetLaunchUrl(GURL(launch_url));
+    app->SetScope(GURL(scope));
+    app->SetThemeColor(theme_color);
 
     return app;
   }
@@ -178,4 +184,36 @@ TEST_F(WebAppDatabaseTest, OpenDatabaseAndReadRegistry) {
   EXPECT_TRUE(IsRegistryEqual(registrar_->registry(), registry));
 }
 
+TEST_F(WebAppDatabaseTest, WebAppWithoutOptionalFields) {
+  InitRegistrar();
+
+  const auto launch_url = GURL("https://example.com/");
+  const AppId app_id = GenerateAppIdFromURL(GURL(launch_url));
+
+  auto app = std::make_unique<WebApp>(app_id);
+
+  app->SetLaunchUrl(launch_url);
+  EXPECT_TRUE(app->name().empty());
+  EXPECT_TRUE(app->description().empty());
+  EXPECT_TRUE(app->scope().is_empty());
+  EXPECT_FALSE(app->theme_color().has_value());
+
+  registrar_->RegisterApp(std::move(app));
+
+  Registry registry = ReadRegistry();
+  EXPECT_EQ(1UL, registry.size());
+
+  std::unique_ptr<WebApp>& app_copy = registry.at(app_id);
+
+  // Mandatory members.
+  EXPECT_EQ(app_id, app_copy->app_id());
+  EXPECT_EQ(launch_url, app_copy->launch_url());
+
+  // No optional members.
+  EXPECT_TRUE(app_copy->name().empty());
+  EXPECT_TRUE(app_copy->description().empty());
+  EXPECT_TRUE(app_copy->scope().is_empty());
+  EXPECT_FALSE(app_copy->theme_color().has_value());
+}
+
 }  // namespace web_app

chrome/browser/web_applications/web_app_install_manager.cc

@@ -148,6 +148,8 @@ void WebAppInstallManager::OnDidPerformInstallableCheck(
   web_app->SetName(base::UTF16ToUTF8(web_app_info_->title));
   web_app->SetDescription(base::UTF16ToUTF8(web_app_info_->description));
   web_app->SetLaunchUrl(web_app_info_->app_url);
+  web_app->SetScope(web_app_info_->scope);
+  web_app->SetThemeColor(web_app_info_->theme_color);
 
   registrar_->RegisterApp(std::move(web_app));
 

chrome/browser/web_applications/web_app_install_manager_unittest.cc

@@ -65,17 +65,27 @@ class WebAppInstallManagerTest : public WebAppTest {
 
   void CreateRendererAppInfo(const GURL& url,
                              const std::string name,
-                             const std::string description) {
+                             const std::string description,
+                             const GURL& scope,
+                             base::Optional<SkColor> theme_color) {
     auto web_app_info = std::make_unique<WebApplicationInfo>();
 
     web_app_info->app_url = url;
     web_app_info->title = base::UTF8ToUTF16(name);
     web_app_info->description = base::UTF8ToUTF16(description);
+    web_app_info->scope = scope;
+    web_app_info->theme_color = theme_color;
 
     install_manager_->SetDataRetrieverForTesting(
         std::make_unique<TestDataRetriever>(std::move(web_app_info)));
   }
 
+  void CreateRendererAppInfo(const GURL& url,
+                             const std::string name,
+                             const std::string description) {
+    CreateRendererAppInfo(url, name, description, GURL(), base::nullopt);
+  }
+
   void CreateDefaultInstallableManager() {
     InstallableManager::CreateForWebContents(web_contents());
     // Required by InstallableManager.
@@ -118,9 +128,12 @@ TEST_F(WebAppInstallManagerTest, InstallFromWebContents) {
   const GURL url = GURL("https://example.com/path");
   const std::string name = "Name";
   const std::string description = "Description";
+  const GURL scope = GURL("https://example.com/scope");
+  const base::Optional<SkColor> theme_color = 0xAABBCCDD;
+
   const AppId app_id = GenerateAppIdFromURL(url);
 
-  CreateRendererAppInfo(url, name, description);
+  CreateRendererAppInfo(url, name, description, scope, theme_color);
   CreateDefaultInstallableManager();
 
   base::RunLoop run_loop;
@@ -146,7 +159,9 @@ TEST_F(WebAppInstallManagerTest, InstallFromWebContents) {
   EXPECT_EQ(app_id, web_app->app_id());
   EXPECT_EQ(name, web_app->name());
   EXPECT_EQ(description, web_app->description());
-  EXPECT_EQ(url.spec(), web_app->launch_url());
+  EXPECT_EQ(url, web_app->launch_url());
+  EXPECT_EQ(scope, web_app->scope());
+  EXPECT_EQ(theme_color, web_app->theme_color());
 }
 
 TEST_F(WebAppInstallManagerTest, GetWebApplicationInfoFailed) {
@@ -205,17 +220,22 @@ TEST_F(WebAppInstallManagerTest, WebContentsDestroyed) {
 // TODO(loyso): Convert more tests from bookmark_app_helper_unittest.cc
 TEST_F(WebAppInstallManagerTest, InstallableCheck) {
   const std::string renderer_description = "RendererDescription";
-  CreateRendererAppInfo(GURL("https://example.com/path"), "RendererName",
-                        renderer_description);
+  CreateRendererAppInfo(GURL("https://renderer.com/path"), "RendererName",
+                        renderer_description,
+                        GURL("https://renderer.com/scope"), 0x00);
 
   const GURL manifest_start_url = GURL("https://example.com/start");
   const AppId app_id = GenerateAppIdFromURL(manifest_start_url);
   const std::string manifest_name = "Name from Manifest";
+  const GURL manifest_scope = GURL("https://example.com/scope");
+  const base::Optional<SkColor> manifest_theme_color = 0xAABBCCDD;
 
   blink::Manifest manifest;
   manifest.short_name = ToNullableUTF16("Short Name from Manifest");
   manifest.name = ToNullableUTF16(manifest_name);
-  manifest.start_url = GURL(manifest_start_url);
+  manifest.start_url = manifest_start_url;
+  manifest.scope = manifest_scope;
+  manifest.theme_color = manifest_theme_color;
 
   CreateTestInstallableManager(GURL("https://example.com/manifest"), &manifest);
 
@@ -239,10 +259,13 @@ TEST_F(WebAppInstallManagerTest, InstallableCheck) {
   WebApp* web_app = registrar_->GetAppById(app_id);
   EXPECT_NE(nullptr, web_app);
 
+  // Manifest data overrides Renderer data, except |description|.
   EXPECT_EQ(app_id, web_app->app_id());
   EXPECT_EQ(manifest_name, web_app->name());
-  EXPECT_EQ(manifest_start_url.spec(), web_app->launch_url());
+  EXPECT_EQ(manifest_start_url, web_app->launch_url());
   EXPECT_EQ(renderer_description, web_app->description());
+  EXPECT_EQ(manifest_scope, web_app->scope());
+  EXPECT_EQ(manifest_theme_color, web_app->theme_color());
 }
 
 }  // namespace web_app

chrome/browser/web_applications/web_app_registrar_unittest.cc

@@ -55,6 +55,8 @@ TEST(WebAppRegistrar, CreateRegisterUnregister) {
   const AppId app_id = GenerateAppIdFromURL(launch_url);
   const std::string name = "Name";
   const std::string description = "Description";
+  const GURL scope = GURL("https://example.com/scope");
+  const base::Optional<SkColor> theme_color = 0xAABBCCDD;
 
   const GURL launch_url2 = GURL("https://example.com/path2");
   const AppId app_id2 = GenerateAppIdFromURL(launch_url2);
@@ -65,6 +67,8 @@ TEST(WebAppRegistrar, CreateRegisterUnregister) {
   web_app->SetName(name);
   web_app->SetDescription(description);
   web_app->SetLaunchUrl(launch_url);
+  web_app->SetScope(scope);
+  web_app->SetThemeColor(theme_color);
 
   EXPECT_EQ(nullptr, registrar->GetAppById(app_id));
   EXPECT_EQ(nullptr, registrar->GetAppById(app_id2));
@@ -76,7 +80,9 @@ TEST(WebAppRegistrar, CreateRegisterUnregister) {
   EXPECT_EQ(app_id, app->app_id());
   EXPECT_EQ(name, app->name());
   EXPECT_EQ(description, app->description());
-  EXPECT_EQ(launch_url.spec(), app->launch_url());
+  EXPECT_EQ(launch_url, app->launch_url());
+  EXPECT_EQ(scope, app->scope());
+  EXPECT_EQ(theme_color, app->theme_color());
 
   EXPECT_EQ(nullptr, registrar->GetAppById(app_id2));
   EXPECT_FALSE(registrar->is_empty());