New issue
Advanced search Search tips

Issue 913160 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 13
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug



Sign in to add a comment

WebAPK with share target requests update despite no manifest changes

Project Member Reported by pkotw...@chromium.org, Dec 8

Issue description

Repro steps:
1) In browser, navigate to https://webapk-test.appspot.com/generated.html?manifest=%7B%0A%20%20%22name%22%3A%20%22Long%20Name%22%2C%0A%20%20%22short_name%22%3A%20%22Short%20Name%22%2C%0A%20%20%22scope%22%3A%20%22%2F%22%2C%0A%20%20%22display%22%3A%20%22standalone%22%2C%0A%20%20%22orientation%22%3A%20%22%22%2C%0A%20%20%22theme_color%22%3A%20%22purple%22%2C%0A%20%20%22background_color%22%3A%20%22teal%22%2C%0A%20%20%22icons%22%3A%20%5B%0A%20%20%20%20%7B%0A%20%20%20%20%20%20%22src%22%3A%20%22%2Fstatic%2Ficons%2Fhexagon_256.png%22%2C%0A%20%20%20%20%20%20%22sizes%22%3A%20%22256x256%22%2C%0A%20%20%20%20%20%20%22type%22%3A%20%22image%2Fpng%22%0A%20%20%20%20%7D%0A%20%20%5D%2C%0A%20%20%22share_target%22%3A%7B%0A%20%20%20%20%22action%22%3A%22compose%2Ftweet%22%2C%0A%20%20%20%20%22params%22%3A%7B%0A%20%20%20%20%20%20%22title%22%3A%22title%22%2C%0A%20%20%20%20%20%20%22text%22%3A%22text%22%2C%0A%20%20%20%20%20%20%22url%22%3A%22url%22%0A%20%20%20%20%7D%0A%20%20%7D%0A%7D%0A&manifest_base_url=&start_base_url=&update=true&update_manifest=%7B%0A%20%20%22name%22%3A%20%22Long%20Name2%22%2C%0A%20%20%22short_name%22%3A%20%22Short%20Name2%22%2C%0A%20%20%22scope%22%3A%20%22%2F%22%2C%0A%20%20%22display%22%3A%20%22standalone%22%2C%0A%20%20%22orientation%22%3A%20%22%22%2C%0A%20%20%22theme_color%22%3A%20%22purple%22%2C%0A%20%20%22background_color%22%3A%20%22teal%22%2C%0A%20%20%22icons%22%3A%20%5B%0A%20%20%20%20%7B%0A%20%20%20%20%20%20%22src%22%3A%20%22%2Fstatic%2Ficons%2Fhexagon_256.png%22%2C%0A%20%20%20%20%20%20%22sizes%22%3A%20%22256x256%22%2C%0A%20%20%20%20%20%20%22type%22%3A%20%22image%2Fpng%22%0A%20%20%20%20%7D%0A%20%20%5D%2C%0A%20%20%22share_target%22%3A%7B%0A%20%20%20%20%22action%22%3A%22compose%2Ftweet%22%2C%0A%20%20%20%20%22params%22%3A%7B%0A%20%20%20%20%20%20%22title%22%3A%22title%22%2C%0A%20%20%20%20%20%20%22text%22%3A%22text%22%2C%0A%20%20%20%20%20%20%22url%22%3A%22url%22%0A%20%20%20%20%7D%0A%20%20%7D%0A%7D%0A%0A&seed=29
2) Tap "add to home screen" in app menu. Add dialog should show up. Tap "Add". This should install a WebAPK.
3) Set --check-for-web-manifest-update-on-startup command line flag
4) Launch WebAPK
Expected: "WebAPK upgrade needed: false" in adb logcat
Actual: "WebAPK upgrade needed: true" in adb logcat
 
This regression was caused by https://chromiumreview.googlesource.com/c/chromium/src/+/1271776

WebApkInfo#getSerializedShareTarget() returns a different result if the parameters are null and if the parameters are empty strings
Project Member

Comment 2 by bugdroid1@chromium.org, Dec 8

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/24ca62a15732f3487e4c0d810ef33f0b78e03831

commit 24ca62a15732f3487e4c0d810ef33f0b78e03831
Author: Peter Kotwicz <pkotwicz@chromium.org>
Date: Sat Dec 08 03:07:50 2018

[Android WebAPK] Do not request WebAPK updates if Web Manifest has not changed

This CL fixes a regression caused by  http://crbug.com/913160  for WebAPKs with
a share target.
WebApkInfo#getSerializedShareTarget() returns a different value depending on
whether the parameters are empty or null.
WebApkInfo#extractSerialiedShareTarget() passes null if a <meta-data> tag is
missing
WebApkDataFetcher#onDataAvailable() passes an empty string for values which
have not yet been plumbed through.

This CL:
- Changes WebApkInfo#getSerializedShareTarget() to return the same value
regardless of whether parameters are empty strings or null
- Changes WebApkDataFetcher#onDataAvailable() to pass in null for values
which have not yet been plumbed through

BUG= 913160 

