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

Issue 655877 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

BeforeInstallPromptEvent.userChoice breaks ability to use prompt()

Project Member Reported by mgiuca@chromium.org, Oct 14 2016

Issue description

Version: 56
OS: Android

What steps will reproduce the problem?
(1) Set up a PWA site with a service worker, manifest, etc.
(2) Use attached index.js.
(3) Turn on chrome://flags#bypass-app-banner-engagement-checks
(4) Open inspector.
(5) Visit site.

What is the expected output?
After 1 second, add to home screen banner is shown.

What do you see instead?
After 1 second, console error:
prompt() failed: InvalidStateError: The prompt() method may only be called once, following preventDefault().

The code in index.js is:

window.addEventListener('beforeinstallprompt', e => {
  e.preventDefault();
  window.setTimeout(() => {
    e.prompt()
        .then(() => console.log('prompt() succeeded'))
        .catch(e => console.error('prompt() failed: ' + e));
  }, 1000);
  e.userChoice.then(c => console.log('userChoice resolved: ' + c));
});

The line "e.userChoice.then(...)" is causing this. Comment out that line, and you get the expected behaviour. Note that this happens regardless of whether you call then() on userChoice (just accessing userChoice is sufficient).

Accessing the userChoice attribute before calling prompt() causes prompt() to fail. A work-around is to only access userChoice after prompt() resolves, but this shouldn't be the case.
 
index.js
432 bytes View Download
Cc: -dominickn@chromium.org
Owner: dominickn@chromium.org
Status: Started (was: Available)
I believe my in-flight app banners to Mojo CL fixes this: crrev.com/2393513004

Comment 2 by mgiuca@chromium.org, Oct 14 2016

#1 That's great. I think there should be a test for this also.
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 28 2016

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

commit 09f7b579849cc8725455d0468b03a0237a6b624d
Author: dominickn <dominickn@chromium.org>
Date: Fri Oct 28 01:44:15 2016

Convert app banners to use Mojo.

This CL converts the app banner system from Chrome IPC to Mojo and moves
all banner code from chrome/renderer to Blink. It also substantially
improves the layout test coverage for the BeforeInstallPromptEvent,
fixing various renderer crash bugs that are covered by the tests.

The key change is that the browser-side AppBannerManager makes a Mojo
request to Blink, which is intercepted by the WebLocalFrame and rerouted
to the AppBannerController. The AppBannerManager passes a bound
InterfaceRequest/Ptr in the Mojo request, which are used to create a
bound BeforeInstallPromptEvent. This ensures that when a
BeforeInstallPromptEvent is created, it already has a browser connection,
and does not need another asynchronous (and possibly racey) call to
establish full two-way communication with the browser process.

Several files in Blink's modules/app_banner directories that were solely
required for the AppBannerClient layer in chrome/renderer are deleted.
The existing layout tests are simplified by removing request_ids and
eliminating the AppBannerClient from ContentRendererClients. This
requires a dependency on content/public/common from
components/test_runner to allow a shim mojom::AppBannerService to be
injected.

dominickn@chromium.org is added to OWNERS for the Blink-side app
banner code.

BUG= 499704 , 655877 , 655902 

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

