After a discussion in a separate thread, it seems to me that the infinite loop problem (Point2 above) is caused by a crack in the BIP API, which is to re-fire the event after an user explicitly canceled the dialog.

As the API owner, it's your call whether to break this re-firing loop or to consume activation.

Re the other point (Point1 above): let's discuss the "no per-context tokens" aspect of UAv2 in the intent thread:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/nkTDR8AUlwM/xsPcojA5BAAJ

We should try and centralise discussion of this bug in this public forum, but to summarise: the API deliberately refires, and the protection against abuse is the existence of user activation. Prior to user activation v2, we were advised to not consume a gesture, and simply check for the existence of one, since user gestures would disappear after 1 second anyway. User activation v2 changes this from underneath us and any other API which was also implemented in a similar way, so it's important that each API which uses activation transiently also gets checked to ensure no bad changes in behaviour occur.

The BIP API seem to have gone through changes on this re-firing aspect already, so I trust your judgement on the design.  As a non-expert on BIP, I only suggested the "no loop" idea assuming (perhaps wrongly) that it's still open for changes.

---

Re impact on other APIs:

We have been working with many other APIs (fullscreen, vibration, password prompt, WebUSB, popups, ...), in the past ~8 months fixed/adjusted many things.  We clearly missed BIP within the long list of dependencies for UAv2, apologies for that, but we are unaware of any test failures for BIP+UAv2.

So far we haven't seen any other case where an APIs breaks an internal cycle through dependence on activation.  Our finch trial didn't suggest anything else either. 

Two other APIs have been mentioned as possible ones that could fail like BIP, our observation rules them out, correct us if we missed something:

[Fullscreen] We are pretty confident that it works fine with UAv2.  The extensive set of tests for fullscreen are all passing now except for a single wpt test failure (which is caused by the frame-hierarchy constraint of UAv2, like  Issue 805015 ).  Btw, fullscreen has no "cyclic event" idea.

[Permissions] We encountered a few permission test failures so far, all are fixed now except for two browser_tests but they are unrelated failures (caused through extension messaging APIs,  Issue 860509 ).  Is there a specific reason permission risks failing similarly?

