WebApp: Add local name, theme_color and is_sync_placeholder fields.

According to go/chrome-bmo design doc sync data model,
we should have two copies of app's name and theme_color fields.

Rename specifics to sync_data.
Rename proto to local_data.

Add is_sync_placeholder flag.

Add comments.

All these fields will be used in WebAppSyncBridge implementation:
https://chromium-review.googlesource.com/c/chromium/src/+/1830494

Bug: 860583
Change-Id: Idce95b5fdd4843cf9a8e290fe61293bffb938bc5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1833347
Commit-Queue: Alexey Baskakov <loyso@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Reviewed-by: Alan Cutter <alancutter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#702713}

chrome/browser/web_applications/proto/web_app.proto

@@ -28,19 +28,27 @@ message SourcesProto {
   required bool default = 5;
 }
 
-// Full WebApp object data. Note on database identities:
+// Full WebApp object data. See detailed comments in
+// chrome/browser/web_applications/web_app.h. Note on database identities:
 // app_id is a hash of launch_url. app_id is the client tag for sync system.
 // app_id is the storage key in ModelTypeStore.
 message WebAppProto {
-  // Synced data.
-  required sync_pb.WebAppSpecifics specifics = 1;
+  // Synced data. It is replicated across all devices with WEB_APPS.
+  //
+  // |sync_data.name| and |sync_data.theme_color| are read by a device to
+  // generate a placeholder icon. Any device may write new values to synced
+  // |name| and |theme_color|. A random last update wins.
+  required sync_pb.WebAppSpecifics sync_data = 1;
 
-  // Local data. Not to be synced.
+  // Local data. May vary across devices. Not to be synced.
   //
-  optional string description = 2;
-  optional string scope = 3;
-  required SourcesProto sources = 4;
-  required bool is_locally_installed = 5;
+  required string name = 2;
+  optional uint32 theme_color = 3;
+  optional string description = 4;
+  optional string scope = 5;
+  required SourcesProto sources = 6;
+  required bool is_locally_installed = 7;
+  optional bool is_sync_placeholder = 8;
   // A list of icon infos.
-  repeated WebAppIconInfoProto icons = 6;
+  repeated WebAppIconInfoProto icons = 9;
 }

chrome/browser/web_applications/web_app.cc

@@ -65,18 +65,35 @@ void WebApp::SetIsLocallyInstalled(bool is_locally_installed) {
   is_locally_installed_ = is_locally_installed;
 }
 
+void WebApp::SetIsSyncPlaceholder(bool is_sync_placeholder) {
+  is_sync_placeholder_ = is_sync_placeholder;
+}
+
 void WebApp::SetIcons(Icons icons) {
   icons_ = std::move(icons);
 }
 