Change-Id: I4147abf661b1fefd46a8c10a8686f49a986e96cd
Reviewed-on: https://chromium-review.googlesource.com/c/1368843
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Commit-Queue: Peter Kotwicz <pkotwicz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#614932}
[modify] https://crrev.com/24ca62a15732f3487e4c0d810ef33f0b78e03831/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkInfo.java
[modify] https://crrev.com/24ca62a15732f3487e4c0d810ef33f0b78e03831/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkUpdateDataFetcher.java
[modify] https://crrev.com/24ca62a15732f3487e4c0d810ef33f0b78e03831/chrome/android/junit/src/org/chromium/chrome/browser/webapps/WebApkInfoTest.java

Labels: Target-72 RegressedIn-72 Merge-Request-72
Project Member

Comment 4 by sheriffbot@chromium.org, Dec 11

Labels: -Merge-Request-72 Hotlist-Merge-Approved Merge-Approved-72
Your change meets the bar and is auto-approved for M72. Please go ahead and merge the CL to branch 3626 manually. Please contact milestone owner if you have questions.
Owners: govind@(Android), kariahda@(iOS), djmm@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Pls merge your change to M72 branch 3626 ASAP. Thank you.
Off topic: that URL is pretty awful but the functionality of a site that lets you write and serve a custom manifest via the URL is a good idea :). I've got a similar site at https://killer-marmot.appspot.com/custom/ which encodes the manifest in base64 which is much shorter and less likely to get munged by form fields.
Project Member

Comment 7 by bugdroid1@chromium.org, Dec 13

Labels: -merge-approved-72 merge-merged-3626
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/326ae97cc863d13ab7f07b6c4e112df9d4a22f75

commit 326ae97cc863d13ab7f07b6c4e112df9d4a22f75
Author: Peter Kotwicz <pkotwicz@chromium.org>
Date: Thu Dec 13 18:11:46 2018

[Android WebAPK] Do not request WebAPK updates if Web Manifest has not changed

This CL fixes a regression caused by  http://crbug.com/913160  for WebAPKs with
a share target.
WebApkInfo#getSerializedShareTarget() returns a different value depending on
whether the parameters are empty or null.
WebApkInfo#extractSerialiedShareTarget() passes null if a <meta-data> tag is
missing
WebApkDataFetcher#onDataAvailable() passes an empty string for values which
have not yet been plumbed through.

This CL:
- Changes WebApkInfo#getSerializedShareTarget() to return the same value
regardless of whether parameters are empty strings or null
- Changes WebApkDataFetcher#onDataAvailable() to pass in null for values
which have not yet been plumbed through

BUG= 913160 

Change-Id: I4147abf661b1fefd46a8c10a8686f49a986e96cd
Reviewed-on: https://chromium-review.googlesource.com/c/1368843
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Commit-Queue: Peter Kotwicz <pkotwicz@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#614932}(cherry picked from commit 24ca62a15732f3487e4c0d810ef33f0b78e03831)
Reviewed-on: https://chromium-review.googlesource.com/c/1376313
Reviewed-by: Peter Kotwicz <pkotwicz@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#331}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
[modify] https://crrev.com/326ae97cc863d13ab7f07b6c4e112df9d4a22f75/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkInfo.java
[modify] https://crrev.com/326ae97cc863d13ab7f07b6c4e112df9d4a22f75/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkUpdateDataFetcher.java
[modify] https://crrev.com/326ae97cc863d13ab7f07b6c4e112df9d4a22f75/chrome/android/junit/src/org/chromium/chrome/browser/webapps/WebApkInfoTest.java

Status: Fixed (was: Started)
Thanks Matt for the link
Labels: Merge-Merged-72-3626
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/326ae97cc863d13ab7f07b6c4e112df9d4a22f75

Commit: 326ae97cc863d13ab7f07b6c4e112df9d4a22f75
Author: pkotwicz@chromium.org
Commiter: pkotwicz@chromium.org
Date: 2018-12-13 18:11:46 +0000 UTC

[Android WebAPK] Do not request WebAPK updates if Web Manifest has not changed

This CL fixes a regression caused by  http://crbug.com/913160  for WebAPKs with
a share target.
WebApkInfo#getSerializedShareTarget() returns a different value depending on
whether the parameters are empty or null.
WebApkInfo#extractSerialiedShareTarget() passes null if a <meta-data> tag is
missing
WebApkDataFetcher#onDataAvailable() passes an empty string for values which
have not yet been plumbed through.

This CL:
- Changes WebApkInfo#getSerializedShareTarget() to return the same value
regardless of whether parameters are empty strings or null
- Changes WebApkDataFetcher#onDataAvailable() to pass in null for values
which have not yet been plumbed through

BUG= 913160 

Change-Id: I4147abf661b1fefd46a8c10a8686f49a986e96cd
Reviewed-on: https://chromium-review.googlesource.com/c/1368843
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Commit-Queue: Peter Kotwicz <pkotwicz@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#614932}(cherry picked from commit 24ca62a15732f3487e4c0d810ef33f0b78e03831)
Reviewed-on: https://chromium-review.googlesource.com/c/1376313
Reviewed-by: Peter Kotwicz <pkotwicz@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#331}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}

Sign in to add a comment