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

Issue 811578 link

Starred by 2 users

Issue metadata

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



Sign in to add a comment

Implement modal app banner UI

Project Member Reported by dominickn@chromium.org, Feb 13 2018

Issue description

This work involves replacing the infobar app banner with a modal app banner using the existing add to homescreen dialog. The concrete work involves:

1. decoupling the app installation logic from the UI code in AppBannerInfoBarDelegateAndroid

2. decoupling AddToHomescreenDialog from AddToHomescreenManager

3. allowing native app details to be shown in AddToHomescreenDialog

4. allowing AppBannerManagerAndroid to create an AddToHomescreenDialog that hooks into the banner app installation logic (native or web)


 
Project Member

Comment 1 by bugdroid1@chromium.org, Feb 14 2018

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

commit 00cf77bd86dcc41b375289ceaf999398fc2349db
Author: Dominick Ng <dominickn@chromium.org>
Date: Wed Feb 14 08:05:38 2018

Decouple AddToHomescreenDialog and AddToHomescreenManager.

This CL removes the dependence of AddToHomescreenDialog on
AddToHomescreenManager. A delegate interface on AddToHomescreenDialog is
introduced to allow the dialog to communicate with its host on when the
user accepts or dismisses the add to home screen prompt. This means that
other hosts can embed the dialog, not just the AddToHomescreenManager
(which is updated in this CL to implement the interface).

This CL should have no functional changes. This work is a precursor to
using the AddToHomescreenDialog as a surface for modal app install
banners.

BUG= 811578 

Change-Id: I0fe176e8f6853af180591b9a94b2e89898be89bd
Reviewed-on: https://chromium-review.googlesource.com/915243
Commit-Queue: Dominick Ng <dominickn@chromium.org>
Reviewed-by: Peter Kotwicz <pkotwicz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#536675}
[modify] https://crrev.com/00cf77bd86dcc41b375289ceaf999398fc2349db/chrome/android/java/src/org/chromium/chrome/browser/webapps/AddToHomescreenDialog.java
[modify] https://crrev.com/00cf77bd86dcc41b375289ceaf999398fc2349db/chrome/android/java/src/org/chromium/chrome/browser/webapps/AddToHomescreenManager.java
[modify] https://crrev.com/00cf77bd86dcc41b375289ceaf999398fc2349db/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/AddToHomescreenDialogTest.java
[modify] https://crrev.com/00cf77bd86dcc41b375289ceaf999398fc2349db/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/AddToHomescreenManagerTest.java
[modify] https://crrev.com/00cf77bd86dcc41b375289ceaf999398fc2349db/chrome/browser/android/webapps/add_to_homescreen_manager.cc
[modify] https://crrev.com/00cf77bd86dcc41b375289ceaf999398fc2349db/chrome/browser/android/webapps/add_to_homescreen_manager.h

Project Member

Comment 2 by bugdroid1@chromium.org, Feb 20 2018

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

commit f2432a4c27abc086387cae586c3a0ad70ea06816
Author: Dominick Ng <dominickn@chromium.org>
Date: Tue Feb 20 22:37:16 2018

Rename members and variables in AddToHomescreenDialog.

This CL is a minor renaming CL with no functional changes.

This prepares for the introduction of a new use for the app layout:
app install banners for web and native apps.

BUG= 811578 

Change-Id: Iff4201d160cb93bb2881d24a2f24cadf7fcd94bc
Reviewed-on: https://chromium-review.googlesource.com/920125
Reviewed-by: Peter Kotwicz <pkotwicz@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Commit-Queue: Dominick Ng <dominickn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537913}
[modify] https://crrev.com/f2432a4c27abc086387cae586c3a0ad70ea06816/chrome/android/java/res/layout/add_to_homescreen_dialog.xml
[modify] https://crrev.com/f2432a4c27abc086387cae586c3a0ad70ea06816/chrome/android/java/src/org/chromium/chrome/browser/webapps/AddToHomescreenDialog.java
[modify] https://crrev.com/f2432a4c27abc086387cae586c3a0ad70ea06816/chrome/android/java/src/org/chromium/chrome/browser/webapps/AddToHomescreenManager.java
[modify] https://crrev.com/f2432a4c27abc086387cae586c3a0ad70ea06816/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/AddToHomescreenManagerTest.java
[modify] https://crrev.com/f2432a4c27abc086387cae586c3a0ad70ea06816/chrome/browser/android/webapps/add_to_homescreen_manager.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Feb 20 2018

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

