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

Issue 1989283002: Upstream: Launch WebApkActivity from WebAPK. (Closed)

Created:
4 years, 7 months ago by Xi Han
Modified:
4 years, 6 months ago
Reviewers:
Marc Treib, pkotwicz, gone
CC:
chromium-reviews, Yaron
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Upstream: Launch WebApkActivity from WebAPK. This CL introduces WebApkActivity which extends WebappActivity to launch a WebAPK in a fullscreen activity without Chrome's UI. Besides, upstreams changes in WebappLaunchActivity, WebappActivity and WebappInfo. BUG=609122 Committed: https://crrev.com/f31b1ab8f6d904b935aee21968c183920554206d Cr-Commit-Position: refs/heads/master@{#396305}

Patch Set 1 : #

Total comments: 18

Patch Set 2 : pkotwicz@'s comments. #

Total comments: 12

Patch Set 3 : Nits. #

Total comments: 4

Patch Set 4 : Introduce WebApkActivity. #

Total comments: 15

Patch Set 5 : Update ActivityAssigner. #

Total comments: 12

Patch Set 6 : Clean up. #

Total comments: 1

Patch Set 7 : Nits. #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+467 lines, -116 lines) Patch
M chrome/android/BUILD.gn View 1 2 3 2 chunks +2 lines, -0 lines 0 comments Download
M chrome/android/java/AndroidManifest.xml View 1 2 3 4 5 1 chunk +24 lines, -0 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/ShortcutHelper.java View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/webapps/ActivityAssigner.java View 1 2 3 4 5 6 13 chunks +80 lines, -35 lines 0 comments Download
A chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkActivity.java View 1 2 3 4 1 chunk +42 lines, -0 lines 0 comments Download
A chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkActivity0.java View 1 2 3 1 chunk +11 lines, -0 lines 0 comments Download
A chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkActivity1.java View 1 2 3 1 chunk +11 lines, -0 lines 0 comments Download
A chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkActivity2.java View 1 2 3 1 chunk +11 lines, -0 lines 0 comments Download
A chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkActivity3.java View 1 2 3 1 chunk +11 lines, -0 lines 0 comments Download
A chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkActivity4.java View 1 2 3 1 chunk +11 lines, -0 lines 0 comments Download
A chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkActivity5.java View 1 2 3 1 chunk +11 lines, -0 lines 0 comments Download
A chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkActivity6.java View 1 2 3 1 chunk +11 lines, -0 lines 0 comments Download
A chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkActivity7.java View 1 2 3 1 chunk +11 lines, -0 lines 0 comments Download
A chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkActivity8.java View 1 2 3 1 chunk +11 lines, -0 lines 0 comments Download
A chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkActivity9.java View 1 2 3 1 chunk +11 lines, -0 lines 0 comments Download
A + chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkManagedActivity.java View 1 2 3 4 5 2 chunks +12 lines, -13 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappActivity.java View 1 2 3 3 chunks +9 lines, -1 line 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappInfo.java View 1 2 3 7 chunks +17 lines, -4 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappLauncherActivity.java View 1 2 3 4 5 6 chunks +58 lines, -19 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappManagedActivity.java View 1 2 3 4 5 1 chunk +2 lines, -1 line 0 comments Download
M chrome/android/java_sources.gni View 1 2 3 1 chunk +13 lines, -0 lines 2 comments Download
M chrome/android/javatests/src/org/chromium/chrome/browser/webapps/ActivityAssignerTest.java View 1 2 3 4 5 7 chunks +51 lines, -19 lines 0 comments Download
M chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappInfoTest.java View 1 2 7 chunks +24 lines, -5 lines 0 comments Download
M chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappModeTest.java View 1 2 1 chunk +1 line, -1 line 0 comments Download
M chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/MainActivity.java View 1 2 3 4 5 4 chunks +20 lines, -18 lines 0 comments Download

Messages

