dpwas: Support display_override in webapp manifest

This change adds support for the display_override property to the
webapp manifest. This change proposes that we reuse the existing
DisplayMode enum. It adds the field to manifest.mojom, as well
as parsing of the field.

This change proposes that we ignore unknown entries, and that
the overrides are stored in order.

Explainer: https://github.com/WICG/display-override/blob/master/explainer.m
Design document: https://docs.google.com/document/d/1hEmbGVHMN38q1YTaaGccQ-Y5CHr7xIURYPRWXTuvZLo/edit?usp=sharing
I2P: https://groups.google.com/a/chromium.org/d/topic/blink-dev/WvIeZT8uSzw/discussion

Bug: 1092667
Change-Id: I772abbadf00f32cad2fbd62280ebcee7b4616702
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2274170
Commit-Queue: Mike Jackson <mjackson@microsoft.com>
Reviewed-by: Alan Cutter <alancutter@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Reviewed-by: Matt Giuca <mgiuca@chromium.org>
Reviewed-by: Alexey Baskakov <loyso@chromium.org>
Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Cr-Commit-Position: refs/heads/master@{#788625}

chrome/browser/web_applications/components/web_app_install_utils.cc

@@ -152,6 +152,9 @@ void UpdateWebAppInfoFromManifest(const blink::Manifest& manifest,
   if (manifest.display != DisplayMode::kUndefined)
     web_app_info->display_mode = manifest.display;
 
+  if (!manifest.display_override.empty())
+    web_app_info->display_override = manifest.display_override;
+
   // Create the WebApplicationInfo icons list *outside* of |web_app_info|, so
   // that we can decide later whether or not to replace the existing icons array
   // (conditionally on whether there were any that didn't have purpose ANY).

chrome/browser/web_applications/components/web_app_install_utils_unittest.cc

@@ -112,6 +112,7 @@ TEST(WebAppInstallUtils, UpdateWebAppInfoFromManifest) {
   EXPECT_EQ(AppUrl(), web_app_info.app_url);
   EXPECT_EQ(AppUrl().GetWithoutFilename(), web_app_info.scope);
   EXPECT_EQ(DisplayMode::kBrowser, web_app_info.display_mode);
+  EXPECT_TRUE(web_app_info.display_override.empty());
 
   // The icon info from |web_app_info| should be left as is, since the manifest
   // doesn't have any icon information.
@@ -133,10 +134,15 @@ TEST(WebAppInstallUtils, UpdateWebAppInfoFromManifest) {
   // Add an icon without purpose ANY (expect to be ignored).
   icon.purpose = {blink::Manifest::ImageResource::Purpose::MONOCHROME};
   manifest.icons.push_back(icon);
+  manifest.display_override.push_back(DisplayMode::kMinimalUi);
+  manifest.display_override.push_back(DisplayMode::kStandalone);
 
   UpdateWebAppInfoFromManifest(manifest, &web_app_info);
   EXPECT_EQ(base::UTF8ToUTF16(kAppTitle), web_app_info.title);
   EXPECT_EQ(DisplayMode::kMinimalUi, web_app_info.display_mode);
+  ASSERT_EQ(2u, web_app_info.display_override.size());
+  EXPECT_EQ(DisplayMode::kMinimalUi, web_app_info.display_override[0]);
+  EXPECT_EQ(DisplayMode::kStandalone, web_app_info.display_override[1]);
 
   EXPECT_EQ(2u, web_app_info.icon_infos.size());
   EXPECT_EQ(AppIcon2(), web_app_info.icon_infos[0].url);

chrome/common/web_application_info.h

@@ -124,6 +124,9 @@ struct WebApplicationInfo {
   // https://w3c.github.io/manifest/#display-modes
   blink::mojom::DisplayMode display_mode = blink::mojom::DisplayMode::kBrowser;
 
+  // App preference to control display fallback ordering
+  std::vector<blink::mojom::DisplayMode> display_override;
+
   // User preference as to whether the app should be opened in a window.
   // If false, the app will be opened in a tab.
   // If true, the app will be opened in a window, with minimal-ui buttons

third_party/blink/common/manifest/manifest.cc

@@ -42,6 +42,7 @@ Manifest::~Manifest() = default;
 bool Manifest::IsEmpty() const {
   return name.is_null() && short_name.is_null() && start_url.is_empty() &&
          display == blink::mojom::DisplayMode::kUndefined &&
+         display_override.empty() &&
          orientation == device::mojom::ScreenOrientationLockType::DEFAULT &&
          icons.empty() && shortcuts.empty() && !share_target.has_value() &&
          related_applications.empty() && file_handlers.empty() &&

third_party/blink/common/manifest/manifest_mojom_traits.cc

@@ -91,6 +91,9 @@ bool StructTraits<blink::mojom::ManifestDataView, ::blink::Manifest>::Read(
   if (!data.ReadDisplay(&out->display))
     return false;
 
+  if (!data.ReadDisplayOverride(&out->display_override))
+    return false;
+
   if (!data.ReadOrientation(&out->orientation))
     return false;
 

third_party/blink/public/common/manifest/manifest.h

@@ -173,6 +173,10 @@ struct BLINK_COMMON_EXPORT Manifest {
   // present.
   blink::mojom::DisplayMode display = blink::mojom::DisplayMode::kUndefined;
 
+  // Empty if the parsing failed, the field was not present, or all the
+  // values inside the JSON array were invalid.
+  std::vector<blink::mojom::DisplayMode> display_override;
+
   // Set to device::mojom::ScreenOrientationLockType::DEFAULT if the parsing
   // failed or the field was not present.
   device::mojom::ScreenOrientationLockType orientation =

third_party/blink/public/common/manifest/manifest_mojom_traits.h

@@ -71,6 +71,11 @@ struct BLINK_COMMON_EXPORT
     return manifest.display;
   }
 
+  static const std::vector<blink::mojom::DisplayMode> display_override(
+      const ::blink::Manifest& manifest) {
+    return manifest.display_override;
+  }
+
   static device::mojom::ScreenOrientationLockType orientation(
       const ::blink::Manifest& manifest) {
     return manifest.orientation;

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

@@ -24,6 +24,8 @@ struct Manifest {
 
   DisplayMode display;
 
+  array<DisplayMode> display_override;
+
   device.mojom.ScreenOrientationLockType orientation;
 
   array<ManifestImageResource> icons;

third_party/blink/renderer/modules/manifest/manifest_parser.cc

@@ -82,6 +82,7 @@ void ManifestParser::Parse() {
   manifest_->start_url = ParseStartURL(root_object.get());
   manifest_->scope = ParseScope(root_object.get(), manifest_->start_url);
   manifest_->display = ParseDisplay(root_object.get());
+  manifest_->display_override = ParseDisplayOverride(root_object.get());
   manifest_->orientation = ParseOrientation(root_object.get());
   manifest_->icons = ParseIcons(root_object.get());
 
@@ -306,6 +307,39 @@ blink::mojom::DisplayMode ManifestParser::ParseDisplay(
   return display_enum;
 }
 
+Vector<mojom::blink::DisplayMode> ManifestParser::ParseDisplayOverride(
+    const JSONObject* object) {
+  Vector<mojom::blink::DisplayMode> display_override;
+  if (!RuntimeEnabledFeatures::DisplayOverrideEnabled())
+    return display_override;
+
+  JSONValue* json_value = object->Get("display_override");
+  if (!json_value)
+    return display_override;
+
+  JSONArray* display_override_list = object->GetArray("display_override");
+  if (!display_override_list) {
+    AddErrorInfo("property 'display_override' ignored, type array expected.");
+    return display_override;
+  }
+
+  for (wtf_size_t i = 0; i < display_override_list->size(); ++i) {
+    String display_enum_string;
+    // AsString will return an empty string if a type error occurs,
+    // which will cause DisplayModeFromString to return kUndefined,
+    // resulting in this entry being ignored.
+    display_override_list->at(i)->AsString(&display_enum_string);
+    display_enum_string = display_enum_string.StripWhiteSpace();
+    mojom::blink::DisplayMode display_enum =
+        DisplayModeFromString(display_enum_string.Utf8());
+
+    if (display_enum != mojom::blink::DisplayMode::kUndefined)
+      display_override.push_back(display_enum);
+  }
+
+  return display_override;
+}
+
 device::mojom::blink::ScreenOrientationLockType
 ManifestParser::ParseOrientation(const JSONObject* object) {
   base::Optional<String> orientation = ParseString(object, "orientation", Trim);

third_party/blink/renderer/modules/manifest/manifest_parser.h

@@ -131,6 +131,13 @@ class MODULES_EXPORT ManifestParser {
   // parsing failed.
   blink::mojom::DisplayMode ParseDisplay(const JSONObject* object);
 
+  // Parses the 'display_override' field of the manifest.
+  // https://github.com/WICG/display-override/blob/master/explainer.md
+  // Returns a vector of the parsed DisplayMode if any, an empty vector if
+  // the field was not present or empty.
+  Vector<mojom::blink::DisplayMode> ParseDisplayOverride(
+      const JSONObject* object);
+
   // Parses the 'orientation' field of the manifest, as defined in:
   // https://w3c.github.io/manifest/#dfn-steps-for-processing-the-orientation-member
   // Returns the parsed device::mojom::blink::ScreenOrientationLockType if any,

third_party/blink/renderer/modules/manifest/manifest_parser_unittest.cc

@@ -507,7 +507,7 @@ TEST_F(ManifestParserTest, DisplayParseRules) {
     EXPECT_EQ(0u, GetErrorCount());
   }
 
-  // Accept 'fullscreen'.
+  // Accept 'standalone'.
   {
     auto& manifest = ParseManifest("{ \"display\": \"standalone\" }");
     EXPECT_EQ(manifest->display, blink::mojom::DisplayMode::kStandalone);
@@ -536,6 +536,133 @@ TEST_F(ManifestParserTest, DisplayParseRules) {
   }
 }
 
+TEST_F(ManifestParserTest, DisplayOverrideParseRules) {
+  // Smoke test: if no display_override, no value.
+  {
+    auto& manifest = ParseManifest("{ \"display_override\": [] }");
+    EXPECT_TRUE(manifest->display_override.IsEmpty());
+    EXPECT_EQ(0u, GetErrorCount());
+  }
+
+  // Smoke test: if not array, value will be ignored
+  {
+    auto& manifest = ParseManifest("{ \"display_override\": 23 }");
+    EXPECT_TRUE(manifest->display_override.IsEmpty());
+    EXPECT_EQ(1u, GetErrorCount());
+    EXPECT_EQ("property 'display_override' ignored, type array expected.",
+              errors()[0]);
+  }
+
+  // Smoke test: if array value is not a string, it will be ignored
+  {
+    auto& manifest = ParseManifest("{ \"display_override\": [ 23 ] }");
+    EXPECT_TRUE(manifest->display_override.IsEmpty());
+    EXPECT_EQ(0u, GetErrorCount());
+  }
+
+  // Smoke test: if array value is not not recognized, it will be ignored
+  {
+    auto& manifest = ParseManifest("{ \"display_override\": [ \"test\" ] }");
+    EXPECT_TRUE(manifest->display_override.IsEmpty());
+    EXPECT_EQ(0u, GetErrorCount());
+  }
+
+  // Case insensitive
+  {
+    auto& manifest = ParseManifest("{ \"display_override\": [ \"BROWSER\" ] }");
+    EXPECT_FALSE(manifest->display_override.IsEmpty());
+    EXPECT_EQ(manifest->display_override[0],
+              blink::mojom::DisplayMode::kBrowser);
+    EXPECT_FALSE(IsManifestEmpty(manifest));
+    EXPECT_EQ(0u, GetErrorCount());
+  }
+
+  // Trim whitespace
+  {
+    auto& manifest =
+        ParseManifest("{ \"display_override\": [ \" browser \" ] }");
+    EXPECT_FALSE(manifest->display_override.IsEmpty());
+    EXPECT_EQ(manifest->display_override[0],
+              blink::mojom::DisplayMode::kBrowser);
+    EXPECT_FALSE(IsManifestEmpty(manifest));
+    EXPECT_EQ(0u, GetErrorCount());
+  }
+
+  // Accept 'browser'
+  {
+    auto& manifest = ParseManifest("{ \"display_override\": [ \"browser\" ] }");
+    EXPECT_FALSE(manifest->display_override.IsEmpty());
+    EXPECT_EQ(manifest->display_override[0],
+              blink::mojom::DisplayMode::kBrowser);
+    EXPECT_FALSE(IsManifestEmpty(manifest));
+    EXPECT_EQ(0u, GetErrorCount());
+  }
+
+  // Accept 'browser', 'minimal-ui'
+  {
+    auto& manifest = ParseManifest(
+        "{ \"display_override\": [ \"browser\", \"minimal-ui\" ] }");
+    EXPECT_FALSE(manifest->display_override.IsEmpty());
+    EXPECT_EQ(manifest->display_override[0],
+              blink::mojom::DisplayMode::kBrowser);
+    EXPECT_EQ(manifest->display_override[1],
+              blink::mojom::DisplayMode::kMinimalUi);
+    EXPECT_FALSE(IsManifestEmpty(manifest));
+    EXPECT_EQ(0u, GetErrorCount());
+  }
+
+  // if array value is not not recognized, it will be ignored
+  // Accept 'browser', 'minimal-ui'
+  {
+    auto& manifest = ParseManifest(
+        "{ \"display_override\": [ 3, \"browser\", \"invalid-display\", "
+        "\"minimal-ui\" ] }");
+    EXPECT_FALSE(manifest->display_override.IsEmpty());
+    EXPECT_EQ(manifest->display_override[0],
+              blink::mojom::DisplayMode::kBrowser);
+    EXPECT_EQ(manifest->display_override[1],
+              blink::mojom::DisplayMode::kMinimalUi);
+    EXPECT_FALSE(IsManifestEmpty(manifest));
+    EXPECT_EQ(0u, GetErrorCount());
+  }
+
+  // validate both display and display-override fields are parsed
+  // if array value is not not recognized, it will be ignored
+  // Accept 'browser', 'minimal-ui', 'standalone'
+  {
+    auto& manifest = ParseManifest(
+        "{ \"display\": \"standalone\", \"display_override\": [ \"browser\", "
+        "\"minimal-ui\", \"standalone\" ] }");
+    EXPECT_EQ(manifest->display, blink::mojom::DisplayMode::kStandalone);
+    EXPECT_EQ(0u, GetErrorCount());
+    EXPECT_FALSE(manifest->display_override.IsEmpty());
+    EXPECT_EQ(manifest->display_override[0],
+              blink::mojom::DisplayMode::kBrowser);
+    EXPECT_EQ(manifest->display_override[1],
+              blink::mojom::DisplayMode::kMinimalUi);
+    EXPECT_EQ(manifest->display_override[2],
+              blink::mojom::DisplayMode::kStandalone);
+    EXPECT_FALSE(IsManifestEmpty(manifest));
+  }
+
+  // validate duplicate entries.
+  // Accept 'browser', 'minimal-ui', 'browser'
+  {
+    auto& manifest = ParseManifest(
+        "{ \"display_override\": [ \"browser\", \"minimal-ui\", "
+        "\"browser\" ] }");
+    EXPECT_FALSE(manifest->display_override.IsEmpty());
+    EXPECT_EQ(manifest->display_override[0],
+              blink::mojom::DisplayMode::kBrowser);
+    EXPECT_EQ(manifest->display_override[1],
+              blink::mojom::DisplayMode::kMinimalUi);
+    EXPECT_EQ(manifest->display_override[2],
+              blink::mojom::DisplayMode::kBrowser);
+    EXPECT_FALSE(IsManifestEmpty(manifest));
+    EXPECT_EQ(0u, GetErrorCount());
+  }
+}
+
 TEST_F(ManifestParserTest, OrientationParseRules) {
   // Smoke test.
   {

third_party/blink/renderer/platform/runtime_enabled_features.json5

@@ -647,6 +647,10 @@
       name: "DisplayCutoutAPI",
       settable_from_internals: true,
     },
+    {
+      name: "DisplayOverride",
+      status: "test",
+    },
     {
       name: "DocumentCookie",
     },