ChromeWebApkHost#canUseGooglePlayToInstallWebApk() being asynchronous is super duper confusing |
||||
Issue descriptionChromeWebApkHost#canUseGooglePlayToInstallWebApk() being asynchronous is super duper confusing e.g. Issue 696131 - We should make the check synchronous. We can check whether Google Play is new enough synchronously - In the unlikely case that the GServices flags have not propagated to the user we should fall back to creating a regular webapp. We should fall back after an attempt to create a WebAPK has failed
,
Feb 28 2017
My concern is if the value in the Shared preferences was set to false due to the first install failed, we won't update the value even though the flag is propagated in the future. I suggest to see the data of the Google Play install metrics first and figure out how many times the install fails due to the Service isn't available. Once the number is low enough to show the flags are propagated for most of the users, we then begin to remove the asynchronous call.
,
Feb 28 2017
Yaron's suggestion is to compute the value on startup each time that Chrome is launched but to only use the new value on the subsequent run of Chrome
,
Feb 28 2017
,
Feb 28 2017
Yes, recompute the value on startup definitely helps. Just want to confirm, does "compute the value" simply checks whether the Play is new enough? That means we always wait for an install success / failure to know exactly whether the Google Play service is available or not.
,
Feb 28 2017
My suggestion was proposed as a short term fix and is inline with how webapks were already enabled. We already have the case where we cache the value to a shared pref because we need to know whether webapks are enabled for native is loaded so it seems no less "bad" then current state of play Re: #1. It depends on which way you default. You can default to on or off. Realistically most user sessions won't involve installing a WebApk so the likelihood that the value is out of date is small
,
Mar 1 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bd0dac0340caf61da16475a45fa372199e3b8c00 commit bd0dac0340caf61da16475a45fa372199e3b8c00 Author: pkotwicz <pkotwicz@chromium.org> Date: Wed Mar 01 05:04:20 2017 Rename ShortcutHelper::FetchSplashScreenImage() parameters to reflect correct units ShortcutHelper::FetchSplashScreenImage() gets the "ideal size" and the "minimum size" in pixels not density independant pixels. BUG= 696132 Review-Url: https://codereview.chromium.org/2720403002 Cr-Commit-Position: refs/heads/master@{#453848} [modify] https://crrev.com/bd0dac0340caf61da16475a45fa372199e3b8c00/chrome/browser/android/shortcut_helper.h
,
Mar 1 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d0d459e8dda997152b63f3fee88a1bf35b323a20 commit d0d459e8dda997152b63f3fee88a1bf35b323a20 Author: pkotwicz <pkotwicz@chromium.org> Date: Wed Mar 01 22:55:10 2017 [WebAPKs]: Reduce the parameters of ShortcutHelper::AddToLauncherWithSkBitmap() This CL removes the |webapp_id| and |splash_image_callback| parameters from ShortcutHelper::AddToLauncherWithSkBitmap(). This CL adds the needed information to ShortcutInfo so that AddToLauncherWithSkBitmap() can create the callback itself. The goal of this CL is to enable AppBannerInfoBarDelegateAndroid::OnWebApkInstallFinished() to call ShortcutHelper::AddToLauncherWithSkBitmap() in a subsequent CL. BUG= 696132 Review-Url: https://codereview.chromium.org/2724723002 Cr-Commit-Position: refs/heads/master@{#454080} [modify] https://crrev.com/d0d459e8dda997152b63f3fee88a1bf35b323a20/chrome/browser/android/banners/app_banner_infobar_delegate_android.cc [modify] https://crrev.com/d0d459e8dda997152b63f3fee88a1bf35b323a20/chrome/browser/android/banners/app_banner_manager_android.cc [modify] https://crrev.com/d0d459e8dda997152b63f3fee88a1bf35b323a20/chrome/browser/android/banners/app_banner_manager_android.h [modify] https://crrev.com/d0d459e8dda997152b63f3fee88a1bf35b323a20/chrome/browser/android/shortcut_helper.cc [modify] https://crrev.com/d0d459e8dda997152b63f3fee88a1bf35b323a20/chrome/browser/android/shortcut_helper.h [modify] https://crrev.com/d0d459e8dda997152b63f3fee88a1bf35b323a20/chrome/browser/android/shortcut_info.cc [modify] https://crrev.com/d0d459e8dda997152b63f3fee88a1bf35b323a20/chrome/browser/android/shortcut_info.h [modify] https://crrev.com/d0d459e8dda997152b63f3fee88a1bf35b323a20/chrome/browser/android/webapps/add_to_homescreen_data_fetcher.cc [modify] https://crrev.com/d0d459e8dda997152b63f3fee88a1bf35b323a20/chrome/browser/android/webapps/add_to_homescreen_data_fetcher.h [modify] https://crrev.com/d0d459e8dda997152b63f3fee88a1bf35b323a20/chrome/browser/android/webapps/add_to_homescreen_manager.cc [modify] https://crrev.com/d0d459e8dda997152b63f3fee88a1bf35b323a20/chrome/browser/banners/app_banner_manager.cc [modify] https://crrev.com/d0d459e8dda997152b63f3fee88a1bf35b323a20/chrome/browser/banners/app_banner_manager.h
,
Mar 2 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2b338ea073b62cbd75b0a9971a3e126c81847f35 commit 2b338ea073b62cbd75b0a9971a3e126c81847f35 Author: pkotwicz <pkotwicz@chromium.org> Date: Thu Mar 02 08:20:41 2017 Add webapp homescreen shortcut when WebAPK install fails In the unlikely case that the WebAPK install fails, create a webapp homescreen shortcut instead. This way, the "Add to Homescreen" menu item in the app menu will "do something" for all users. BUG= 696132 Review-Url: https://codereview.chromium.org/2721203004 Cr-Commit-Position: refs/heads/master@{#454210} [modify] https://crrev.com/2b338ea073b62cbd75b0a9971a3e126c81847f35/chrome/android/java/src/org/chromium/chrome/browser/infobar/AppBannerInfoBarDelegateAndroid.java [modify] https://crrev.com/2b338ea073b62cbd75b0a9971a3e126c81847f35/chrome/browser/android/banners/app_banner_infobar_delegate_android.cc [modify] https://crrev.com/2b338ea073b62cbd75b0a9971a3e126c81847f35/chrome/browser/android/banners/app_banner_infobar_delegate_android.h
,
Mar 2 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/54d3ba6d6186cd35ef2b124d8c65eab7b2d7cb5e commit 54d3ba6d6186cd35ef2b124d8c65eab7b2d7cb5e Author: pkotwicz <pkotwicz@chromium.org> Date: Thu Mar 02 22:05:21 2017 [WebAPK]: Don't do fake install to determine whether Google Play supports WebAPK We currently do a fake WebAPK install in order to determine whether Google Play supports WebAPKs. With https://codereview.chromium.org/2721203004/ we add a webapp to the homescreen if the WebAPK install fails. This makes the fake WebAPK install no longer necessary BUG= 696132 Review-Url: https://codereview.chromium.org/2730653002 Cr-Commit-Position: refs/heads/master@{#454395} [modify] https://crrev.com/54d3ba6d6186cd35ef2b124d8c65eab7b2d7cb5e/chrome/android/java/src/org/chromium/chrome/browser/init/ProcessInitializationHandler.java [modify] https://crrev.com/54d3ba6d6186cd35ef2b124d8c65eab7b2d7cb5e/chrome/android/java/src/org/chromium/chrome/browser/webapps/ChromeWebApkHost.java
,
Mar 3 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fc4948593710fbddb8da850cb9867566a554837a commit fc4948593710fbddb8da850cb9867566a554837a Author: pkotwicz <pkotwicz@chromium.org> Date: Fri Mar 03 02:58:41 2017 WebAPKs: Pass callback of type WebApkInstallService::FinishCallback to ShortcutHelper WebApkInstallService::FinishCallback and WebApkInstaller::FinishCallback have the same signature. ShortcutHelper calls WebApkInstallService::InstallAsync() and hence should use the WebApkInstallService::FinishCallback type. BUG= 696132 Review-Url: https://codereview.chromium.org/2727363002 Cr-Original-Commit-Position: refs/heads/master@{#454425} Review-Url: https://codereview.chromium.org/2729993002 Cr-Commit-Position: refs/heads/master@{#454486} [modify] https://crrev.com/fc4948593710fbddb8da850cb9867566a554837a/chrome/browser/android/banners/app_banner_infobar_delegate_android.cc [modify] https://crrev.com/fc4948593710fbddb8da850cb9867566a554837a/chrome/browser/android/shortcut_helper.cc [modify] https://crrev.com/fc4948593710fbddb8da850cb9867566a554837a/chrome/browser/android/shortcut_helper.h
,
Mar 14 2017
,
Mar 14 2017
Issue 695060 has been merged into this issue.
,
Mar 14 2017
Issue 694791 has been merged into this issue.
,
Mar 14 2017
Issue 693555 has been merged into this issue.
,
Mar 14 2017
,
Mar 14 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/01bf47ad61ffef17d3a4e96ada6e2703ae67cf18 commit 01bf47ad61ffef17d3a4e96ada6e2703ae67cf18 Author: pkotwicz <pkotwicz@chromium.org> Date: Tue Mar 14 23:26:33 2017 [WebAPK] Refactor "unsigned sources" installation path This CL: - Refactors WebApkInstaller::StartInstallingDownloadedWebApk() and WebApkInstaller::StartUpdateUsingDownloadedWebApk() to not have a return value. - Makes both Google Play and "unsigned sources" installs use WebApkInstaller#notify(). - Combines the install and update methods in WebApkInstaller to get rid of some duplicate code. BUG= 696132 Review-Url: https://codereview.chromium.org/2746723002 Cr-Commit-Position: refs/heads/master@{#456888} [modify] https://crrev.com/01bf47ad61ffef17d3a4e96ada6e2703ae67cf18/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkInstaller.java [modify] https://crrev.com/01bf47ad61ffef17d3a4e96ada6e2703ae67cf18/chrome/browser/android/webapk/webapk_installer.cc [modify] https://crrev.com/01bf47ad61ffef17d3a4e96ada6e2703ae67cf18/chrome/browser/android/webapk/webapk_installer.h [modify] https://crrev.com/01bf47ad61ffef17d3a4e96ada6e2703ae67cf18/chrome/browser/android/webapk/webapk_installer_unittest.cc
,
Mar 15 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/67dd4e7e82a41fef1e90f489f4b5e91ea5179c66 commit 67dd4e7e82a41fef1e90f489f4b5e91ea5179c66 Author: pkotwicz <pkotwicz@chromium.org> Date: Wed Mar 15 20:39:04 2017 [Android:WebAPK] Don't add webapp to homescreen if WebAPK install times out part 1/3 Chrome considers a install as "failed" if Google Play does not finish the install within 3 minutes. It is possible for the install to take more time if Google Play is busy with another install. When an install fails we add a webapp shortcut to the homescreen. This CL changes the logic so that a webapp shortcut is not added to the homescreen if the WebAPK install times out. It would be confusing for the user to end up with two shortcuts with the same icon on their homescreen: - one for a WebAPK - one for a webapp BUG= 696132 Review-Url: https://codereview.chromium.org/2733543002 Cr-Commit-Position: refs/heads/master@{#457193} [modify] https://crrev.com/67dd4e7e82a41fef1e90f489f4b5e91ea5179c66/chrome/android/BUILD.gn [modify] https://crrev.com/67dd4e7e82a41fef1e90f489f4b5e91ea5179c66/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkInstaller.java [modify] https://crrev.com/67dd4e7e82a41fef1e90f489f4b5e91ea5179c66/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkUpdateManager.java [modify] https://crrev.com/67dd4e7e82a41fef1e90f489f4b5e91ea5179c66/chrome/android/java/strings/android_chrome_strings.grd [modify] https://crrev.com/67dd4e7e82a41fef1e90f489f4b5e91ea5179c66/chrome/browser/android/banners/app_banner_infobar_delegate_android.cc [modify] https://crrev.com/67dd4e7e82a41fef1e90f489f4b5e91ea5179c66/chrome/browser/android/banners/app_banner_infobar_delegate_android.h [modify] https://crrev.com/67dd4e7e82a41fef1e90f489f4b5e91ea5179c66/chrome/browser/android/webapk/webapk_install_service.cc [modify] https://crrev.com/67dd4e7e82a41fef1e90f489f4b5e91ea5179c66/chrome/browser/android/webapk/webapk_install_service.h [modify] https://crrev.com/67dd4e7e82a41fef1e90f489f4b5e91ea5179c66/chrome/browser/android/webapk/webapk_installer.cc [modify] https://crrev.com/67dd4e7e82a41fef1e90f489f4b5e91ea5179c66/chrome/browser/android/webapk/webapk_installer.h [modify] https://crrev.com/67dd4e7e82a41fef1e90f489f4b5e91ea5179c66/chrome/browser/android/webapk/webapk_installer_unittest.cc [modify] https://crrev.com/67dd4e7e82a41fef1e90f489f4b5e91ea5179c66/chrome/browser/android/webapk/webapk_update_manager.cc [modify] https://crrev.com/67dd4e7e82a41fef1e90f489f4b5e91ea5179c66/chrome/browser/android/webapk/webapk_update_manager.h
,
Mar 16 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/19d929ae378491116d7683764baa6365d1ddd50d commit 19d929ae378491116d7683764baa6365d1ddd50d Author: pkotwicz <pkotwicz@chromium.org> Date: Thu Mar 16 01:28:28 2017 [Android:WebAPK] Don't add webapp to homescreen if WebAPK install times out part 2/3 This CL: - Changes GooglePlayWebApkInstallDelegate#installAsync() to return a WebApkInstallResult in the callback. - Deletes unused GooglePlayWebApkInstallDelegate#canInstallWebApk(). BUG= 696132 Review-Url: https://codereview.chromium.org/2728053002 Cr-Commit-Position: refs/heads/master@{#457300} [modify] https://crrev.com/19d929ae378491116d7683764baa6365d1ddd50d/chrome/android/java/src/org/chromium/chrome/browser/webapps/GooglePlayWebApkInstallDelegate.java [modify] https://crrev.com/19d929ae378491116d7683764baa6365d1ddd50d/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkInstaller.java
,
Mar 16 2017
The following revision refers to this bug: https://chrome-internal.googlesource.com/clank/internal/apps/+/a97d9b77f1c767e4c8682ca8bb53adae6674d606 commit a97d9b77f1c767e4c8682ca8bb53adae6674d606 Author: Peter Kotwicz <pkotwicz@google.com> Date: Thu Mar 16 05:59:48 2017
,
Mar 16 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6da294d05e78dcdd1109ae8dc763ea0485c714af commit 6da294d05e78dcdd1109ae8dc763ea0485c714af Author: cblume <cblume@chromium.org> Date: Thu Mar 16 06:06:32 2017 Revert of [Android:WebAPK] Don't add webapp to homescreen if WebAPK install times out part 2/3 (patchset #2 id:20001 of https://codereview.chromium.org/2728053002/ ) Reason for revert: This is causing build failures on clang-clankium-tot-builder See: https://crbug.com/702086 https://uberchromegw.corp.google.com/i/internal.client.clank_tot/builders/clang-clankium-tot-builder/builds/43251 Original issue's description: > [Android:WebAPK] Don't add webapp to homescreen if WebAPK install times out part 2/3 > > This CL: > - Changes GooglePlayWebApkInstallDelegate#installAsync() to return a > WebApkInstallResult in the callback. > - Deletes unused GooglePlayWebApkInstallDelegate#canInstallWebApk(). > > BUG= 696132 > > Review-Url: https://codereview.chromium.org/2728053002 > Cr-Commit-Position: refs/heads/master@{#457300} > Committed: https://chromium.googlesource.com/chromium/src/+/19d929ae378491116d7683764baa6365d1ddd50d TBR=dominickn@chromium.org,pkotwicz@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= 696132 Review-Url: https://codereview.chromium.org/2757563002 Cr-Commit-Position: refs/heads/master@{#457361} [modify] https://crrev.com/6da294d05e78dcdd1109ae8dc763ea0485c714af/chrome/android/java/src/org/chromium/chrome/browser/webapps/GooglePlayWebApkInstallDelegate.java [modify] https://crrev.com/6da294d05e78dcdd1109ae8dc763ea0485c714af/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkInstaller.java
,
Mar 16 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0b8414c122248ea1db697651dee163f7517216dd commit 0b8414c122248ea1db697651dee163f7517216dd Author: clamy <clamy@chromium.org> Date: Thu Mar 16 12:11:53 2017 Reland of [Android:WebAPK] Don't add webapp to homescreen if WebAPK install times out part 2/3 (patchset #1 id:1 of https://codereview.chromium.org/2757563002/ ) Reason for revert: The CL reverted was part of a two-sided CL, and the downstream part landed just before this got reverted. So the Android ToT roller bot is broken again. Let's land this CL again, so that everything goes back to green. Original issue's description: > Revert of [Android:WebAPK] Don't add webapp to homescreen if WebAPK install times out part 2/3 (patchset #2 id:20001 of https://codereview.chromium.org/2728053002/ ) > > Reason for revert: > This is causing build failures on clang-clankium-tot-builder > See: > https://crbug.com/702086 > https://uberchromegw.corp.google.com/i/internal.client.clank_tot/builders/clang-clankium-tot-builder/builds/43251 > > Original issue's description: > > [Android:WebAPK] Don't add webapp to homescreen if WebAPK install times out part 2/3 > > > > This CL: > > - Changes GooglePlayWebApkInstallDelegate#installAsync() to return a > > WebApkInstallResult in the callback. > > - Deletes unused GooglePlayWebApkInstallDelegate#canInstallWebApk(). > > > > BUG= 696132 > > > > Review-Url: https://codereview.chromium.org/2728053002 > > Cr-Commit-Position: refs/heads/master@{#457300} > > Committed: https://chromium.googlesource.com/chromium/src/+/19d929ae378491116d7683764baa6365d1ddd50d > > TBR=dominickn@chromium.org,pkotwicz@chromium.org > # Skipping CQ checks because original CL landed less than 1 days ago. > NOPRESUBMIT=true > NOTREECHECKS=true > NOTRY=true > BUG= 696132 > > Review-Url: https://codereview.chromium.org/2757563002 > Cr-Commit-Position: refs/heads/master@{#457361} > Committed: https://chromium.googlesource.com/chromium/src/+/6da294d05e78dcdd1109ae8dc763ea0485c714af TBR=dominickn@chromium.org,pkotwicz@chromium.org,aelias@chromium.org,cblume@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= 696132 Review-Url: https://codereview.chromium.org/2757543003 Cr-Commit-Position: refs/heads/master@{#457402} [modify] https://crrev.com/0b8414c122248ea1db697651dee163f7517216dd/chrome/android/java/src/org/chromium/chrome/browser/webapps/GooglePlayWebApkInstallDelegate.java [modify] https://crrev.com/0b8414c122248ea1db697651dee163f7517216dd/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkInstaller.java
,
Mar 22 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5fe87344efe197796525404ffa44f5a0049a77e6 commit 5fe87344efe197796525404ffa44f5a0049a77e6 Author: Peter Kotwicz <pkotwicz@google.com> Date: Wed Mar 22 20:29:26 2017 [Android:WebAPK] Don't add webapp to homescreen if WebAPK install times out part 2/3 This CL: - Changes GooglePlayWebApkInstallDelegate#installAsync() to return a WebApkInstallResult in the callback. - Deletes unused GooglePlayWebApkInstallDelegate#canInstallWebApk(). BUG= 696132 Review-Url: https://codereview.chromium.org/2728053002 Cr-Commit-Position: refs/heads/master@{#457300} (cherry picked from commit 19d929ae378491116d7683764baa6365d1ddd50d) Review-Url: https://codereview.chromium.org/2773503002 . Cr-Commit-Position: refs/branch-heads/3029@{#369} Cr-Branched-From: 939b32ee5ba05c396eef3fd992822fcca9a2e262-refs/heads/master@{#454471} [modify] https://crrev.com/5fe87344efe197796525404ffa44f5a0049a77e6/chrome/android/java/src/org/chromium/chrome/browser/webapps/GooglePlayWebApkInstallDelegate.java [modify] https://crrev.com/5fe87344efe197796525404ffa44f5a0049a77e6/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkInstaller.java
,
Mar 22 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5fe87344efe197796525404ffa44f5a0049a77e6 commit 5fe87344efe197796525404ffa44f5a0049a77e6 Author: Peter Kotwicz <pkotwicz@google.com> Date: Wed Mar 22 20:29:26 2017 [Android:WebAPK] Don't add webapp to homescreen if WebAPK install times out part 2/3 This CL: - Changes GooglePlayWebApkInstallDelegate#installAsync() to return a WebApkInstallResult in the callback. - Deletes unused GooglePlayWebApkInstallDelegate#canInstallWebApk(). BUG= 696132 Review-Url: https://codereview.chromium.org/2728053002 Cr-Commit-Position: refs/heads/master@{#457300} (cherry picked from commit 19d929ae378491116d7683764baa6365d1ddd50d) Review-Url: https://codereview.chromium.org/2773503002 . Cr-Commit-Position: refs/branch-heads/3029@{#369} Cr-Branched-From: 939b32ee5ba05c396eef3fd992822fcca9a2e262-refs/heads/master@{#454471} [modify] https://crrev.com/5fe87344efe197796525404ffa44f5a0049a77e6/chrome/android/java/src/org/chromium/chrome/browser/webapps/GooglePlayWebApkInstallDelegate.java [modify] https://crrev.com/5fe87344efe197796525404ffa44f5a0049a77e6/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkInstaller.java |
||||
►
Sign in to add a comment |
||||
Comment 1 by pkotw...@chromium.org
, Feb 28 2017