Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2956993002:
Introduce a new WebAPK launch action. (Closed)
DescriptionIntroduce a new WebAPK launch action.
If a browser can handle action:
"com.google.android.apps.chrome.webapps.WebappManager.ACTION_START_WEBAPP",
we think that browser can host WebAPKs. In the future, we want to make this
action more generic, something like:
"org.webapk.ACTION_START_WEBAPK".
In this CL, we add a new intent filter for the WebappLauncherActivity. Once this
change propergate to all Chrome's channels, we can change the action in the
MainActivity of the shell apk to adopt the new action.
BUG=735970
NOPRESUBMIT=true
Review-Url: https://codereview.chromium.org/2956993002
Cr-Commit-Position: refs/heads/master@{#482980}
Committed: https://chromium.googlesource.com/chromium/src/+/56ccf932aa2976881d7da78c38dd64b34f719e4b
Patch Set 1 #
Total comments: 6
Patch Set 2 : Revert #Patch Set 3 #
Total comments: 2
Patch Set 4 : File a bug. #
Messages
Total messages: 23 (13 generated)
Description was changed from ========== Introduce a new WebAPK launch action. If a browser can handle action: "com.google.android.apps.chrome.webapps.WebappManager.ACTION_START_WEBAPP", we think that browser can host WebAPKs. In the future, we want to make this action more generic, something like: "org.chromium.webapk.ACTION_START_WEBAPK". In this CL, we add a new intent filter for the WebappLauncherActivity. Once this change propergate to all Chrome's channels, we can change the action in the MainActivity of the shell apk to adopt the new action. BUG=735970 ========== to ========== Introduce a new WebAPK launch action. If a browser can handle action: "com.google.android.apps.chrome.webapps.WebappManager.ACTION_START_WEBAPP", we think that browser can host WebAPKs. In the future, we want to make this action more generic, something like: "org.chromium.webapk.ACTION_START_WEBAPK". In this CL, we add a new intent filter for the WebappLauncherActivity. Once this change propergate to all Chrome's channels, we can change the action in the MainActivity of the shell apk to adopt the new action. BUG=735970 ==========
hanxi@chromium.org changed reviewers: + yfriedman@chromium.org
Hi Yaron, could you please take a look? Thanks!
https://codereview.chromium.org/2956993002/diff/1/chrome/android/java/Android... File chrome/android/java/AndroidManifest.xml (right): https://codereview.chromium.org/2956993002/diff/1/chrome/android/java/Android... chrome/android/java/AndroidManifest.xml:485: <action android:name="org.chromium.webapk.ACTION_START_WEBAPK" /> wdyt about omitting "chromium" this time? Or I take it you want to keep consistent with our package names? If so, I"m ok with it. https://codereview.chromium.org/2956993002/diff/1/chrome/android/webapk/shell... File chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/MainActivity.java (right): https://codereview.chromium.org/2956993002/diff/1/chrome/android/webapk/shell... chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/MainActivity.java:41: // Action for launching {@link WebappLauncherActivity}. TODO(hanxi): Replaces this string with We shouldn't update the shell apk version just for a comment change. I'd add NOPRESUBMIT=true to the CL description and revert that file
Description was changed from ========== Introduce a new WebAPK launch action. If a browser can handle action: "com.google.android.apps.chrome.webapps.WebappManager.ACTION_START_WEBAPP", we think that browser can host WebAPKs. In the future, we want to make this action more generic, something like: "org.chromium.webapk.ACTION_START_WEBAPK". In this CL, we add a new intent filter for the WebappLauncherActivity. Once this change propergate to all Chrome's channels, we can change the action in the MainActivity of the shell apk to adopt the new action. BUG=735970 ========== to ========== Introduce a new WebAPK launch action. If a browser can handle action: "com.google.android.apps.chrome.webapps.WebappManager.ACTION_START_WEBAPP", we think that browser can host WebAPKs. In the future, we want to make this action more generic, something like: "org.chromium.webapk.ACTION_START_WEBAPK". In this CL, we add a new intent filter for the WebappLauncherActivity. Once this change propergate to all Chrome's channels, we can change the action in the MainActivity of the shell apk to adopt the new action. BUG=735970 NOPRESUBMIT=true ==========
Patchset #2 (id:20001) has been deleted
Hi Yaron, PTAL, thanks! https://codereview.chromium.org/2956993002/diff/1/chrome/android/java/Android... File chrome/android/java/AndroidManifest.xml (right): https://codereview.chromium.org/2956993002/diff/1/chrome/android/java/Android... chrome/android/java/AndroidManifest.xml:485: <action android:name="org.chromium.webapk.ACTION_START_WEBAPK" /> On 2017/06/27 14:38:24, Yaron wrote: > wdyt about omitting "chromium" this time? Or I take it you want to keep > consistent with our package names? If so, I"m ok with it. I don't have a strong preference between "org.chromium.webapk" and "android.intent", and I am open to suggestions:) https://codereview.chromium.org/2956993002/diff/1/chrome/android/webapk/shell... File chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/MainActivity.java (right): https://codereview.chromium.org/2956993002/diff/1/chrome/android/webapk/shell... chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/MainActivity.java:41: // Action for launching {@link WebappLauncherActivity}. TODO(hanxi): Replaces this string with On 2017/06/27 14:38:24, Yaron wrote: > We shouldn't update the shell apk version just for a comment change. I'd add > NOPRESUBMIT=true to the CL description and revert that file Thanks, add the NOPRESUBMIT=true to the CL description and revert.
yfriedman@chromium.org changed reviewers: + dominickn@chromium.org
https://codereview.chromium.org/2956993002/diff/1/chrome/android/java/Android... File chrome/android/java/AndroidManifest.xml (right): https://codereview.chromium.org/2956993002/diff/1/chrome/android/java/Android... chrome/android/java/AndroidManifest.xml:485: <action android:name="org.chromium.webapk.ACTION_START_WEBAPK" /> On 2017/06/27 15:05:49, Xi Han wrote: > On 2017/06/27 14:38:24, Yaron wrote: > > wdyt about omitting "chromium" this time? Or I take it you want to keep > > consistent with our package names? If so, I"m ok with it. > > I don't have a strong preference between "org.chromium.webapk" and > "android.intent", and I am open to suggestions:) In the doc (#5), we selected org.webapk.ACTION_START_WEBAPK. Shall we go with that? I could also see us renaming this to ACTION_START_WEBAPP / ACTION_START_PWA but maybe it's ok to mention the specific implementation tech because it affects our expectations. +Dom for general installability thoughts. FTR I think org.webapk.ACTION_START_WEBAPK is fine, just want to give him a chance to chime in :)
Description was changed from ========== Introduce a new WebAPK launch action. If a browser can handle action: "com.google.android.apps.chrome.webapps.WebappManager.ACTION_START_WEBAPP", we think that browser can host WebAPKs. In the future, we want to make this action more generic, something like: "org.chromium.webapk.ACTION_START_WEBAPK". In this CL, we add a new intent filter for the WebappLauncherActivity. Once this change propergate to all Chrome's channels, we can change the action in the MainActivity of the shell apk to adopt the new action. BUG=735970 NOPRESUBMIT=true ========== to ========== Introduce a new WebAPK launch action. If a browser can handle action: "com.google.android.apps.chrome.webapps.WebappManager.ACTION_START_WEBAPP", we think that browser can host WebAPKs. In the future, we want to make this action more generic, something like: "org.webapk.ACTION_START_WEBAPK". In this CL, we add a new intent filter for the WebappLauncherActivity. Once this change propergate to all Chrome's channels, we can change the action in the MainActivity of the shell apk to adopt the new action. BUG=735970 NOPRESUBMIT=true ==========
https://codereview.chromium.org/2956993002/diff/1/chrome/android/java/Android... File chrome/android/java/AndroidManifest.xml (right): https://codereview.chromium.org/2956993002/diff/1/chrome/android/java/Android... chrome/android/java/AndroidManifest.xml:485: <action android:name="org.chromium.webapk.ACTION_START_WEBAPK" /> On 2017/06/27 15:19:41, Yaron wrote: > On 2017/06/27 15:05:49, Xi Han wrote: > > On 2017/06/27 14:38:24, Yaron wrote: > > > wdyt about omitting "chromium" this time? Or I take it you want to keep > > > consistent with our package names? If so, I"m ok with it. > > > > I don't have a strong preference between "org.chromium.webapk" and > > "android.intent", and I am open to suggestions:) > > In the doc (#5), we selected org.webapk.ACTION_START_WEBAPK. Shall we go with > that? I could also see us renaming this to ACTION_START_WEBAPP / > ACTION_START_PWA but maybe it's ok to mention the specific implementation tech > because it affects our expectations. +Dom for general installability thoughts. > FTR I think org.webapk.ACTION_START_WEBAPK is fine, just want to give him a > chance to chime in :) I am fine with "org.webapk.ACTION_START_WEBAPK" too. Let's see how Dom feels:)
Action name sgtm - it seems unlikely that we'll move away from the "webapk" terminology so org.webapk.START_WEBAPK seems appropriately general and accurate. https://codereview.chromium.org/2956993002/diff/60001/chrome/android/webapk/s... File chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/MainActivity.java (right): https://codereview.chromium.org/2956993002/diff/60001/chrome/android/webapk/s... chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/MainActivity.java:41: // Action for launching {@link WebappLauncherActivity}. TODO(hanxi): Replaces this string with Minor nit: put the TODO on a new line so it's easy to see (and possibly file a bug too :) )
lgtm
Hi Dom, PTAL, thanks! https://codereview.chromium.org/2956993002/diff/60001/chrome/android/webapk/s... File chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/MainActivity.java (right): https://codereview.chromium.org/2956993002/diff/60001/chrome/android/webapk/s... chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/MainActivity.java:41: // Action for launching {@link WebappLauncherActivity}. TODO(hanxi): Replaces this string with On 2017/06/28 00:02:11, dominickn wrote: > Minor nit: put the TODO on a new line so it's easy to see (and possibly file a > bug too :) ) Done.
The CQ bit was checked by hanxi@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by hanxi@chromium.org
The CQ bit was checked by hanxi@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from yfriedman@chromium.org Link to the patchset: https://codereview.chromium.org/2956993002/#ps80001 (title: "File a bug.")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 80001, "attempt_start_ts": 1498657318418170,
"parent_rev": "8d0adb0a32053ae5be48a551c6f59abff6b89a2b", "commit_rev":
"56ccf932aa2976881d7da78c38dd64b34f719e4b"}
Message was sent while issue was closed.
Description was changed from ========== Introduce a new WebAPK launch action. If a browser can handle action: "com.google.android.apps.chrome.webapps.WebappManager.ACTION_START_WEBAPP", we think that browser can host WebAPKs. In the future, we want to make this action more generic, something like: "org.webapk.ACTION_START_WEBAPK". In this CL, we add a new intent filter for the WebappLauncherActivity. Once this change propergate to all Chrome's channels, we can change the action in the MainActivity of the shell apk to adopt the new action. BUG=735970 NOPRESUBMIT=true ========== to ========== Introduce a new WebAPK launch action. If a browser can handle action: "com.google.android.apps.chrome.webapps.WebappManager.ACTION_START_WEBAPP", we think that browser can host WebAPKs. In the future, we want to make this action more generic, something like: "org.webapk.ACTION_START_WEBAPK". In this CL, we add a new intent filter for the WebappLauncherActivity. Once this change propergate to all Chrome's channels, we can change the action in the MainActivity of the shell apk to adopt the new action. BUG=735970 NOPRESUBMIT=true Review-Url: https://codereview.chromium.org/2956993002 Cr-Commit-Position: refs/heads/master@{#482980} Committed: https://chromium.googlesource.com/chromium/src/+/56ccf932aa2976881d7da78c38dd... ==========
Message was sent while issue was closed.
Committed patchset #4 (id:80001) as https://chromium.googlesource.com/chromium/src/+/56ccf932aa2976881d7da78c38dd... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
