New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 788072 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Long OOO (go/where-is-mgiuca)
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Feature



Sign in to add a comment

Consolidate Desktop PWA flags so there is just one to turn on

Project Member Reported by mgiuca@chromium.org, Nov 23 2017

Issue description

Currently there are four relevant flags to Desktop PWAs:

1. #enable-app-banners / AppBanners: old app banners, enabled by default on CrOS for some time.
2. #enable-experimental-app-banners / ExperimentalAppBanners: new app banners. Implies AppBanners.
3. #bypass-app-banner-engagement-checks / --bypass-app-banner-engagement-checks: special debug mode.
4. #enable-desktop-pwa-windowing / DesktopPWAWindowing: new window UI.

#1 and #3 can stay as-is. Particularly, #3 should not be implied by any other flag because it is for debugging only.

We want one flag to tell people to turn on to try the full DPWA feature set. I propose that we unify #2 and #4 into a single flag: #enable-desktop-pwas / DesktopPWAs, which also implies AppBanners.

Alternatively (simpler), we could just have #4 imply #2.
 

Comment 1 by ortuno@chromium.org, Nov 23 2017

If I remember correctly we wanted a separate flag for #2 so that we could enable it on mobile incrementally, (I believe it changes the install prompt behavior?). I think that still applies, so unifying wouldn't be a good idea. If not, then +1 for unifying.

I'm OK with #4 implying #2 if unifying doesn't work out.

My understanding is that "PWA" is mainly a marketing name so we've been avoiding it in code (except that flag which I added before realizing this). That said, flags aren't meant to be permanent so it might be OK to add it in the flag.

Comment 2 by mgiuca@chromium.org, Nov 23 2017

#1 Good point, OK I will keep the flag #2 but make #4 imply #2.

Arguably, the flag is part of marketing since we tell people to activate it. It should use the branded name, not the internal name. (Though #enable-experimental-app-list says different.)
Project Member

Comment 3 by bugdroid1@chromium.org, Nov 24 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d487043f96cb39e2fc405cb7369257dcd432de4e

commit d487043f96cb39e2fc405cb7369257dcd432de4e
Author: Matt Giuca <mgiuca@chromium.org>
Date: Fri Nov 24 01:04:38 2017

Simplify Desktop PWA flags.

- desktop-pwa-windowing now implies experimental-app-banners.
- Renamed desktop-pwa-windowing to desktop-pwas. (This will de-activate
  the existing flag for anyone who has it turned on.)
- Updated flag descriptions in about:flags.
- Fixed code behind experimental-app-banners to use the function to
  check if it's turned on.

Note that the internal feature name DesktopPWAWindowing (i.e., what you
would use on the command line) has not changed, and is now a misnomer as
it also turns on experimental app banners.

Bug:  788072 
Tbr: msw@chromium.org
Change-Id: Ic39674c20313e632f427bb795557d70dce894599
Reviewed-on: https://chromium-review.googlesource.com/786357
Reviewed-by: Ben Wells <benwells@chromium.org>
Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Commit-Queue: Matt Giuca <mgiuca@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519038}
[modify] https://crrev.com/d487043f96cb39e2fc405cb7369257dcd432de4e/chrome/browser/about_flags.cc
[modify] https://crrev.com/d487043f96cb39e2fc405cb7369257dcd432de4e/chrome/browser/banners/app_banner_manager.cc
[modify] https://crrev.com/d487043f96cb39e2fc405cb7369257dcd432de4e/chrome/browser/extensions/extension_util.cc
[modify] https://crrev.com/d487043f96cb39e2fc405cb7369257dcd432de4e/chrome/browser/flag_descriptions.cc
[modify] https://crrev.com/d487043f96cb39e2fc405cb7369257dcd432de4e/chrome/browser/flag_descriptions.h
[modify] https://crrev.com/d487043f96cb39e2fc405cb7369257dcd432de4e/chrome/browser/ui/toolbar/app_menu_model.cc

Comment 4 by mgiuca@chromium.org, Nov 24 2017

Status: Fixed (was: Started)

Comment 5 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Comment 6 by dchan@chromium.org, Jan 23 2018

Status: Fixed (was: Archived)

Sign in to add a comment