WebApp: Share sync proto and enums between the sync and the embedder.

Reuse sync WebAppSpecifics in web_applications code.
This works much better than a comment with a requirement to keep protos/enums
in sync.

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

chrome/browser/web_applications/proto/BUILD.gn

@@ -5,7 +5,9 @@
 import("//third_party/protobuf/proto_library.gni")
 
 proto_library("proto") {
+  import_dirs = [ "//components/sync/protocol" ]
   sources = [
     "web_app.proto",
   ]
+  link_deps = [ "//components/sync/protocol" ]
 }

chrome/browser/web_applications/proto/web_app.proto

@@ -4,6 +4,8 @@
 
 syntax = "proto2";
 
+import "web_app_specifics.proto";
+
 option optimize_for = LITE_RUNTIME;
 
 package web_app;
@@ -26,29 +28,19 @@ message SourcesProto {
   required bool default = 5;
 }
 
-// WebApp class data. This should be a superset for WebAppSpecifics in
-// components/sync/protocol/web_app_specifics.proto
+// Full WebApp object data. 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 {
-  // Keep in sync with WebAppSpecifics::LaunchContainer.
-  enum LaunchContainer {
-    TAB = 1;
-    WINDOW = 2;
-  }
-
-  // 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.
-  required string launch_url = 1;
-  required string name = 2;
-  required LaunchContainer launch_container = 3;
-  optional uint32 theme_color = 4;
-
-  // Local data members, not to be synced:
-  optional string description = 5;
-  optional string scope = 6;
-
-  required SourcesProto sources = 7;
-  required bool is_locally_installed = 8;
-
+  // Synced data.
+  required sync_pb.WebAppSpecifics specifics = 1;
+
+  // Local data. Not to be synced.
+  //
+  optional string description = 2;
+  optional string scope = 3;
+  required SourcesProto sources = 4;
+  required bool is_locally_installed = 5;
   // A list of icon infos.
-  repeated WebAppIconInfoProto icons = 9;
+  repeated WebAppIconInfoProto icons = 6;
 }

chrome/browser/web_applications/web_app_database.cc

@@ -12,6 +12,7 @@
 #include "chrome/browser/web_applications/web_app_database_factory.h"
 #include "components/sync/base/model_type.h"
 #include "components/sync/model/model_error.h"
+#include "components/sync/protocol/web_app_specifics.pb.h"
 
 namespace web_app {
 
@@ -77,15 +78,17 @@ std::unique_ptr<WebAppProto> WebAppDatabase::CreateWebAppProto(
   DCHECK(!web_app.app_id().empty());
   DCHECK_EQ(web_app.app_id(), GenerateAppIdFromURL(launch_url));
 
-  proto->set_launch_url(launch_url.spec());
+  sync_pb::WebAppSpecifics* specifics = proto->mutable_specifics();
 
-  proto->set_name(web_app.name());
+  specifics->set_launch_url(launch_url.spec());
+
+  specifics->set_name(web_app.name());
 
   DCHECK_NE(LaunchContainer::kDefault, web_app.launch_container());
-  proto->set_launch_container(web_app.launch_container() ==
-                                      LaunchContainer::kWindow
-                                  ? WebAppProto::WINDOW
-                                  : WebAppProto::TAB);
+  specifics->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]);
@@ -102,7 +105,7 @@ std::unique_ptr<WebAppProto> WebAppDatabase::CreateWebAppProto(
   if (!web_app.scope().is_empty())
     proto->set_scope(web_app.scope().spec());
   if (web_app.theme_color().has_value())
-    proto->set_theme_color(web_app.theme_color().value());
+    specifics->set_theme_color(web_app.theme_color().value());
 
   for (const WebApp::IconInfo& icon : web_app.icons()) {
     WebAppIconInfoProto* icon_proto = proto->add_icons();
@@ -115,8 +118,15 @@ std::unique_ptr<WebAppProto> WebAppDatabase::CreateWebAppProto(
 
 // static
 std::unique_ptr<WebApp> WebAppDatabase::CreateWebApp(const WebAppProto& proto) {
+  if (!proto.has_specifics()) {
+    DLOG(ERROR) << "WebApp proto parse error: no specifics field";
+    return nullptr;
+  }
+
+  const sync_pb::WebAppSpecifics& specifics = proto.specifics();
+
   // AppId is a hash of launch_url. Read launch_url first:
-  GURL launch_url(proto.launch_url());
+  GURL launch_url(specifics.launch_url());
   if (launch_url.is_empty() || !launch_url.is_valid()) {
     DLOG(ERROR) << "WebApp proto launch_url parse error: "
                 << launch_url.possibly_invalid_spec();
@@ -146,17 +156,18 @@ std::unique_ptr<WebApp> WebAppDatabase::CreateWebApp(const WebAppProto& proto) {
   }
   web_app->sources_ = sources;
 
-  if (!proto.has_name()) {
+  if (!specifics.has_name()) {
     DLOG(ERROR) << "WebApp proto parse error: no name field";
     return nullptr;
   }
-  web_app->SetName(proto.name());
+  web_app->SetName(specifics.name());
 
-  if (!proto.has_launch_container()) {
+  if (!specifics.has_launch_container()) {
     DLOG(ERROR) << "WebApp proto parse error: no launch_container field";
     return nullptr;
   }
-  web_app->SetLaunchContainer(proto.launch_container() == WebAppProto::WINDOW
+  web_app->SetLaunchContainer(specifics.launch_container() ==
+                                      sync_pb::WebAppSpecifics::WINDOW
                                   ? LaunchContainer::kWindow
                                   : LaunchContainer::kTab);
 
@@ -180,8 +191,8 @@ std::unique_ptr<WebApp> WebAppDatabase::CreateWebApp(const WebAppProto& proto) {
     web_app->SetScope(scope);
   }
 
-  if (proto.has_theme_color())
-    web_app->SetThemeColor(proto.theme_color());
+  if (specifics.has_theme_color())
+    web_app->SetThemeColor(specifics.theme_color());
 
   WebApp::Icons icons;
   for (int i = 0; i < proto.icons_size(); ++i) {

components/sync/protocol/web_app_specifics.proto

@@ -11,10 +11,9 @@ option optimize_for = LITE_RUNTIME;
 
 package sync_pb;
 
-// WebApp data. This should be a subset of WebAppProto in
-// chrome/browser/web_applications/proto/web_app.proto
+// WebApp data. This is a synced part of
+// chrome/browser/web_applications/proto/web_app.proto data.
 message WebAppSpecifics {
-  // Keep in sync with WebAppProto::LaunchContainer.
   enum LaunchContainer {
     TAB = 1;
     WINDOW = 2;