desktop-pwas: Replace WebAppSpecifics LaunchContainer with DisplayMode

The proto used by WebAppSyncBridge and WebAppDatabase now contains
DisplayMode.

TBR=dominickn@chromium.org

Bug: 1009909
Change-Id: Ic720d4837f5b6fb15e4f025370bc8d3191a8d2ad
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1846634Reviewed-by: Eric Willigers <ericwilligers@chromium.org>
Reviewed-by: Alexey Baskakov <loyso@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Commit-Queue: Eric Willigers <ericwilligers@chromium.org>
Cr-Commit-Position: refs/heads/master@{#704081}

chrome/browser/web_applications/web_app_database.cc

@@ -14,7 +14,6 @@
 #include "components/sync/base/model_type.h"
 #include "components/sync/model/metadata_batch.h"
 #include "components/sync/model/model_error.h"
-#include "components/sync/protocol/web_app_specifics.pb.h"
 #include "third_party/blink/public/mojom/manifest/display_mode.mojom.h"
 
 namespace web_app {
@@ -102,11 +101,8 @@ std::unique_ptr<WebAppProto> WebAppDatabase::CreateWebAppProto(
 
   local_data->set_name(web_app.name());
 
-  DCHECK_NE(blink::mojom::DisplayMode::kUndefined, web_app.display_mode());
-  sync_data->set_launch_container(web_app.display_mode() ==
-                                          blink::mojom::DisplayMode::kStandalone
-                                      ? sync_pb::WebAppSpecifics::WINDOW
-                                      : sync_pb::WebAppSpecifics::TAB);
+  sync_data->set_display_mode(
+      ToWebAppSpecificsDisplayMode(web_app.display_mode()));
 
   DCHECK(web_app.sources_.any());
   local_data->mutable_sources()->set_system(web_app.sources_[Source::kSystem]);
@@ -189,14 +185,11 @@ std::unique_ptr<WebApp> WebAppDatabase::CreateWebApp(
   }
   web_app->SetName(local_data.name());
 
-  if (!sync_data.has_launch_container()) {
-    DLOG(ERROR) << "WebApp proto parse error: no launch_container field";
+  if (!sync_data.has_display_mode()) {
+    DLOG(ERROR) << "WebApp proto parse error: no display_mode field";
     return nullptr;
   }
-  web_app->SetDisplayMode(sync_data.launch_container() ==
-                                  sync_pb::WebAppSpecifics::WINDOW
-                              ? blink::mojom::DisplayMode::kStandalone
-                              : blink::mojom::DisplayMode::kBrowser);
+  web_app->SetDisplayMode(ToMojomDisplayMode(sync_data.display_mode()));
 
   if (!local_data.has_is_locally_installed()) {
     DLOG(ERROR) << "WebApp proto parse error: no is_locally_installed field";
@@ -340,4 +333,33 @@ std::unique_ptr<WebApp> WebAppDatabase::ParseWebApp(const AppId& app_id,
   return web_app;
 }
 
+blink::mojom::DisplayMode ToMojomDisplayMode(
+    ::sync_pb::WebAppSpecifics::DisplayMode display_mode) {
+  switch (display_mode) {
+    case ::sync_pb::WebAppSpecifics::kBrowser:
+      return blink::mojom::DisplayMode::kBrowser;
+    case ::sync_pb::WebAppSpecifics::kMinimalUi:
+      return blink::mojom::DisplayMode::kMinimalUi;
+    case ::sync_pb::WebAppSpecifics::kStandalone:
+      return blink::mojom::DisplayMode::kStandalone;
+  }
+}
+
+::sync_pb::WebAppSpecifics::DisplayMode ToWebAppSpecificsDisplayMode(
+    blink::mojom::DisplayMode display_mode) {
+  switch (display_mode) {
+    case blink::mojom::DisplayMode::kBrowser:
+      return ::sync_pb::WebAppSpecifics::kBrowser;
+    case blink::mojom::DisplayMode::kMinimalUi:
+      return ::sync_pb::WebAppSpecifics::kMinimalUi;
+    case blink::mojom::DisplayMode::kUndefined:
+      NOTREACHED();
+      FALLTHROUGH;
+    case blink::mojom::DisplayMode::kFullscreen:
+    case blink::mojom::DisplayMode::kStandalone:
+      // We do not persist kFullscreen - see crbug.com/850465.
+      return ::sync_pb::WebAppSpecifics::kStandalone;
+  }
+}
+
 }  // namespace web_app

chrome/browser/web_applications/web_app_database.h

@@ -15,6 +15,8 @@
 #include "chrome/browser/web_applications/components/web_app_helpers.h"
 #include "chrome/browser/web_applications/web_app_registrar.h"
 #include "components/sync/model/model_type_store.h"
+#include "components/sync/protocol/web_app_specifics.pb.h"
+#include "third_party/blink/public/mojom/manifest/display_mode.mojom-forward.h"
 
 namespace syncer {
 class ModelError;
@@ -92,6 +94,12 @@ class WebAppDatabase {
   DISALLOW_COPY_AND_ASSIGN(WebAppDatabase);
 };
 
+blink::mojom::DisplayMode ToMojomDisplayMode(
+    ::sync_pb::WebAppSpecifics::DisplayMode display_mode);
+
+::sync_pb::WebAppSpecifics::DisplayMode ToWebAppSpecificsDisplayMode(
+    blink::mojom::DisplayMode display_mode);
+
 }  // namespace web_app
 
 #endif  // CHROME_BROWSER_WEB_APPLICATIONS_WEB_APP_DATABASE_H_

chrome/browser/web_applications/web_app_sync_bridge.cc

@@ -40,10 +40,7 @@ std::unique_ptr<syncer::EntityData> CreateSyncEntityData(const WebApp& app) {
       entity_data->specifics.mutable_web_app();
   specifics->set_launch_url(app.launch_url().spec());
   specifics->set_name(app.name());
-  specifics->set_launch_container(app.display_mode() ==
-                                          blink::mojom::DisplayMode::kStandalone
-                                      ? sync_pb::WebAppSpecifics::WINDOW
-                                      : sync_pb::WebAppSpecifics::TAB);
+  specifics->set_display_mode(ToWebAppSpecificsDisplayMode(app.display_mode()));
   if (app.theme_color().has_value())
     specifics->set_theme_color(app.theme_color().value());
 

components/sync/protocol/proto_enum_conversions.cc

@@ -540,11 +540,13 @@ const char* ProtoEnumToString(
 }
 
 const char* ProtoEnumToString(
-    sync_pb::WebAppSpecifics::LaunchContainer launch_container) {
-  ASSERT_ENUM_BOUNDS(sync_pb::WebAppSpecifics, LaunchContainer, TAB, WINDOW);
-  switch (launch_container) {
-    ENUM_CASE(sync_pb::WebAppSpecifics, TAB);
-    ENUM_CASE(sync_pb::WebAppSpecifics, WINDOW);
+    sync_pb::WebAppSpecifics::DisplayMode display_mode) {
+  ASSERT_ENUM_BOUNDS(sync_pb::WebAppSpecifics, DisplayMode, kBrowser,
+                     kStandalone);
+  switch (display_mode) {
+    ENUM_CASE(sync_pb::WebAppSpecifics, kBrowser);
+    ENUM_CASE(sync_pb::WebAppSpecifics, kMinimalUi);
+    ENUM_CASE(sync_pb::WebAppSpecifics, kStandalone);
   }
   NOTREACHED();
   return "";

components/sync/protocol/proto_enum_conversions.h

@@ -111,7 +111,7 @@ const char* ProtoEnumToString(
     sync_pb::WalletMetadataSpecifics::Type wallet_metadata_type);
 
 const char* ProtoEnumToString(
-    sync_pb::WebAppSpecifics::LaunchContainer launch_container);
+    sync_pb::WebAppSpecifics::DisplayMode display_mode);
 
 const char* ProtoEnumToString(
     sync_pb::WifiConfigurationSpecificsData::SecurityType security_type);

components/sync/protocol/proto_visitors.h

@@ -1082,7 +1082,7 @@ VISIT_PROTO_FIELDS(const sync_pb::WalletSyncFlags& proto) {
 VISIT_PROTO_FIELDS(const sync_pb::WebAppSpecifics& proto) {
   VISIT(launch_url);
   VISIT(name);
-  VISIT_ENUM(launch_container);
+  VISIT_ENUM(display_mode);
   VISIT(theme_color);
 }
 

components/sync/protocol/web_app_specifics.proto

@@ -14,13 +14,18 @@ package sync_pb;
 // WebApp data. This is a synced part of
 // chrome/browser/web_applications/proto/web_app.proto data.
 message WebAppSpecifics {
-  enum LaunchContainer {
-    TAB = 1;
-    WINDOW = 2;
+  // This enum should be synced with
+  // third_party/blink/public/mojom/manifest/display_mode.mojom
+  enum DisplayMode {
+    // kUndefined is never serialized.
+    kBrowser = 1;
+    kMinimalUi = 2;
+    kStandalone = 3;
+    // kFullscreen is never serialized.
   }
 
   optional string launch_url = 1;
   optional string name = 2;
-  optional LaunchContainer launch_container = 3;
+  optional DisplayMode display_mode = 3;
   optional uint32 theme_color = 4;
 }

third_party/blink/public/mojom/manifest/display_mode.mojom

@@ -6,6 +6,7 @@ module blink.mojom;
 
 // This enum should be synced with
 // chrome/android/java/src/org/chromium/chrome/browser/webapps/WebDisplayMode.java
+// and components/sync/protocol/web_app_specifics.proto
 
 enum DisplayMode {
   kUndefined, // User for override setting (ie. not set).