Re permissions: there appears to be effort leading towards enforcing a user gesture requirement for requesting notifications permission (see https://chromium-review.googlesource.com/c/chromium/src/+/1165143). This currently uses a transient gesture since we only use the gesture bit for metrics purposes at the moment. I left a comment on the CL, but it may be worth following up directly with the team on their plans.

As for test coverage, that's definitely our bad. We had coverage of the user gesture flow in browser_tests, but not in blink_tests (because previously we were checking the requirement for the user gesture in the browser rather than in Blink - it was controlled by a Chrome-side feature switch and rolled out in stages to different platforms).

I've already landed some new tests in Blink which exercise a gesture, and we should probably add some more when we switch to consuming the gesture.

Thanks dominickn@ for the pointer to Permission discussion, I will post a comment there.

And for the tests, let me know if you need any details around UAv2 or the differences with current model.

Situation #2 seems like a bug but I'm not sure I understand problem #1. Do you mind elaborating?

#8 are you referring to the two issues I outlined in the original post?

> There are two separate issues here:
> 1. prompt() no longer requires that the "beforeinstallprompt" event itself ...
> 2. Because cancelling the dialog causes "beforeinstallprompt" to be fired again ...

What I meant by problem 1 is that UA tokens no longer exist in a given execution context, they are global. Which means if you have like this sequence diagram shows, two concurrent threads of execution:

   1             2                3
click -----------------------> timeout
       beforeinstallprompt

Under the old model, the user activation token is available to the click event handler, and the timeout handler that it spawned, but isn't available on the unrelated beforeinstallprompt handler.

Under the new model, the user activation token created by the user click is available to all code until consumed, which means the beforeinstallprompt handler can use it, even though the code in beforeinstallprompt wasn't triggered by a user gesture.

The more tricky detail (which I presented above) is that technically, these are equivalent, because a clever developer can go out of their way to make the user gesture from the click available to some code called by the beforeinstallprompt handler. That is mustaq@'s argument... that UAv1 doesn't offer any real security over UAv2 because a malicious site under UAv1 can plumb the tokens around wherever they want.

However, my counter argument is that it's much easier to get the bad behaviour by accident under UAv2, because you don't need to plumb the UA token around. It just happens automatically. So this isn't a security-based argument of "a bad site could create a bad user experience", but rather a user experience argument of "a good site could accidentally create a bad user experience."

dominickn@ and I met two days ago, to discuss relevant details of the BIP API. 
 The cyclic event firing (#2) is orthogonal to this problem as per dominickn@, and he prefers to stick to it for reasons I didn't fully see from my limited knowledge of the API's history.

If BIP wants to break the cycle through activation, consuming is the only possible solution with UAv2.  I see two possible ways to consume:
- consume when BIP.prompt() is called, or
- consume only if user cancels the dialog (since clicking "Cancel" seems to be only way to trigger BIP event cycle).

Re the impact of #1 (the token-less model of UAv2), I will post a reply to the intent thread shortly where it seems more appropriate.

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

commit 15dc5d1a5e2ffaaee10daab8ee0f7da1dc50caf0
Author: Dominick Ng <dominickn@chromium.org>
Date: Fri Aug 31 20:04:52 2018

Consume a transient user activation in beforeinstallpromptevent.prompt()

With UserActivationV2, user gestures no longer expire in 1 second. This
CL consumes the user activation when the method is called in order to
avoid multiple usages of the beforeinstallpromptevent.prompt() method
with only one activation.

This CL also fixes a test to trigger beforeinstallpromptevent.prompt()
on keyUp rather than keyPress. This avoids a race condition where the
user activation is generated *after* the consuming call to prompt(),
causing subsequent tests to fail as they assumed the gesture had been
consumed when it was actually being immediately regenerated.

BUG= 869756 

Change-Id: Ie9249c0933a5c8f263de4171ec2559cd4f0e80f8
Reviewed-on: https://chromium-review.googlesource.com/1196622
Reviewed-by: Mustaq Ahmed <mustaq@chromium.org>
Reviewed-by: Matt Giuca <mgiuca@chromium.org>
Commit-Queue: Dominick Ng <dominickn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588138}
[modify] https://crrev.com/15dc5d1a5e2ffaaee10daab8ee0f7da1dc50caf0/third_party/WebKit/LayoutTests/app_banner/app-banner-event-prompt.html
[modify] https://crrev.com/15dc5d1a5e2ffaaee10daab8ee0f7da1dc50caf0/third_party/blink/renderer/modules/app_banner/before_install_prompt_event.cc

Thanks dominickn@ for landing the fix.

mgiuca@: We can close the bug now, right?

Thanks Appreciate

I was going to merge this to M70 so I hadn't marked as fixed yet.

Based on #c14, marking as fixed with a merge request.

This CL has only just rolled out on Android and not yet on Chrome OS, which are the two relevant platforms. That's why I hadn't applied the merge request label yet.

Requesting merge of #11 to M70. Seems to be working without issues on Canary.

Your change meets the bar and is auto-approved for M70. Please go ahead and merge the CL to branch 3538 manually. Please contact milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), geohsu@(ChromeOS), abdulsyed@(Desktop)

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

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

commit a6f4a7812ee6feff9420139c7063a9656df34af9
Author: Dominick Ng <dominickn@chromium.org>
Date: Wed Sep 12 01:08:58 2018

Consume a transient user activation in beforeinstallpromptevent.prompt()

With UserActivationV2, user gestures no longer expire in 1 second. This
CL consumes the user activation when the method is called in order to
avoid multiple usages of the beforeinstallpromptevent.prompt() method
with only one activation.

This CL also fixes a test to trigger beforeinstallpromptevent.prompt()
on keyUp rather than keyPress. This avoids a race condition where the
user activation is generated *after* the consuming call to prompt(),
causing subsequent tests to fail as they assumed the gesture had been
consumed when it was actually being immediately regenerated.

BUG= 869756 

Change-Id: Ie9249c0933a5c8f263de4171ec2559cd4f0e80f8
Reviewed-on: https://chromium-review.googlesource.com/1196622
Reviewed-by: Mustaq Ahmed <mustaq@chromium.org>
Reviewed-by: Matt Giuca <mgiuca@chromium.org>
Commit-Queue: Dominick Ng <dominickn@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#588138}(cherry picked from commit 15dc5d1a5e2ffaaee10daab8ee0f7da1dc50caf0)
Reviewed-on: https://chromium-review.googlesource.com/1218305
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Cr-Commit-Position: refs/branch-heads/3538@{#307}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
[modify] https://crrev.com/a6f4a7812ee6feff9420139c7063a9656df34af9/third_party/WebKit/LayoutTests/app_banner/app-banner-event-prompt.html
[modify] https://crrev.com/a6f4a7812ee6feff9420139c7063a9656df34af9/third_party/blink/renderer/modules/app_banner/before_install_prompt_event.cc