Skip to content

Commit

Permalink
dpwas: deprecate existing display_overrides field in proto
Browse files Browse the repository at this point in the history
Feedback provided in a different CL (2375597: dpwas: Add
window-controls-overlay as a new display mode |
https://chromium-review.googlesource.com/c/chromium/src/+/2375597)
and in w3c/manifest#932 is that we need a
way to say new display modes are allowed in display_override but
not display.

The plan is to introduce a new enum EnhancedDisplayMode, and
display_override should be:
  sequence<DisplayModeType or EnhancedDisplayModeType>.

To support that, this CL deprecates the existing display_override field,
and renames it. Renaming here should be fine, as long as we don't use
JSON serialization. Once the new field has landed, we will remove
this field, and mark it reserved so that future users don't reuse
the field.

Explainer: https://github.com/WICG/display-override/blob/master/explainer.md
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
Manifest w3c PR: w3c/manifest#932

Bug: 1092667
Change-Id: Ifbc0160a88263d4994d138f99921371ceea2bcb8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2436417
Commit-Queue: Mike Jackson <mjackson@microsoft.com>
Reviewed-by: Alexey Baskakov <loyso@chromium.org>
Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Cr-Commit-Position: refs/heads/master@{#811932}
  • Loading branch information
mwjacksonmsft authored and Commit Bot committed Sep 30, 2020
1 parent 93941bd commit 3ce0075
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 5 deletions.
6 changes: 4 additions & 2 deletions chrome/browser/web_applications/proto/web_app.proto
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,10 @@ message WebAppProto {

optional uint32 background_color = 21;

// A list of display modes specified in app manifest.
repeated DisplayMode display_mode_override = 22;
// Deprecated 9/28/2020. A list of display modes specified in app manifest.
// TODO (crbug.com/1092667): Replace with RESERVED
repeated DisplayMode deprecated_display_mode_override = 22
[deprecated = true];

// A list of icon sizes we successfully downloaded to store on disk, for icons
// that are designed for masking (ie. IconPurpose::MASKABLE). See also:
Expand Down
7 changes: 4 additions & 3 deletions chrome/browser/web_applications/web_app_database.cc
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ std::unique_ptr<WebAppProto> WebAppDatabase::CreateWebAppProto(
}

for (const DisplayMode& display_mode : web_app.display_mode_override()) {
local_data->add_display_mode_override(
local_data->add_deprecated_display_mode_override(
ToWebAppProtoDisplayMode(display_mode));
}

Expand Down Expand Up @@ -404,8 +404,9 @@ std::unique_ptr<WebApp> WebAppDatabase::CreateWebApp(
web_app->SetDisplayMode(ToMojomDisplayMode(local_data.display_mode()));

std::vector<DisplayMode> display_mode_override;
for (int i = 0; i < local_data.display_mode_override_size(); i++) {
WebAppProto::DisplayMode display_mode = local_data.display_mode_override(i);
for (int i = 0; i < local_data.deprecated_display_mode_override_size(); i++) {
WebAppProto::DisplayMode display_mode =
local_data.deprecated_display_mode_override(i);
display_mode_override.push_back(ToMojomDisplayMode(display_mode));
}
web_app->SetDisplayModeOverride(std::move(display_mode_override));
Expand Down

0 comments on commit 3ce0075

Please sign in to comment.