commit 2220d25df211fef71485ef27356fa3fd856b8fad
Author: Dominick Ng <dominickn@chromium.org>
Date: Tue Feb 20 22:43:15 2018

Decouple app banner installation logic from UI on Android.

This CL extracts the UI-independent logic from
AppBannerInfoBarDelegateAndroid, and moves it to a new class,
AppBannerUiDelegateAndroid. This will allow a new modal UI surface to be
introduced for banners that uses the same app installation logic.

BUG= 811578 

Change-Id: I0998b0086783d4c78f25c966917c40adb9638f7f
Reviewed-on: https://chromium-review.googlesource.com/915442
Commit-Queue: Dominick Ng <dominickn@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Reviewed-by: Ben Wells <benwells@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537915}
[add] https://crrev.com/2220d25df211fef71485ef27356fa3fd856b8fad/chrome/android/java/src/org/chromium/chrome/browser/banners/AppBannerUiDelegateAndroid.java
[modify] https://crrev.com/2220d25df211fef71485ef27356fa3fd856b8fad/chrome/android/java/src/org/chromium/chrome/browser/infobar/AppBannerInfoBarDelegateAndroid.java
[modify] https://crrev.com/2220d25df211fef71485ef27356fa3fd856b8fad/chrome/android/java_sources.gni
[modify] https://crrev.com/2220d25df211fef71485ef27356fa3fd856b8fad/chrome/browser/BUILD.gn
[modify] https://crrev.com/2220d25df211fef71485ef27356fa3fd856b8fad/chrome/browser/banners/app_banner_infobar_delegate_android.cc
[modify] https://crrev.com/2220d25df211fef71485ef27356fa3fd856b8fad/chrome/browser/banners/app_banner_infobar_delegate_android.h
[modify] https://crrev.com/2220d25df211fef71485ef27356fa3fd856b8fad/chrome/browser/banners/app_banner_manager_android.cc
[add] https://crrev.com/2220d25df211fef71485ef27356fa3fd856b8fad/chrome/browser/banners/app_banner_ui_delegate_android.cc
[add] https://crrev.com/2220d25df211fef71485ef27356fa3fd856b8fad/chrome/browser/banners/app_banner_ui_delegate_android.h
[modify] https://crrev.com/2220d25df211fef71485ef27356fa3fd856b8fad/chrome/browser/ui/android/infobars/app_banner_infobar_android.cc
[modify] https://crrev.com/2220d25df211fef71485ef27356fa3fd856b8fad/chrome/browser/ui/android/infobars/app_banner_infobar_android.h

Project Member

Comment 4 by bugdroid1@chromium.org, Feb 21 2018

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

commit 93fa824c07ee59354ee318b7c41c13c235117b4e
Author: Dominick Ng <dominickn@chromium.org>
Date: Wed Feb 21 02:37:41 2018

Implement modal app banners on Android.

This CL shifts app banners from using infobars to using a modal dialog
on Android when the "ExperimentalAppBanners" feature is active. The
modal banner can only be triggered by developers explicitly listening
for the beforeinstallpromptevent, and calling prompt() on it with a user
gesture active.

Modal banners are implemented for sites that are progressive web apps,
as well as sites advertising a native app in the Play Store in their web
app manifest. Similar to the existing banner, tapping on the icon or the
title of a native app banner will open the Play Store to view the native
app details.

This UI change mirrors the behaviour for desktop PWAs, which will launch
with a modal installation banner. Tests will be added in a follow-up CL
to keep this change to a manageable size.

Screenshots of the new modal banners are viewable at
https://drive.google.com/drive/folders/1jBY65wt_UsdcJZY_dVuA4QMMJiNhrs4Y?usp=sharing
with a chromium.org account.

BUG= 811578 

