Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(375)

Issue 2954933002: WebApk needs to be restarted to notice that host browser was uninstalled. (Closed)

Created:
3 years, 5 months ago by Xi Han
Modified:
3 years, 5 months ago
Reviewers:
pkotwicz, Yaron, hartmanng
CC:
chromium-reviews, zpeng+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

WebApk needs to be restarted to notice that host browser was uninstalled. We cache a static variable for the WebAPK's runtime host, and it won't updated when the runtime host is uninstalled. Therefore, the WebAPK won't launch until it is forced to stop and relaunch. In this CL, we add an additional check of whether the runtime host is installed. If not, continue the regular browser selection process. BUG=736036 Review-Url: https://codereview.chromium.org/2954933002 Cr-Commit-Position: refs/heads/master@{#483064} Committed: https://chromium.googlesource.com/chromium/src/+/5b7fc7601292363b4b02869601b4b2c3010107c8

Patch Set 1 #

Total comments: 2

Patch Set 2 : Rebase. #

Total comments: 2

Patch Set 3 : Move the logic. #

Total comments: 4

Patch Set 4 : Nits. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+49 lines, -7 lines) Patch
M chrome/android/webapk/shell_apk/junit/src/org/chromium/webapk/shell_apk/WebApkUtilsTest.java View 1 2 3 1 chunk +34 lines, -0 lines 0 comments Download
M chrome/android/webapk/shell_apk/shell_apk_version.gni View 1 1 chunk +1 line, -1 line 0 comments Download
M chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/WebApkUtils.java View 1 2 3 3 chunks +14 lines, -6 lines 0 comments Download

Messages

Total messages: 33 (20 generated)
Xi Han
3 years, 5 months ago (2017-06-26 15:00:42 UTC) #7
Xi Han
Hi Peter, could you please take a look? Thanks!
3 years, 5 months ago (2017-06-26 15:01:00 UTC) #8
hartmanng
https://codereview.chromium.org/2954933002/diff/80001/chrome/android/webapk/shell_apk/shell_apk_version.gni File chrome/android/webapk/shell_apk/shell_apk_version.gni (right): https://codereview.chromium.org/2954933002/diff/80001/chrome/android/webapk/shell_apk/shell_apk_version.gni#newcode9 chrome/android/webapk/shell_apk/shell_apk_version.gni:9: template_shell_apk_version = 9 drive-by nit: https://codereview.chromium.org/2948653002/ already incremented this ...
3 years, 5 months ago (2017-06-26 17:35:23 UTC) #10
Xi Han
PTAL, thanks! https://codereview.chromium.org/2954933002/diff/80001/chrome/android/webapk/shell_apk/shell_apk_version.gni File chrome/android/webapk/shell_apk/shell_apk_version.gni (right): https://codereview.chromium.org/2954933002/diff/80001/chrome/android/webapk/shell_apk/shell_apk_version.gni#newcode9 chrome/android/webapk/shell_apk/shell_apk_version.gni:9: template_shell_apk_version = 9 On 2017/06/26 17:35:22, hartmanng ...
3 years, 5 months ago (2017-06-26 17:45:56 UTC) #11
pkotwicz
https://codereview.chromium.org/2954933002/diff/100001/chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/MainActivity.java File chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/MainActivity.java (right): https://codereview.chromium.org/2954933002/diff/100001/chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/MainActivity.java#newcode102 chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/MainActivity.java:102: boolean isInstalled = isInstalled(getPackageManager(), runtimeHost); Can you fix this ...
3 years, 5 months ago (2017-06-27 01:14:27 UTC) #12
Xi Han
Hi Peter, PTAL, thanks! https://codereview.chromium.org/2954933002/diff/100001/chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/MainActivity.java File chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/MainActivity.java (right): https://codereview.chromium.org/2954933002/diff/100001/chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/MainActivity.java#newcode102 chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/MainActivity.java:102: boolean isInstalled = isInstalled(getPackageManager(), runtimeHost); ...
3 years, 5 months ago (2017-06-27 17:07:13 UTC) #14
pkotwicz
LGTM https://codereview.chromium.org/2954933002/diff/140001/chrome/android/webapk/shell_apk/junit/src/org/chromium/webapk/shell_apk/WebApkUtilsTest.java File chrome/android/webapk/shell_apk/junit/src/org/chromium/webapk/shell_apk/WebApkUtilsTest.java (right): https://codereview.chromium.org/2954933002/diff/140001/chrome/android/webapk/shell_apk/junit/src/org/chromium/webapk/shell_apk/WebApkUtilsTest.java#newcode235 chrome/android/webapk/shell_apk/junit/src/org/chromium/webapk/shell_apk/WebApkUtilsTest.java:235: private void uninstallBrowser(String packageName) { Can you move ...
3 years, 5 months ago (2017-06-28 15:37:23 UTC) #19
pkotwicz
LGTM
3 years, 5 months ago (2017-06-28 15:37:25 UTC) #20
Xi Han
Hi Yaron, could you please take a look? Thanks! https://codereview.chromium.org/2954933002/diff/140001/chrome/android/webapk/shell_apk/junit/src/org/chromium/webapk/shell_apk/WebApkUtilsTest.java File chrome/android/webapk/shell_apk/junit/src/org/chromium/webapk/shell_apk/WebApkUtilsTest.java (right): https://codereview.chromium.org/2954933002/diff/140001/chrome/android/webapk/shell_apk/junit/src/org/chromium/webapk/shell_apk/WebApkUtilsTest.java#newcode235 chrome/android/webapk/shell_apk/junit/src/org/chromium/webapk/shell_apk/WebApkUtilsTest.java:235: ...
3 years, 5 months ago (2017-06-28 15:55:35 UTC) #23
Yaron
lgtm
3 years, 5 months ago (2017-06-28 16:02:30 UTC) #24
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2954933002/170001
3 years, 5 months ago (2017-06-28 16:40:52 UTC) #27
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2954933002/170001
3 years, 5 months ago (2017-06-28 16:46:33 UTC) #30
commit-bot: I haz the power
3 years, 5 months ago (2017-06-28 18:17:54 UTC) #33
Message was sent while issue was closed.
Committed patchset #4 (id:170001) as
https://chromium.googlesource.com/chromium/src/+/5b7fc7601292363b4b02869601b4...

Powered by Google App Engine
This is Rietveld 408576698