[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/chrome/browser/DEPS
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/chrome/browser/android/banners/app_banner_infobar_delegate_android.cc
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/chrome/browser/android/banners/app_banner_infobar_delegate_android.h
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/chrome/browser/android/banners/app_banner_manager_android.cc
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/chrome/browser/android/banners/app_banner_manager_android.h
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/chrome/browser/banners/app_banner_infobar_delegate_desktop.cc
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/chrome/browser/banners/app_banner_manager.cc
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/chrome/browser/banners/app_banner_manager.h
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/chrome/browser/banners/app_banner_manager_desktop.cc
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/chrome/browser/banners/app_banner_manager_desktop.h
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/chrome/common/render_messages.h
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/chrome/renderer/BUILD.gn
[delete] https://crrev.com/ae4f0f021e5e13154580b2ae57a71cbb573930b2/chrome/renderer/banners/OWNERS
[delete] https://crrev.com/ae4f0f021e5e13154580b2ae57a71cbb573930b2/chrome/renderer/banners/app_banner_client.cc
[delete] https://crrev.com/ae4f0f021e5e13154580b2ae57a71cbb573930b2/chrome/renderer/banners/app_banner_client.h
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/chrome/renderer/chrome_content_renderer_client.cc
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/chrome/renderer/chrome_content_renderer_client.h
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/chrome/renderer/chrome_render_frame_observer.cc
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/chrome/renderer/chrome_render_frame_observer.h
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/components/test_runner/BUILD.gn
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/components/test_runner/DEPS
[delete] https://crrev.com/ae4f0f021e5e13154580b2ae57a71cbb573930b2/components/test_runner/app_banner_client.cc
[delete] https://crrev.com/ae4f0f021e5e13154580b2ae57a71cbb573930b2/components/test_runner/app_banner_client.h
[add] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/components/test_runner/app_banner_service.cc
[add] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/components/test_runner/app_banner_service.h
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/components/test_runner/test_interfaces.cc
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/components/test_runner/test_interfaces.h
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/components/test_runner/test_runner.cc
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/components/test_runner/test_runner.h
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/components/test_runner/test_runner_for_specific_view.cc
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/components/test_runner/test_runner_for_specific_view.h
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/components/test_runner/web_test_delegate.h
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/components/test_runner/web_test_interfaces.cc
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/components/test_runner/web_test_interfaces.h
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/content/public/renderer/content_renderer_client.cc
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/content/public/renderer/content_renderer_client.h
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/content/renderer/render_frame_impl.h
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/content/shell/renderer/layout_test/blink_test_runner.cc
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/content/shell/renderer/layout_test/blink_test_runner.h
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/content/shell/renderer/layout_test/layout_test_content_renderer_client.cc
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/content/shell/renderer/layout_test/layout_test_content_renderer_client.h
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/third_party/WebKit/LayoutTests/app_banner/app-banner-event-dispatch.html
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/third_party/WebKit/LayoutTests/app_banner/app-banner-event-prompt.html
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/third_party/WebKit/LayoutTests/app_banner/before-install-prompt-event-constructor.html
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/third_party/WebKit/LayoutTests/app_banner/testrunner-resolve-crash.html
[delete] https://crrev.com/ae4f0f021e5e13154580b2ae57a71cbb573930b2/third_party/WebKit/Source/modules/app_banner/AppBannerCallbacks.cpp
[delete] https://crrev.com/ae4f0f021e5e13154580b2ae57a71cbb573930b2/third_party/WebKit/Source/modules/app_banner/AppBannerCallbacks.h
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/third_party/WebKit/Source/modules/app_banner/AppBannerController.cpp
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/third_party/WebKit/Source/modules/app_banner/AppBannerController.h
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/third_party/WebKit/Source/modules/app_banner/AppBannerPromptResult.cpp
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/third_party/WebKit/Source/modules/app_banner/AppBannerPromptResult.h
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/third_party/WebKit/Source/modules/app_banner/BUILD.gn
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/third_party/WebKit/Source/modules/app_banner/BeforeInstallPromptEvent.cpp
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/third_party/WebKit/Source/modules/app_banner/BeforeInstallPromptEvent.h
[add] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/third_party/WebKit/Source/modules/app_banner/DEPS
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/third_party/WebKit/Source/modules/app_banner/OWNERS
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/third_party/WebKit/Source/web/WebLocalFrameImpl.cpp
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/third_party/WebKit/Source/web/WebLocalFrameImpl.h
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/third_party/WebKit/public/BUILD.gn
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/third_party/WebKit/public/platform/modules/app_banner/OWNERS
[delete] https://crrev.com/ae4f0f021e5e13154580b2ae57a71cbb573930b2/third_party/WebKit/public/platform/modules/app_banner/WebAppBannerClient.h
[delete] https://crrev.com/ae4f0f021e5e13154580b2ae57a71cbb573930b2/third_party/WebKit/public/platform/modules/app_banner/WebAppBannerPromptReply.h
[delete] https://crrev.com/ae4f0f021e5e13154580b2ae57a71cbb573930b2/third_party/WebKit/public/platform/modules/app_banner/WebAppBannerPromptResult.h
[add] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/third_party/WebKit/public/platform/modules/app_banner/app_banner.mojom
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/third_party/WebKit/public/web/WebFrameClient.h
[modify] https://crrev.com/09f7b579849cc8725455d0468b03a0237a6b624d/third_party/WebKit/public/web/WebLocalFrame.h

Status: Fixed (was: Started)

Sign in to add a comment