Change-Id: I2614fc83ec666e43d216f7f48706c129922b9f11
Reviewed-on: https://chromium-review.googlesource.com/923769
Reviewed-by: Peter Kotwicz <pkotwicz@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Reviewed-by: Ben Wells <benwells@chromium.org>
Commit-Queue: Dominick Ng <dominickn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#538004}
[modify] https://crrev.com/93fa824c07ee59354ee318b7c41c13c235117b4e/chrome/android/java/res/layout/add_to_homescreen_dialog.xml
[modify] https://crrev.com/93fa824c07ee59354ee318b7c41c13c235117b4e/chrome/android/java/src/org/chromium/chrome/browser/banners/AppBannerUiDelegateAndroid.java
[modify] https://crrev.com/93fa824c07ee59354ee318b7c41c13c235117b4e/chrome/android/java/src/org/chromium/chrome/browser/webapps/AddToHomescreenDialog.java
[modify] https://crrev.com/93fa824c07ee59354ee318b7c41c13c235117b4e/chrome/android/java/src/org/chromium/chrome/browser/webapps/AddToHomescreenManager.java
[modify] https://crrev.com/93fa824c07ee59354ee318b7c41c13c235117b4e/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/AddToHomescreenDialogTest.java
[modify] https://crrev.com/93fa824c07ee59354ee318b7c41c13c235117b4e/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/AddToHomescreenManagerTest.java
[modify] https://crrev.com/93fa824c07ee59354ee318b7c41c13c235117b4e/chrome/browser/banners/app_banner_infobar_delegate_android.cc
[modify] https://crrev.com/93fa824c07ee59354ee318b7c41c13c235117b4e/chrome/browser/banners/app_banner_manager_android.cc
[modify] https://crrev.com/93fa824c07ee59354ee318b7c41c13c235117b4e/chrome/browser/banners/app_banner_manager_android.h
[modify] https://crrev.com/93fa824c07ee59354ee318b7c41c13c235117b4e/chrome/browser/banners/app_banner_ui_delegate_android.cc
[modify] https://crrev.com/93fa824c07ee59354ee318b7c41c13c235117b4e/chrome/browser/banners/app_banner_ui_delegate_android.h

Project Member

Comment 5 by bugdroid1@chromium.org, Feb 21 2018

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

commit a428757cace3ba6571abc23f41947fcc92c7415f
Author: Dominick Ng <dominickn@chromium.org>
Date: Wed Feb 21 23:32:44 2018

Remove the CheckInstallabilityForBannerOnLoad feature.

This CL replaces mentions of the CheckInstallabilityForBannerOnLoad
feature with ExperimentalAppBanners, consolidating the feature flags for
the new app banner flow.

Once the modal banner UI and ambient badging changes on Android land,
this CL has the additional effect of ensuring ambient badge checking
will start as soon as the load event dispatches, rather than when
sufficient engagement to trigger the banner is earned.

BUG=782120, 811578 

Change-Id: I86409406bec56012a62c831ad57b799a8bf45547
Reviewed-on: https://chromium-review.googlesource.com/928095
Reviewed-by: Ben Wells <benwells@chromium.org>
Commit-Queue: Dominick Ng <dominickn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#538253}
[modify] https://crrev.com/a428757cace3ba6571abc23f41947fcc92c7415f/chrome/browser/banners/app_banner_manager.cc
[modify] https://crrev.com/a428757cace3ba6571abc23f41947fcc92c7415f/chrome/browser/banners/app_banner_manager_browsertest.cc
[modify] https://crrev.com/a428757cace3ba6571abc23f41947fcc92c7415f/chrome/browser/engagement/site_engagement_service.h
[modify] https://crrev.com/a428757cace3ba6571abc23f41947fcc92c7415f/chrome/common/chrome_features.cc
[modify] https://crrev.com/a428757cace3ba6571abc23f41947fcc92c7415f/chrome/common/chrome_features.h

Project Member

Comment 6 by bugdroid1@chromium.org, Feb 22 2018

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

commit 4b5c7ab51b151604c2a9120c3d1a1ba065b055b4
Author: Dominick Ng <dominickn@chromium.org>
Date: Thu Feb 22 01:09:16 2018

Dismiss the installable ambient badge when a modal app install banner is triggered.

When the ExperimentalAppBanners feature is active, sites with an
installable app (PWA or native app) display an ambient infobar to
inform users of the app. This CL dismisses the ambient badge
infobar if the site shows a modal app banner, as the ambient
badge and app install banner both link to an add to home screen
action.

BUG=782120, 811578 

