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

Issue 729919 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug

Blocking:
issue 728022
issue 729920



Sign in to add a comment

Chrome OS: Stop showing PWA install banner automatically

Project Member Reported by mgiuca@chromium.org, Jun 6 2017

Issue description

Chrome OS counterpart to Android  Issue 728022 .

Rather than automatically prompting to install when engagement is high enough, silently (to the user) deliver the beforeinstallprompt event and let the site decide whether to show the prompt or not.

On Android, this will be an experimental change, but on Chrome OS (where there isn't really an established ecosystem around this) let's just roll out the change.
 
Blocking: 729920
Cc: owe...@chromium.org mgiuca@chromium.org
Owner: benwells@chromium.org
Status: Started (was: Assigned)
Project Member

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

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

commit 8d9f82995bc634440aed9ec773861dc376b08e70
Author: benwells <benwells@chromium.org>
Date: Sat Jun 24 01:59:38 2017

Start banner pipeline immediately after navigating, if enough engagement

Previously if a site had already earned enough engagement and was
navigated to, the banner pipeline would not be triggered until further
engagement was earned. With this change it will trigger immediately.

This could result in slight changes of behavior if sites are holding on
to the beforeinstallprompt event and using it to enable certain parts
of the UI (for example a button).

This change also tweaks some conditions in tests for changes in the
underlying state machine.

BUG= 729919 

Review-Url: https://codereview.chromium.org/2957523003
Cr-Commit-Position: refs/heads/master@{#482115}

[modify] https://crrev.com/8d9f82995bc634440aed9ec773861dc376b08e70/chrome/browser/banners/app_banner_manager.cc
[modify] https://crrev.com/8d9f82995bc634440aed9ec773861dc376b08e70/chrome/browser/banners/app_banner_manager_browsertest.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Jul 7 2017

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

commit 18cea352aa57b1bd0e57e7e60c04cba80728542d
Author: benwells <benwells@chromium.org>
Date: Fri Jul 07 08:54:08 2017

Update app banner state machine to use more states.

This replaces one instance of the ACTIVE state with a new SENDING_EVENT
state, and removes a boolean and replaces it's true state with a new
SENDING_EVENT_GOT_EARLY_PROMPT state.

BUG= 729919 

Review-Url: https://codereview.chromium.org/2957063002
Cr-Commit-Position: refs/heads/master@{#484862}

[modify] https://crrev.com/18cea352aa57b1bd0e57e7e60c04cba80728542d/chrome/android/java/src/org/chromium/chrome/browser/banners/AppBannerManager.java
[modify] https://crrev.com/18cea352aa57b1bd0e57e7e60c04cba80728542d/chrome/android/javatests/src/org/chromium/chrome/browser/banners/AppBannerManagerTest.java
[modify] https://crrev.com/18cea352aa57b1bd0e57e7e60c04cba80728542d/chrome/browser/android/banners/app_banner_manager_android.cc
[modify] https://crrev.com/18cea352aa57b1bd0e57e7e60c04cba80728542d/chrome/browser/android/banners/app_banner_manager_android.h
[modify] https://crrev.com/18cea352aa57b1bd0e57e7e60c04cba80728542d/chrome/browser/banners/app_banner_manager.cc
[modify] https://crrev.com/18cea352aa57b1bd0e57e7e60c04cba80728542d/chrome/browser/banners/app_banner_manager.h
[modify] https://crrev.com/18cea352aa57b1bd0e57e7e60c04cba80728542d/chrome/browser/banners/app_banner_manager_browsertest.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Jul 12 2017

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

commit 28132e66abebde370e168fff9ff2fa157c979d55
Author: Ben Wells <benwells@chromium.org>
Date: Wed Jul 12 04:06:27 2017

Simplify App Banner browser tests.

More tests are going to be added soon that need to run differently. This
change extracts some repeated code into re-usable functions so these
future tests can use them.

A few is_particular_state functions are also removed (e.g. is_inactive)
and instead the state is just exposed and tested directly.

Bug:  729919 
Change-Id: Ia453f0017c0c127e40277d51931c5452381a663d
Reviewed-on: https://chromium-review.googlesource.com/566763
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Commit-Queue: Ben Wells <benwells@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485830}
[modify] https://crrev.com/28132e66abebde370e168fff9ff2fa157c979d55/chrome/browser/banners/app_banner_manager.cc
[modify] https://crrev.com/28132e66abebde370e168fff9ff2fa157c979d55/chrome/browser/banners/app_banner_manager.h
[modify] https://crrev.com/28132e66abebde370e168fff9ff2fa157c979d55/chrome/browser/banners/app_banner_manager_browsertest.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Jul 19 2017

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

commit 39dfb49cb8b1efcb0caae4ee3534ed0e0d2a0b39
Author: Ben Wells <benwells@chromium.org>
Date: Wed Jul 19 01:25:19 2017

Begin implementation of experimental banner flow.

This change updates the flow for showing the app banner when the
experimental app banner is enabled. Specifically, if the experiment flow
is enabled:
- by default app banners will not be shown, instead the web app needs to
call event.prompt
- preventDefault does not need to be called for e.prompt to work
- a gesture is required to call e.prompt

Bug:  729919 
Change-Id: I6e5443b663c6139d8fbb8c9121839143470cdd37
Reviewed-on: https://chromium-review.googlesource.com/569842
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Commit-Queue: Ben Wells <benwells@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487708}
[modify] https://crrev.com/39dfb49cb8b1efcb0caae4ee3534ed0e0d2a0b39/chrome/browser/android/banners/app_banner_manager_android.cc
[modify] https://crrev.com/39dfb49cb8b1efcb0caae4ee3534ed0e0d2a0b39/chrome/browser/android/banners/app_banner_manager_android.h
[modify] https://crrev.com/39dfb49cb8b1efcb0caae4ee3534ed0e0d2a0b39/chrome/browser/banners/app_banner_manager.cc
[modify] https://crrev.com/39dfb49cb8b1efcb0caae4ee3534ed0e0d2a0b39/chrome/browser/banners/app_banner_manager.h
[modify] https://crrev.com/39dfb49cb8b1efcb0caae4ee3534ed0e0d2a0b39/chrome/browser/banners/app_banner_manager_browsertest.cc
[modify] https://crrev.com/39dfb49cb8b1efcb0caae4ee3534ed0e0d2a0b39/chrome/browser/banners/app_banner_manager_desktop.cc
[modify] https://crrev.com/39dfb49cb8b1efcb0caae4ee3534ed0e0d2a0b39/chrome/browser/banners/app_banner_manager_desktop.h
[modify] https://crrev.com/39dfb49cb8b1efcb0caae4ee3534ed0e0d2a0b39/chrome/browser/installable/installable_logging.cc
[modify] https://crrev.com/39dfb49cb8b1efcb0caae4ee3534ed0e0d2a0b39/chrome/browser/installable/installable_logging.h
[modify] https://crrev.com/39dfb49cb8b1efcb0caae4ee3534ed0e0d2a0b39/chrome/browser/installable/installable_manager_browsertest.cc
[add] https://crrev.com/39dfb49cb8b1efcb0caae4ee3534ed0e0d2a0b39/chrome/test/data/banners/prompt_no_preventdefault_test_page.html
[modify] https://crrev.com/39dfb49cb8b1efcb0caae4ee3534ed0e0d2a0b39/tools/metrics/histograms/enums.xml

Components: UI>Browser>WebAppInstalls

Comment 8 by mgiuca@chromium.org, Mar 26 2018

Labels: M-67

Comment 9 by mgiuca@chromium.org, Mar 27 2018

Status: Fixed (was: Started)
I think this is done (verified on https://killer-marmot.appspot.com/web/).

Sign in to add a comment