Total messages: 51 (30 generated)
Xi Han
Hi Peter: could you please take a look? Thanks!
4 years, 7 months ago (2016-05-18 16:25:45 UTC) #7
pkotwicz
Note that I posted a CL which upstreams the removal of WebappInfo#copy() already
4 years, 7 months ago (2016-05-18 16:27:19 UTC) #8
Xi Han
PTAL, thanks!
4 years, 7 months ago (2016-05-18 18:56:28 UTC) #10
pkotwicz
Added some comments You are fast 💨 I started writing a similar CL for ExternalNavigationHandler ...
4 years, 7 months ago (2016-05-18 23:00:18 UTC) #11
Xi Han
PTAL, thanks! https://codereview.chromium.org/1989283002/diff/60001/chrome/android/java/src/org/chromium/chrome/browser/ShortcutHelper.java File chrome/android/java/src/org/chromium/chrome/browser/ShortcutHelper.java (right): https://codereview.chromium.org/1989283002/diff/60001/chrome/android/java/src/org/chromium/chrome/browser/ShortcutHelper.java#newcode75 chrome/android/java/src/org/chromium/chrome/browser/ShortcutHelper.java:75: "org.chromium.chrome.browser.webapp_webapk_package_name"; On 2016/05/18 23:00:17, pkotwicz wrote: > ...
4 years, 7 months ago (2016-05-19 18:31:50 UTC) #13
pkotwicz
LGTM with comments After you address my comments, I think that this CL is ready ...
4 years, 7 months ago (2016-05-19 21:36:36 UTC) #15
Xi Han
dfalcantara@chromium.org: Please review all changes, thanks! https://codereview.chromium.org/1989283002/diff/60001/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappActivity.java File chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappActivity.java (right): https://codereview.chromium.org/1989283002/diff/60001/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappActivity.java#newcode650 chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappActivity.java:650: return info.packageName().startsWith(WebApkConstants.WEBAPK_PACKAGE_PREFIX); On ...
4 years, 7 months ago (2016-05-20 14:45:41 UTC) #18
gone
https://codereview.chromium.org/1989283002/diff/160001/chrome/android/java/AndroidManifest.xml File chrome/android/java/AndroidManifest.xml (right): https://codereview.chromium.org/1989283002/diff/160001/chrome/android/java/AndroidManifest.xml#newcode385 chrome/android/java/AndroidManifest.xml:385: android:launchMode="singleTop" Why would this change to singleTop? Won't that ...
4 years, 7 months ago (2016-05-21 00:07:28 UTC) #19
Xi Han
Hi Dan, could you please take another look? Thanks. https://codereview.chromium.org/1989283002/diff/160001/chrome/android/java/AndroidManifest.xml File chrome/android/java/AndroidManifest.xml (right): https://codereview.chromium.org/1989283002/diff/160001/chrome/android/java/AndroidManifest.xml#newcode385 chrome/android/java/AndroidManifest.xml:385: ...
4 years, 7 months ago (2016-05-25 14:37:58 UTC) #22
gone
https://chromiumcodereview.appspot.com/1989283002/diff/260001/chrome/android/java/AndroidManifest.xml File chrome/android/java/AndroidManifest.xml (right): https://chromiumcodereview.appspot.com/1989283002/diff/260001/chrome/android/java/AndroidManifest.xml#newcode426 chrome/android/java/AndroidManifest.xml:426: <activity-alias android:name="com.google.android.apps.chrome.webapps.WebApkActivity" Don't need this alias. https://chromiumcodereview.appspot.com/1989283002/diff/260001/chrome/android/java/AndroidManifest.xml#newcode442 chrome/android/java/AndroidManifest.xml:442: <activity-alias ...
4 years, 7 months ago (2016-05-25 21:50:14 UTC) #28
Xi Han
Hi Dan, thank you for reviewing my CL. Don't know why there isn't a reply ...
4 years, 7 months ago (2016-05-26 17:23:36 UTC) #30
gone
Eh, I forgot about the state clearing behavior. Storing a second SharedPreferences file is preferable ...
4 years, 7 months ago (2016-05-26 18:20:36 UTC) #31
Xi Han
Hi Dan, could you please take another look? Thanks! https://chromiumcodereview.appspot.com/1989283002/diff/290001/chrome/android/java/AndroidManifest.xml File chrome/android/java/AndroidManifest.xml (right): https://chromiumcodereview.appspot.com/1989283002/diff/290001/chrome/android/java/AndroidManifest.xml#newcode414 chrome/android/java/AndroidManifest.xml:414: ...
4 years, 7 months ago (2016-05-26 20:54:58 UTC) #35
gone
lgtm You should take another pass at the CL description; you're missing a 's in ...
4 years, 7 months ago (2016-05-26 21:06:09 UTC) #36
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1989283002/370001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1989283002/370001
4 years, 7 months ago (2016-05-26 21:17:53 UTC) #42
commit-bot: I haz the power
Committed patchset #7 (id:370001)
4 years, 7 months ago (2016-05-26 22:53:53 UTC) #44
commit-bot: I haz the power
Patchset 7 (id:??) landed as https://crrev.com/f31b1ab8f6d904b935aee21968c183920554206d Cr-Commit-Position: refs/heads/master@{#396305}
4 years, 7 months ago (2016-05-26 22:55:10 UTC) #46
Marc Treib
https://codereview.chromium.org/1989283002/diff/370001/chrome/android/java_sources.gni File chrome/android/java_sources.gni (right): https://codereview.chromium.org/1989283002/diff/370001/chrome/android/java_sources.gni#newcode870 chrome/android/java_sources.gni:870: "java/src/org/chromium/chrome/browser/webapps/WebApkActivity.java", This line appears twice, which breaks the build ...
4 years, 6 months ago (2016-05-27 12:41:02 UTC) #48
Marc Treib
https://codereview.chromium.org/1989283002/diff/370001/chrome/android/java_sources.gni File chrome/android/java_sources.gni (right): https://codereview.chromium.org/1989283002/diff/370001/chrome/android/java_sources.gni#newcode870 chrome/android/java_sources.gni:870: "java/src/org/chromium/chrome/browser/webapps/WebApkActivity.java", On 2016/05/27 12:41:02, Marc Treib wrote: > This ...
4 years, 6 months ago (2016-05-27 12:42:20 UTC) #49
Xi Han
On 2016/05/27 12:42:20, Marc Treib wrote: > https://codereview.chromium.org/1989283002/diff/370001/chrome/android/java_sources.gni > File chrome/android/java_sources.gni (right): > > https://codereview.chromium.org/1989283002/diff/370001/chrome/android/java_sources.gni#newcode870 ...
4 years, 6 months ago (2016-05-27 13:13:40 UTC) #50
Marc Treib
4 years, 6 months ago (2016-05-27 13:16:56 UTC) #51
Message was sent while issue was closed.
On 2016/05/27 13:13:40, Xi Han wrote:
> On 2016/05/27 12:42:20, Marc Treib wrote:
> >
>
https://codereview.chromium.org/1989283002/diff/370001/chrome/android/java_so...
> > File chrome/android/java_sources.gni (right):
> > 
> >
>
https://codereview.chromium.org/1989283002/diff/370001/chrome/android/java_so...
> > chrome/android/java_sources.gni:870:
> > "java/src/org/chromium/chrome/browser/webapps/WebApkActivity.java",
> > On 2016/05/27 12:41:02, Marc Treib wrote:
> > > This line appears twice, which breaks the build for me. Not sure how this
> > passed
> > > the CQ, maybe the bots are somehow configured differently than my machine.
> In
> > > any case, the duplicate line should be removed.
> > 
> > ...ah, and it's just been fixed: https://codereview.chromium.org/2019633002
> > Carry on then :)
> 
> Thanks Marc for fixing it!

I didn't actually fix it, credit goes to toyoshim@ :)

Powered by Google App Engine
This is Rietveld 408576698