We need to pass an AppBannerManager weak pointer to AppBannerInfoBarDelegateAndroid in the native app case so that the userChoice promise can resolve. We should also test userChoice.
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/58ab87ff414c2799e2546aa20f27d53e6da9fa10 commit 58ab87ff414c2799e2546aa20f27d53e6da9fa10 Author: Dominick Ng <dominickn@chromium.org> Date: Tue Jan 30 19:06:07 2018 Consolidate app banner testing. This CL removes several different app banner testing files, and replaces them by calling the necessary JavaScript setup code in one test file. For instance, all tests will no longer have the "appinstalled" title overwriting, as that will only be active when the appropriate setup JavaScript is run. This allows future tests to be more easily added, and reduces collisions when manipulating the page title. BUG= 806923 Change-Id: Ia28837b8675a2e1fe329cb1df1be4798ddb03d02 Reviewed-on: https://chromium-review.googlesource.com/892182 Commit-Queue: Dominick Ng <dominickn@chromium.org> Reviewed-by: Matt Giuca <mgiuca@chromium.org> Cr-Commit-Position: refs/heads/master@{#532978} [modify] https://crrev.com/58ab87ff414c2799e2546aa20f27d53e6da9fa10/chrome/android/javatests/src/org/chromium/chrome/browser/banners/AppBannerManagerTest.java [modify] https://crrev.com/58ab87ff414c2799e2546aa20f27d53e6da9fa10/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/AddToHomescreenManagerTest.java [modify] https://crrev.com/58ab87ff414c2799e2546aa20f27d53e6da9fa10/chrome/browser/banners/app_banner_manager_browsertest.cc [delete] https://crrev.com/5bf01aa8441d96f848df9211cf45881b77e0cdd6/chrome/test/data/banners/appinstalled_test_page.html [delete] https://crrev.com/5bf01aa8441d96f848df9211cf45881b77e0cdd6/chrome/test/data/banners/beforeinstallprompt_test_page.html [delete] https://crrev.com/5bf01aa8441d96f848df9211cf45881b77e0cdd6/chrome/test/data/banners/cancel_test_page.html [modify] https://crrev.com/58ab87ff414c2799e2546aa20f27d53e6da9fa10/chrome/test/data/banners/main.js [modify] https://crrev.com/58ab87ff414c2799e2546aa20f27d53e6da9fa10/chrome/test/data/banners/no_manifest_test_page.html [modify] https://crrev.com/58ab87ff414c2799e2546aa20f27d53e6da9fa10/chrome/test/data/banners/no_sw_fetch_handler_test_page.html [modify] https://crrev.com/58ab87ff414c2799e2546aa20f27d53e6da9fa10/chrome/test/data/banners/play_app_test_page.html [modify] https://crrev.com/58ab87ff414c2799e2546aa20f27d53e6da9fa10/chrome/test/data/banners/play_app_url_test_page.html [delete] https://crrev.com/5bf01aa8441d96f848df9211cf45881b77e0cdd6/chrome/test/data/banners/prompt_in_handler_test_page.html [delete] https://crrev.com/5bf01aa8441d96f848df9211cf45881b77e0cdd6/chrome/test/data/banners/prompt_no_preventdefault_test_page.html [delete] https://crrev.com/5bf01aa8441d96f848df9211cf45881b77e0cdd6/chrome/test/data/banners/prompt_test_page.html
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/744fd44085be2905e5c85d9371a780f2add8f27a commit 744fd44085be2905e5c85d9371a780f2add8f27a Author: Trent Apted <tapted@chromium.org> Date: Wed Jan 31 02:53:39 2018 Revert "Consolidate app banner testing." This reverts commit 58ab87ff414c2799e2546aa20f27d53e6da9fa10. Reason for revert: suspected for lsan browser_tests and mus_browser_tests failures since https://ci.chromium.org/buildbot/chromium.memory/Linux%20Chromium%20OS%20ASan%20LSan%20Tests%20%281%29/25937 - AppBannerManagerBrowserTest.CheckOnLoadWithSufficientEngagementCancelBannerAfterPromptInHandler - AppBannerManagerBrowserTest.ExperimentalFlowWebAppBannerReprompt - AppBannerManagerBrowserTest.ExperimentalFlowWebAppBannerPromptNeedsGesture (and others in later runs)> Possibly timeout related. Original change's description: > Consolidate app banner testing. > > This CL removes several different app banner testing files, and replaces > them by calling the necessary JavaScript setup code in one test file. > For instance, all tests will no longer have the "appinstalled" title > overwriting, as that will only be active when the appropriate setup > JavaScript is run. > > This allows future tests to be more easily added, and reduces collisions > when manipulating the page title. > > BUG= 806923 > > Change-Id: Ia28837b8675a2e1fe329cb1df1be4798ddb03d02 > Reviewed-on: https://chromium-review.googlesource.com/892182 > Commit-Queue: Dominick Ng <dominickn@chromium.org> > Reviewed-by: Matt Giuca <mgiuca@chromium.org> > Cr-Commit-Position: refs/heads/master@{#532978} TBR=mgiuca@chromium.org,dominickn@chromium.org Change-Id: Ie030290324c689b3df616de9aa35b6e52e7584c5 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 806923 Reviewed-on: https://chromium-review.googlesource.com/895024 Reviewed-by: Trent Apted <tapted@chromium.org> Commit-Queue: Trent Apted <tapted@chromium.org> Cr-Commit-Position: refs/heads/master@{#533178} [modify] https://crrev.com/744fd44085be2905e5c85d9371a780f2add8f27a/chrome/android/javatests/src/org/chromium/chrome/browser/banners/AppBannerManagerTest.java [modify] https://crrev.com/744fd44085be2905e5c85d9371a780f2add8f27a/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/AddToHomescreenManagerTest.java [modify] https://crrev.com/744fd44085be2905e5c85d9371a780f2add8f27a/chrome/browser/banners/app_banner_manager_browsertest.cc [add] https://crrev.com/744fd44085be2905e5c85d9371a780f2add8f27a/chrome/test/data/banners/appinstalled_test_page.html [add] https://crrev.com/744fd44085be2905e5c85d9371a780f2add8f27a/chrome/test/data/banners/beforeinstallprompt_test_page.html [add] https://crrev.com/744fd44085be2905e5c85d9371a780f2add8f27a/chrome/test/data/banners/cancel_test_page.html [modify] https://crrev.com/744fd44085be2905e5c85d9371a780f2add8f27a/chrome/test/data/banners/main.js [modify] https://crrev.com/744fd44085be2905e5c85d9371a780f2add8f27a/chrome/test/data/banners/no_manifest_test_page.html [modify] https://crrev.com/744fd44085be2905e5c85d9371a780f2add8f27a/chrome/test/data/banners/no_sw_fetch_handler_test_page.html [modify] https://crrev.com/744fd44085be2905e5c85d9371a780f2add8f27a/chrome/test/data/banners/play_app_test_page.html [modify] https://crrev.com/744fd44085be2905e5c85d9371a780f2add8f27a/chrome/test/data/banners/play_app_url_test_page.html [add] https://crrev.com/744fd44085be2905e5c85d9371a780f2add8f27a/chrome/test/data/banners/prompt_in_handler_test_page.html [add] https://crrev.com/744fd44085be2905e5c85d9371a780f2add8f27a/chrome/test/data/banners/prompt_no_preventdefault_test_page.html [add] https://crrev.com/744fd44085be2905e5c85d9371a780f2add8f27a/chrome/test/data/banners/prompt_test_page.html
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e3acc3e8e9aef95641e6aa1a3898b61280fefc2c commit e3acc3e8e9aef95641e6aa1a3898b61280fefc2c Author: Dominick Ng <dominickn@chromium.org> Date: Thu Feb 01 18:35:16 2018 Reland "Consolidate app banner testing." This is a reland of 58ab87ff414c2799e2546aa20f27d53e6da9fa10. Original change's description: > Consolidate app banner testing. > > This CL removes several different app banner testing files, and replaces > them by calling the necessary JavaScript setup code in one test file. > For instance, all tests will no longer have the "appinstalled" title > overwriting, as that will only be active when the appropriate setup > JavaScript is run. > > This allows future tests to be more easily added, and reduces collisions > when manipulating the page title. > > BUG= 806923 > > Change-Id: Ia28837b8675a2e1fe329cb1df1be4798ddb03d02 > Reviewed-on: https://chromium-review.googlesource.com/892182 > Commit-Queue: Dominick Ng <dominickn@chromium.org> > Reviewed-by: Matt Giuca <mgiuca@chromium.org> > Cr-Commit-Position: refs/heads/master@{#532978} Bug: 806923 Change-Id: Ia85a5be0828ad0820a891f8cde9d9e492dc0b831 Reviewed-on: https://chromium-review.googlesource.com/896326 Reviewed-by: Matt Giuca <mgiuca@chromium.org> Reviewed-by: Ted Choc <tedchoc@chromium.org> Commit-Queue: Dominick Ng <dominickn@chromium.org> Cr-Commit-Position: refs/heads/master@{#533745} [modify] https://crrev.com/e3acc3e8e9aef95641e6aa1a3898b61280fefc2c/chrome/android/javatests/src/org/chromium/chrome/browser/banners/AppBannerManagerTest.java [modify] https://crrev.com/e3acc3e8e9aef95641e6aa1a3898b61280fefc2c/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/AddToHomescreenManagerTest.java [modify] https://crrev.com/e3acc3e8e9aef95641e6aa1a3898b61280fefc2c/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappNavigationTest.java [modify] https://crrev.com/e3acc3e8e9aef95641e6aa1a3898b61280fefc2c/chrome/browser/banners/app_banner_manager_browsertest.cc [modify] https://crrev.com/e3acc3e8e9aef95641e6aa1a3898b61280fefc2c/chrome/browser/installable/installable_manager_browsertest.cc [modify] https://crrev.com/e3acc3e8e9aef95641e6aa1a3898b61280fefc2c/chrome/test/android/javatests/src/org/chromium/chrome/test/util/browser/WebappTestPage.java [delete] https://crrev.com/89a7c021cccaddbb2094c4efe1faced461a76201/chrome/test/data/banners/appinstalled_test_page.html [delete] https://crrev.com/89a7c021cccaddbb2094c4efe1faced461a76201/chrome/test/data/banners/beforeinstallprompt_test_page.html [delete] https://crrev.com/89a7c021cccaddbb2094c4efe1faced461a76201/chrome/test/data/banners/cancel_test_page.html [modify] https://crrev.com/e3acc3e8e9aef95641e6aa1a3898b61280fefc2c/chrome/test/data/banners/main.js [modify] https://crrev.com/e3acc3e8e9aef95641e6aa1a3898b61280fefc2c/chrome/test/data/banners/manifest_no_service_worker.html [modify] https://crrev.com/e3acc3e8e9aef95641e6aa1a3898b61280fefc2c/chrome/test/data/banners/manifest_test_page.html [modify] https://crrev.com/e3acc3e8e9aef95641e6aa1a3898b61280fefc2c/chrome/test/data/banners/no_manifest_test_page.html [modify] https://crrev.com/e3acc3e8e9aef95641e6aa1a3898b61280fefc2c/chrome/test/data/banners/no_sw_fetch_handler_test_page.html [delete] https://crrev.com/89a7c021cccaddbb2094c4efe1faced461a76201/chrome/test/data/banners/play_app_test_page.html [delete] https://crrev.com/89a7c021cccaddbb2094c4efe1faced461a76201/chrome/test/data/banners/play_app_url_test_page.html [delete] https://crrev.com/89a7c021cccaddbb2094c4efe1faced461a76201/chrome/test/data/banners/prompt_in_handler_test_page.html [delete] https://crrev.com/89a7c021cccaddbb2094c4efe1faced461a76201/chrome/test/data/banners/prompt_no_preventdefault_test_page.html [delete] https://crrev.com/89a7c021cccaddbb2094c4efe1faced461a76201/chrome/test/data/banners/prompt_test_page.html
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/013d5d22bcf023d4a4367cd5b249c808e170b072 commit 013d5d22bcf023d4a4367cd5b249c808e170b072 Author: Dominick Ng <dominickn@chromium.org> Date: Fri Feb 02 06:24:58 2018 Resolve beforeinstallpromptevent.userChoice for native app banners. This CL ensures that native app banners resolve the userChoice promise on beforeinstallpromptevent. Specifically, the |weak_manager_| of AppBannerInfoBarDelegateAndroid was not being set in the constructor for native app banners. Now it is set correctly, which allows userChoice to resolve. BUG= 806923 Change-Id: I468c8dbc24e2451a3f6c044d8a2d95768d25347d Reviewed-on: https://chromium-review.googlesource.com/892183 Reviewed-by: Ted Choc <tedchoc@chromium.org> Reviewed-by: Matt Giuca <mgiuca@chromium.org> Commit-Queue: Dominick Ng <dominickn@chromium.org> Cr-Commit-Position: refs/heads/master@{#533975} [modify] https://crrev.com/013d5d22bcf023d4a4367cd5b249c808e170b072/chrome/android/javatests/src/org/chromium/chrome/browser/banners/AppBannerManagerTest.java [modify] https://crrev.com/013d5d22bcf023d4a4367cd5b249c808e170b072/chrome/browser/banners/app_banner_infobar_delegate_android.cc [modify] https://crrev.com/013d5d22bcf023d4a4367cd5b249c808e170b072/chrome/browser/banners/app_banner_infobar_delegate_android.h [modify] https://crrev.com/013d5d22bcf023d4a4367cd5b249c808e170b072/chrome/browser/banners/app_banner_manager_android.cc [modify] https://crrev.com/013d5d22bcf023d4a4367cd5b249c808e170b072/chrome/test/android/javatests/src/org/chromium/chrome/test/util/browser/WebappTestPage.java [modify] https://crrev.com/013d5d22bcf023d4a4367cd5b249c808e170b072/chrome/test/data/banners/main.js [modify] https://crrev.com/013d5d22bcf023d4a4367cd5b249c808e170b072/chrome/test/data/banners/manifest_no_service_worker.html
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/28da4b602e47b53f4e60715fb52f1053d09bead3 commit 28da4b602e47b53f4e60715fb52f1053d09bead3 Author: Dominick Ng <dominickn@chromium.org> Date: Wed Feb 07 21:34:15 2018 Clean up web app tests. This CL: * renames methods on WebappTestPage to be more consistent * removes duplicated code in AppBannerManageTest * removes all mention of manifest_test_page.html from files other than WebappTestPage.java BUG= 806923 Change-Id: I9a7e21fae6c0b3a856dbf57433cc849cabe24834 Reviewed-on: https://chromium-review.googlesource.com/905246 Commit-Queue: Dominick Ng <dominickn@chromium.org> Reviewed-by: Ted Choc <tedchoc@chromium.org> Cr-Commit-Position: refs/heads/master@{#535149} [modify] https://crrev.com/28da4b602e47b53f4e60715fb52f1053d09bead3/chrome/android/javatests/src/org/chromium/chrome/browser/banners/AppBannerManagerTest.java [modify] https://crrev.com/28da4b602e47b53f4e60715fb52f1053d09bead3/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/AddToHomescreenManagerTest.java [modify] https://crrev.com/28da4b602e47b53f4e60715fb52f1053d09bead3/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebApkUpdateDataFetcherTest.java [modify] https://crrev.com/28da4b602e47b53f4e60715fb52f1053d09bead3/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebApkUpdateManagerTest.java [modify] https://crrev.com/28da4b602e47b53f4e60715fb52f1053d09bead3/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappNavigationTest.java [modify] https://crrev.com/28da4b602e47b53f4e60715fb52f1053d09bead3/chrome/test/android/javatests/src/org/chromium/chrome/test/util/browser/WebappTestPage.java
Comment 1 by bugdroid1@chromium.org
, Jan 30 2018