Change-Id: Ifb9d3c9f8db21f3a0fcd0e4e697a881c84f55570
Reviewed-on: https://chromium-review.googlesource.com/923470
Commit-Queue: Dominick Ng <dominickn@chromium.org>
Reviewed-by: Ben Wells <benwells@chromium.org>
Cr-Commit-Position: refs/heads/master@{#538292}
[modify] https://crrev.com/4b5c7ab51b151604c2a9120c3d1a1ba065b055b4/chrome/browser/banners/app_banner_manager_android.cc
[modify] https://crrev.com/4b5c7ab51b151604c2a9120c3d1a1ba065b055b4/chrome/browser/banners/app_banner_manager_android.h

Owner: dominickn@chromium.org
Status: Started (was: Available)
Project Member

Comment 8 by bugdroid1@chromium.org, Mar 6 2018

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

commit 84a3e240604499a6beacf3ab3b1adfd52da5253d
Author: Dominick Ng <dominickn@chromium.org>
Date: Tue Mar 06 04:04:35 2018

Fix some minor UI inconsistencies in modal native app banners.

This CL changes "Add" to "Install" when a modal native app banner is
triggered. It also ensures that:
 - the ambient badge is shown prior to the beforeinstallpromptevent
   being dispatched for native apps.
 - native app banners are subject to the engagement threshold when the
   ExperimentalAppBanners feature is active.

BUG= 811578 

Change-Id: I19c0367e3bc52736467d1ecfc69e3cb472b82170
Reviewed-on: https://chromium-review.googlesource.com/938686
Commit-Queue: Ben Wells <benwells@chromium.org>
Reviewed-by: Ben Wells <benwells@chromium.org>
Cr-Commit-Position: refs/heads/master@{#541040}
[modify] https://crrev.com/84a3e240604499a6beacf3ab3b1adfd52da5253d/chrome/android/java/src/org/chromium/chrome/browser/banners/AppBannerUiDelegateAndroid.java
[modify] https://crrev.com/84a3e240604499a6beacf3ab3b1adfd52da5253d/chrome/android/java/src/org/chromium/chrome/browser/webapps/AddToHomescreenDialog.java
[modify] https://crrev.com/84a3e240604499a6beacf3ab3b1adfd52da5253d/chrome/browser/banners/app_banner_manager_android.cc
[modify] https://crrev.com/84a3e240604499a6beacf3ab3b1adfd52da5253d/chrome/browser/banners/app_banner_manager_android.h

Project Member

Comment 9 by bugdroid1@chromium.org, Mar 12 2018

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

commit 425c5cf7d6b148e218d44089bbe20f395357412a
Author: Dominick Ng <dominickn@chromium.org>
Date: Mon Mar 12 01:10:57 2018

Add tests for modal app install banners on Android.

BUG= 811578 

Change-Id: I89e36faf196520ecc05dea024204792f54caea1b
Reviewed-on: https://chromium-review.googlesource.com/939063
Reviewed-by: Ben Wells <benwells@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Commit-Queue: Dominick Ng <dominickn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542415}
[modify] https://crrev.com/425c5cf7d6b148e218d44089bbe20f395357412a/chrome/android/java/src/org/chromium/chrome/browser/ChromeFeatureList.java
[modify] https://crrev.com/425c5cf7d6b148e218d44089bbe20f395357412a/chrome/android/java/src/org/chromium/chrome/browser/banners/AppBannerManager.java
[modify] https://crrev.com/425c5cf7d6b148e218d44089bbe20f395357412a/chrome/android/java/src/org/chromium/chrome/browser/banners/AppBannerUiDelegateAndroid.java
[modify] https://crrev.com/425c5cf7d6b148e218d44089bbe20f395357412a/chrome/android/javatests/src/org/chromium/chrome/browser/banners/AppBannerManagerTest.java
[modify] https://crrev.com/425c5cf7d6b148e218d44089bbe20f395357412a/chrome/browser/android/chrome_feature_list.cc
[modify] https://crrev.com/425c5cf7d6b148e218d44089bbe20f395357412a/chrome/browser/banners/app_banner_manager_android.cc
[modify] https://crrev.com/425c5cf7d6b148e218d44089bbe20f395357412a/chrome/browser/banners/app_banner_manager_android.h
[modify] https://crrev.com/425c5cf7d6b148e218d44089bbe20f395357412a/chrome/browser/banners/app_banner_ui_delegate_android.cc
[modify] https://crrev.com/425c5cf7d6b148e218d44089bbe20f395357412a/chrome/browser/banners/app_banner_ui_delegate_android.h
[modify] https://crrev.com/425c5cf7d6b148e218d44089bbe20f395357412a/chrome/test/data/banners/main.js

Project Member

Comment 10 by bugdroid1@chromium.org, Apr 5 2018

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

commit e81fab496202e46b7f3834d418520c7750014918
Author: Dominick Ng <dominickn@chromium.org>
Date: Thu Apr 05 08:36:39 2018

Enable ExperimentalAppBanners by default on Android.

BUG=782120, 811578 

Change-Id: I240ed465ddab68c363fdbbac99c86ccd3ffd0194
Reviewed-on: https://chromium-review.googlesource.com/996833
Reviewed-by: Ben Wells <benwells@chromium.org>
Commit-Queue: Dominick Ng <dominickn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548369}
[modify] https://crrev.com/e81fab496202e46b7f3834d418520c7750014918/chrome/android/javatests/src/org/chromium/chrome/browser/banners/AppBannerManagerTest.java
[modify] https://crrev.com/e81fab496202e46b7f3834d418520c7750014918/chrome/common/chrome_features.cc

