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

Issue 716313 link

Starred by 1 user
Status: Fixed
Owner:
█ mgiuca@chromium.org
Long OOO (go/where-is-mgiuca)
Closed: Apr 2017
Cc:
dominickn@chromium.org
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug



Sign in to add a comment

appinstalled event triggers when user presses "Open" on WebAPK install flow

Project Member Reported by mgiuca@chromium.org, Apr 28 2017 
Chrome Version: 59
OS: Android

What steps will reproduce the problem?
(1) Enable chrome://flags#enable-improved-a2hs flag.
(2) Visit https://benfredwells.github.io/killer-marmot/web/index.html
(3) If necessary, click "Show the prompt after all".
(4) Should see PWA WebAPK install banner. Click "Add".
(5) After install, the banner should show an "Open" button. Click "Open".

What is the expected result?
Prints "Got appinstalled!!!" after clicking Add.

What happens instead?
Prints "Got appinstalled!!!" after clicking Add.
Prints "Got appinstalled!!!" again after clicking Open.

The Open button triggers the same code path as Add, and we fire the event on that code path.

Comment 1 by mgiuca@chromium.org, Apr 28 2017 

This same code path calls the BannerAccepted mojo method through to Blink, which then attempts to resolve the beforeinstallprompt method a second time. While this likely isn't user-visible, it's a symptom of the same problem.

Comment 2 by mgiuca@chromium.org, Apr 28 2017

Status: Started (was: Assigned)
CL: https://codereview.chromium.org/2848703005
Project Member

Comment 3 by bugdroid1@chromium.org, Apr 28 2017 

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

commit d398f548f1929bf2a8307d02bc69065cd7cdc035
Author: mgiuca <mgiuca@chromium.org>
Date: Fri Apr 28 05:30:27 2017

Fix appinstalled event firing when user opens a WebAPK from banner.

The "Open" button was calling SendBannerAccepted, which made the
assumption that the app was being installed. Removed this call and
documented the method's semantics.

BUG= 716313 

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

[modify] https://crrev.com/d398f548f1929bf2a8307d02bc69065cd7cdc035/chrome/browser/android/banners/app_banner_infobar_delegate_android.cc
[modify] https://crrev.com/d398f548f1929bf2a8307d02bc69065cd7cdc035/chrome/browser/android/banners/app_banner_infobar_delegate_android.h

Comment 4 by mgiuca@chromium.org, Apr 28 2017

Status: Fixed (was: Started)
Needs testing on Canary.

Comment 5 by mgiuca@chromium.org, May 2 2017

Labels: Merge-Request-59
Confirmed fix on Canary r468266. Requesting merge to M59 branch. Rationale: Regression in previously merged CL r466580 (3071@{#283}).
Project Member

Comment 6 by sheriffbot@chromium.org, May 2 2017

Labels: -Merge-Request-59 Hotlist-Merge-Approved Merge-Approved-59
Your change meets the bar and is auto-approved for M59. Please go ahead and merge the CL to branch 3071 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), gkihumba@(ChromeOS), Abdul Syed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 7 by bugdroid1@chromium.org, May 2 2017

Labels: -merge-approved-59 merge-merged-3071
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/68c2976343b37ebe1568ccc566fe8c72ab74a795

commit 68c2976343b37ebe1568ccc566fe8c72ab74a795
Author: Matt Giuca <mgiuca@chromium.org>
Date: Tue May 02 04:05:35 2017

Fix appinstalled event firing when user opens a WebAPK from banner.

The "Open" button was calling SendBannerAccepted, which made the
assumption that the app was being installed. Removed this call and
documented the method's semantics.

BUG= 716313 

Review-Url: https://codereview.chromium.org/2848703005
Cr-Commit-Position: refs/heads/master@{#467903}
(cherry picked from commit d398f548f1929bf2a8307d02bc69065cd7cdc035)

Review-Url: https://codereview.chromium.org/2851223003 .
Cr-Commit-Position: refs/branch-heads/3071@{#339}
Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641}

[modify] https://crrev.com/68c2976343b37ebe1568ccc566fe8c72ab74a795/chrome/browser/android/banners/app_banner_infobar_delegate_android.cc
[modify] https://crrev.com/68c2976343b37ebe1568ccc566fe8c72ab74a795/chrome/browser/android/banners/app_banner_infobar_delegate_android.h
►

Sign in to add a comment