+void WebApp::SetSyncData(const SyncData& sync_data) {
+  sync_data_ = sync_data;
+}
+
+WebApp::SyncData::SyncData() = default;
+
+WebApp::SyncData::~SyncData() = default;
+
 std::ostream& operator<<(std::ostream& out, const WebApp& app) {
   const std::string theme_color =
-      app.theme_color()
-          ? color_utils::SkColorToRgbaString(app.theme_color().value())
+      app.theme_color_.has_value()
+          ? color_utils::SkColorToRgbaString(app.theme_color_.value())
+          : "none";
+  const std::string sync_theme_color =
+      app.sync_data_.theme_color.has_value()
+          ? color_utils::SkColorToRgbaString(app.sync_data_.theme_color.value())
           : "none";
   const char* launch_container =
-      LaunchContainerEnumToStr(app.launch_container());
-  const bool is_locally_installed = app.is_locally_installed();
+      LaunchContainerEnumToStr(app.launch_container_);
+  const bool is_locally_installed = app.is_locally_installed_;
+  const bool is_sync_placeholder = app.is_sync_placeholder_;
 
   return out << "app_id: " << app.app_id_ << std::endl
              << "  name: " << app.name_ << std::endl
@@ -84,8 +101,11 @@ std::ostream& operator<<(std::ostream& out, const WebApp& app) {
              << "  scope: " << app.scope_ << std::endl
              << "  theme_color: " << theme_color << std::endl
              << "  launch_container: " << launch_container << std::endl
-             << "  sources: " << app.sources_ << std::endl
+             << "  sources: " << app.sources_.to_string() << std::endl
              << "  is_locally_installed: " << is_locally_installed << std::endl
+             << "  is_sync_placeholder: " << is_sync_placeholder << std::endl
+             << "  sync_data.name: " << app.sync_data_.name << std::endl
+             << "  sync_data.theme_color: " << sync_theme_color << std::endl
              << "  description: " << app.description_;
 }
 
@@ -95,15 +115,23 @@ bool operator==(const WebApp::IconInfo& icon_info1,
          std::tie(icon_info2.url, icon_info2.size_in_px);
 }
 
+bool operator==(const WebApp::SyncData& sync_data1,
+                const WebApp::SyncData& sync_data2) {
+  return std::tie(sync_data1.name, sync_data1.theme_color) ==
+         std::tie(sync_data2.name, sync_data2.theme_color);
+}
+
 bool operator==(const WebApp& app1, const WebApp& app2) {
   return std::tie(app1.app_id_, app1.sources_, app1.name_, app1.launch_url_,
                   app1.description_, app1.scope_, app1.theme_color_,
                   app1.icons_, app1.launch_container_,
-                  app1.is_locally_installed_) ==
+                  app1.is_locally_installed_, app1.is_sync_placeholder_,
+                  app1.sync_data_) ==
          std::tie(app2.app_id_, app2.sources_, app2.name_, app2.launch_url_,
                   app2.description_, app2.scope_, app2.theme_color_,
                   app2.icons_, app2.launch_container_,
-                  app2.is_locally_installed_);
+                  app2.is_locally_installed_, app2.is_sync_placeholder_,
+                  app2.sync_data_);
 }
 
 bool operator!=(const WebApp& app1, const WebApp& app2) {

chrome/browser/web_applications/web_app.h

@@ -35,6 +35,12 @@ class WebApp {
   const base::Optional<SkColor>& theme_color() const { return theme_color_; }
   LaunchContainer launch_container() const { return launch_container_; }
   bool is_locally_installed() const { return is_locally_installed_; }
+  // Sync-initiated installation produces a sync placeholder app awaiting for
+  // full installation process. The sync placeholder app has only app_id,
+  // launch_url and sync_data fields defined, no icons. If online install
+  // succeeds, icons get downloaded and all the fields get their values. If
+  // install fails, icons get generated using |sync_data| fields.
+  bool is_sync_placeholder() const { return is_sync_placeholder_; }
 
   struct IconInfo {
     GURL url;
@@ -43,7 +49,19 @@ class WebApp {
   using Icons = std::vector<IconInfo>;
   const Icons& icons() const { return icons_; }
 
-  // A Web App can be installed from multiple sources simulateneously. Installs
+  // While local |name| and |theme_color| may vary from device to device, the
+  // synced copies of these fields are replicated to all devices. The synced
+  // copies are read by a device to generate a placeholder icon (if needed). Any
+  // device may write new values to |sync_data|, random last update wins.
+  struct SyncData {
+    SyncData();
+    ~SyncData();
+    std::string name;
+    base::Optional<SkColor> theme_color;
+  };
+  const SyncData& sync_data() const { return sync_data_; }
+
+  // A Web App can be installed from multiple sources simultaneously. Installs
   // add a source to the app. Uninstalls remove a source from the app.
   void AddSource(Source::Type source);
   void RemoveSource(Source::Type source);
@@ -58,8 +76,11 @@ class WebApp {
   void SetThemeColor(base::Optional<SkColor> theme_color);
   void SetLaunchContainer(LaunchContainer launch_container);
   void SetIsLocallyInstalled(bool is_locally_installed);
+  void SetIsSyncPlaceholder(bool is_sync_placeholder);
   void SetIcons(Icons icons);
 
+  void SetSyncData(const SyncData& sync_data);
+
  private:
   friend class WebAppDatabase;
   friend bool operator==(const WebApp&, const WebApp&);
@@ -80,8 +101,11 @@ class WebApp {
   base::Optional<SkColor> theme_color_;
   LaunchContainer launch_container_;
   bool is_locally_installed_ = true;
+  bool is_sync_placeholder_ = false;
   Icons icons_;
 
+  SyncData sync_data_;
+
   DISALLOW_COPY_AND_ASSIGN(WebApp);
 };
 
@@ -91,6 +115,9 @@ std::ostream& operator<<(std::ostream& out, const WebApp& app);
 bool operator==(const WebApp::IconInfo& icon_info1,
                 const WebApp::IconInfo& icon_info2);
 
+bool operator==(const WebApp::SyncData& sync_data1,
+                const WebApp::SyncData& sync_data2);
+
 bool operator==(const WebApp& app1, const WebApp& app2);
 
 bool operator!=(const WebApp& app1, const WebApp& app2);

chrome/browser/web_applications/web_app_database.cc

@@ -86,7 +86,7 @@ void WebAppDatabase::CancelTransaction() {
 // static
 std::unique_ptr<WebAppProto> WebAppDatabase::CreateWebAppProto(
     const WebApp& web_app) {
-  auto proto = std::make_unique<WebAppProto>();
+  auto local_data = std::make_unique<WebAppProto>();
 
   // Required fields:
   const GURL launch_url = web_app.launch_url();
@@ -95,55 +95,64 @@ std::unique_ptr<WebAppProto> WebAppDatabase::CreateWebAppProto(
   DCHECK(!web_app.app_id().empty());
   DCHECK_EQ(web_app.app_id(), GenerateAppIdFromURL(launch_url));
 
-  sync_pb::WebAppSpecifics* specifics = proto->mutable_specifics();
+  sync_pb::WebAppSpecifics* sync_data = local_data->mutable_sync_data();
 
-  specifics->set_launch_url(launch_url.spec());
+  sync_data->set_launch_url(launch_url.spec());
 
-  specifics->set_name(web_app.name());
+  local_data->set_name(web_app.name());
 
   DCHECK_NE(LaunchContainer::kDefault, web_app.launch_container());
-  specifics->set_launch_container(web_app.launch_container() ==
+  sync_data->set_launch_container(web_app.launch_container() ==
                                           LaunchContainer::kWindow
                                       ? sync_pb::WebAppSpecifics::WINDOW
                                       : sync_pb::WebAppSpecifics::TAB);
 
   DCHECK(web_app.sources_.any());
-  proto->mutable_sources()->set_system(web_app.sources_[Source::kSystem]);
-  proto->mutable_sources()->set_policy(web_app.sources_[Source::kPolicy]);
-  proto->mutable_sources()->set_web_app_store(
+  local_data->mutable_sources()->set_system(web_app.sources_[Source::kSystem]);
+  local_data->mutable_sources()->set_policy(web_app.sources_[Source::kPolicy]);
+  local_data->mutable_sources()->set_web_app_store(
       web_app.sources_[Source::kWebAppStore]);
-  proto->mutable_sources()->set_sync(web_app.sources_[Source::kSync]);
-  proto->mutable_sources()->set_default_(web_app.sources_[Source::kDefault]);
+  local_data->mutable_sources()->set_sync(web_app.sources_[Source::kSync]);
+  local_data->mutable_sources()->set_default_(
+      web_app.sources_[Source::kDefault]);
 
-  proto->set_is_locally_installed(web_app.is_locally_installed());
+  local_data->set_is_locally_installed(web_app.is_locally_installed());
 
   // Optional fields:
-  proto->set_description(web_app.description());
+  local_data->set_description(web_app.description());
   if (!web_app.scope().is_empty())
-    proto->set_scope(web_app.scope().spec());
+    local_data->set_scope(web_app.scope().spec());
   if (web_app.theme_color().has_value())
-    specifics->set_theme_color(web_app.theme_color().value());
+    local_data->set_theme_color(web_app.theme_color().value());
+
+  local_data->set_is_sync_placeholder(web_app.is_sync_placeholder());
+
+  // Set sync_data to sync proto.
+  sync_data->set_name(web_app.sync_data().name);
+  if (web_app.sync_data().theme_color.has_value())
+    sync_data->set_theme_color(web_app.sync_data().theme_color.value());
 
   for (const WebApp::IconInfo& icon : web_app.icons()) {
-    WebAppIconInfoProto* icon_proto = proto->add_icons();
+    WebAppIconInfoProto* icon_proto = local_data->add_icons();
     icon_proto->set_url(icon.url.spec());
     icon_proto->set_size_in_px(icon.size_in_px);
   }
 
-  return proto;
+  return local_data;
 }
 
 // static
-std::unique_ptr<WebApp> WebAppDatabase::CreateWebApp(const WebAppProto& proto) {
-  if (!proto.has_specifics()) {
-    DLOG(ERROR) << "WebApp proto parse error: no specifics field";
+std::unique_ptr<WebApp> WebAppDatabase::CreateWebApp(
+    const WebAppProto& local_data) {
+  if (!local_data.has_sync_data()) {
+    DLOG(ERROR) << "WebApp proto parse error: no sync_data field";
     return nullptr;
   }
 
-  const sync_pb::WebAppSpecifics& specifics = proto.specifics();
+  const sync_pb::WebAppSpecifics& sync_data = local_data.sync_data();
 
   // AppId is a hash of launch_url. Read launch_url first:
-  GURL launch_url(specifics.launch_url());
+  GURL launch_url(sync_data.launch_url());
   if (launch_url.is_empty() || !launch_url.is_valid()) {
     DLOG(ERROR) << "WebApp proto launch_url parse error: "
                 << launch_url.possibly_invalid_spec();
@@ -156,50 +165,50 @@ std::unique_ptr<WebApp> WebAppDatabase::CreateWebApp(const WebAppProto& proto) {
   web_app->SetLaunchUrl(launch_url);
 
   // Required fields:
-  if (!proto.has_sources()) {
+  if (!local_data.has_sources()) {
     DLOG(ERROR) << "WebApp proto parse error: no sources field";
     return nullptr;
   }
 
   WebApp::Sources sources;
-  sources[Source::kSystem] = proto.sources().system();
-  sources[Source::kPolicy] = proto.sources().policy();
-  sources[Source::kWebAppStore] = proto.sources().web_app_store();
-  sources[Source::kSync] = proto.sources().sync();
-  sources[Source::kDefault] = proto.sources().default_();
+  sources[Source::kSystem] = local_data.sources().system();
+  sources[Source::kPolicy] = local_data.sources().policy();
+  sources[Source::kWebAppStore] = local_data.sources().web_app_store();
+  sources[Source::kSync] = local_data.sources().sync();
+  sources[Source::kDefault] = local_data.sources().default_();
   if (!sources.any()) {
     DLOG(ERROR) << "WebApp proto parse error: no any source in sources field";
     return nullptr;
   }
   web_app->sources_ = sources;
 
-  if (!specifics.has_name()) {
+  if (!local_data.has_name()) {
     DLOG(ERROR) << "WebApp proto parse error: no name field";
     return nullptr;
   }
-  web_app->SetName(specifics.name());
+  web_app->SetName(local_data.name());
 
-  if (!specifics.has_launch_container()) {
+  if (!sync_data.has_launch_container()) {
     DLOG(ERROR) << "WebApp proto parse error: no launch_container field";
     return nullptr;
   }
-  web_app->SetLaunchContainer(specifics.launch_container() ==
+  web_app->SetLaunchContainer(sync_data.launch_container() ==
                                       sync_pb::WebAppSpecifics::WINDOW
                                   ? LaunchContainer::kWindow
                                   : LaunchContainer::kTab);
 
-  if (!proto.has_is_locally_installed()) {
+  if (!local_data.has_is_locally_installed()) {
     DLOG(ERROR) << "WebApp proto parse error: no is_locally_installed field";
     return nullptr;
   }
-  web_app->SetIsLocallyInstalled(proto.is_locally_installed());
+  web_app->SetIsLocallyInstalled(local_data.is_locally_installed());
 
   // Optional fields:
-  if (proto.has_description())
-    web_app->SetDescription(proto.description());
+  if (local_data.has_description())
+    web_app->SetDescription(local_data.description());
 
-  if (proto.has_scope()) {
-    GURL scope(proto.scope());
+  if (local_data.has_scope()) {
+    GURL scope(local_data.scope());
     if (scope.is_empty() || !scope.is_valid()) {
       DLOG(ERROR) << "WebApp proto scope parse error: "
                   << scope.possibly_invalid_spec();
@@ -208,12 +217,23 @@ std::unique_ptr<WebApp> WebAppDatabase::CreateWebApp(const WebAppProto& proto) {
     web_app->SetScope(scope);
   }
 
-  if (specifics.has_theme_color())
-    web_app->SetThemeColor(specifics.theme_color());
+  if (local_data.has_theme_color())
+    web_app->SetThemeColor(local_data.theme_color());
+
+  if (local_data.has_is_sync_placeholder())
+    web_app->SetIsSyncPlaceholder(local_data.is_sync_placeholder());
+
+  // Parse sync_data from sync proto.
+  WebApp::SyncData parsed_sync_data;
+  if (sync_data.has_name())
+    parsed_sync_data.name = sync_data.name();
+  if (sync_data.has_theme_color())
+    parsed_sync_data.theme_color = sync_data.theme_color();
+  web_app->SetSyncData(parsed_sync_data);
 
   WebApp::Icons icons;
-  for (int i = 0; i < proto.icons_size(); ++i) {
-    const WebAppIconInfoProto& icon_proto = proto.icons(i);
+  for (int i = 0; i < local_data.icons_size(); ++i) {
+    const WebAppIconInfoProto& icon_proto = local_data.icons(i);
 
     GURL icon_url(icon_proto.url());
     if (icon_url.is_empty() || !icon_url.is_valid()) {

chrome/browser/web_applications/web_app_database.h

@@ -58,7 +58,7 @@ class WebAppDatabase {
                                              const std::string& value);
 
  private:
-  static std::unique_ptr<WebApp> CreateWebApp(const WebAppProto& proto);
+  static std::unique_ptr<WebApp> CreateWebApp(const WebAppProto& local_data);
 
   void OnDatabaseOpened(RegistryOpenedCallback callback,
                         const base::Optional<syncer::ModelError>& error,

chrome/browser/web_applications/web_app_database_unittest.cc

@@ -46,6 +46,7 @@ class WebAppDatabaseTest : public WebAppTest {
     const std::string scope =
         base_url + "/scope" + base::NumberToString(suffix);
     const base::Optional<SkColor> theme_color = suffix;
+    const base::Optional<SkColor> synced_theme_color = suffix ^ UINT_MAX;
 
     auto app = std::make_unique<WebApp>(app_id);
 
@@ -69,6 +70,7 @@ class WebAppDatabaseTest : public WebAppTest {
     app->SetScope(GURL(scope));
     app->SetThemeColor(theme_color);
     app->SetIsLocallyInstalled(!(suffix & 2));
+    app->SetIsSyncPlaceholder(!(suffix & 4));
     app->SetLaunchContainer((suffix & 1) ? LaunchContainer::kTab
                                          : LaunchContainer::kWindow);
 
@@ -81,6 +83,11 @@ class WebAppDatabaseTest : public WebAppTest {
 
     app->SetIcons(std::move(icons));
 
+    WebApp::SyncData sync_data;
+    sync_data.name = "Sync" + name;
+    sync_data.theme_color = synced_theme_color;
+    app->SetSyncData(sync_data);
+
     return app;
   }
 
@@ -263,6 +270,9 @@ TEST_F(WebAppDatabaseTest, WebAppWithoutOptionalFields) {
   EXPECT_TRUE(app->scope().is_empty());
   EXPECT_FALSE(app->theme_color().has_value());
   EXPECT_TRUE(app->icons().empty());
+  EXPECT_FALSE(app->is_sync_placeholder());
+  EXPECT_TRUE(app->sync_data().name.empty());
+  EXPECT_FALSE(app->sync_data().theme_color.has_value());
   controller().RegisterApp(std::move(app));
 
   Registry registry = database_factory().ReadRegistry();
@@ -288,6 +298,9 @@ TEST_F(WebAppDatabaseTest, WebAppWithoutOptionalFields) {
   EXPECT_TRUE(app_copy->scope().is_empty());
   EXPECT_FALSE(app_copy->theme_color().has_value());
   EXPECT_TRUE(app_copy->icons().empty());
+  EXPECT_FALSE(app_copy->is_sync_placeholder());
+  EXPECT_TRUE(app_copy->sync_data().name.empty());
+  EXPECT_FALSE(app_copy->sync_data().theme_color.has_value());
 }
 
 TEST_F(WebAppDatabaseTest, WebAppWithManyIcons) {

chrome/browser/web_applications/web_app_install_finalizer.cc

@@ -129,6 +129,12 @@ void WebAppInstallFinalizer::FinalizeInstall(
 
   SetIcons(web_app_info, web_app.get());
 
+  web_app->SetIsSyncPlaceholder(false);
+  WebApp::SyncData sync_data;
+  sync_data.name = base::UTF16ToUTF8(web_app_info.title);
+  sync_data.theme_color = web_app_info.theme_color;
+  web_app->SetSyncData(sync_data);
+
   icon_manager_->WriteData(
       std::move(app_id), std::make_unique<WebApplicationInfo>(web_app_info),
       base::BindOnce(&WebAppInstallFinalizer::OnIconsDataWritten,