Project Member

Comment 11 by bugdroid1@chromium.org, Apr 20 2018

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

commit 6f604f9fc31e56f447fa55fbbb30ccfe45f561eb
Author: Dominick Ng <dominickn@chromium.org>
Date: Fri Apr 20 01:41:59 2018

Disable ExperimentalAppBanners by default on Android.

This will be relanded to target M68.

BUG=782120, 811578 

Change-Id: I2f67f27e8aacc76c32efb3e3773e2482033b19fb
Reviewed-on: https://chromium-review.googlesource.com/1020560
Reviewed-by: Patti <patricialor@chromium.org>
Commit-Queue: Dominick Ng <dominickn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552226}
[modify] https://crrev.com/6f604f9fc31e56f447fa55fbbb30ccfe45f561eb/chrome/android/javatests/src/org/chromium/chrome/browser/banners/AppBannerManagerTest.java
[modify] https://crrev.com/6f604f9fc31e56f447fa55fbbb30ccfe45f561eb/chrome/common/chrome_features.cc

Project Member

Comment 12 by bugdroid1@chromium.org, Apr 24 2018

Labels: merge-merged-3396
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1c5e44652dd6cf27a0543ef02a43ea111741ccd4

commit 1c5e44652dd6cf27a0543ef02a43ea111741ccd4
Author: Patti <patricialor@chromium.org>
Date: Tue Apr 24 00:56:14 2018

Disable ExperimentalAppBanners by default on Android.

This will be relanded to target M68.

BUG=782120, 811578 
TBR=dominickn@chromium.org

(cherry picked from commit 6f604f9fc31e56f447fa55fbbb30ccfe45f561eb)

Change-Id: I2f67f27e8aacc76c32efb3e3773e2482033b19fb
Reviewed-on: https://chromium-review.googlesource.com/1020560
Reviewed-by: Patti <patricialor@chromium.org>
Commit-Queue: Dominick Ng <dominickn@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#552226}
Reviewed-on: https://chromium-review.googlesource.com/1025151
Cr-Commit-Position: refs/branch-heads/3396@{#249}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/1c5e44652dd6cf27a0543ef02a43ea111741ccd4/chrome/android/javatests/src/org/chromium/chrome/browser/banners/AppBannerManagerTest.java
[modify] https://crrev.com/1c5e44652dd6cf27a0543ef02a43ea111741ccd4/chrome/common/chrome_features.cc

Project Member

Comment 13 by bugdroid1@chromium.org, Apr 24 2018

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

commit d0553a3dabeb3692185282df8efd2b91ca1eda4f
Author: Dominick Ng <dominickn@chromium.org>
Date: Tue Apr 24 02:51:15 2018

Enable ExperimentalAppBanners by default on Android.

BUG=782120, 811578 

Change-Id: I24d1129b61bf16b800331e1c512191bce529f5b1
Reviewed-on: https://chromium-review.googlesource.com/1020620
Commit-Queue: Patti <patricialor@chromium.org>
Reviewed-by: Patti <patricialor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552979}
[modify] https://crrev.com/d0553a3dabeb3692185282df8efd2b91ca1eda4f/chrome/android/javatests/src/org/chromium/chrome/browser/banners/AppBannerManagerTest.java
[modify] https://crrev.com/d0553a3dabeb3692185282df8efd2b91ca1eda4f/chrome/common/chrome_features.cc

Status: Fixed (was: Started)

Sign in to add a comment