New issue
Advanced search Search tips

Issue 709226 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

WebAPK updates are broken

Project Member Reported by pkotw...@chromium.org, Apr 6 2017

Issue description

WebAPK updates are broken

 
i don't think they are - they were working just fine for me yesterday on dev - can you elaborate? This bug isn't very helpful
Project Member

Comment 2 by bugdroid1@chromium.org, Apr 15 2017

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

commit b4c7f73c1d4ee45836f07d959d7af1e4a12cdcc7
Author: pkotwicz <pkotwicz@chromium.org>
Date: Sat Apr 15 02:52:07 2017

[Android WebAPK] Fix regression by https://codereview.chromium.org/2725813004/

https://codereview.chromium.org/2725813004/ caused
WebappDataStorage#updateTimeOfLastCheckForUpdatedWebManifest() to be called each
time that the WebAPK is launched effectively disabling WebAPK updates.

BUG= 709226 

Review-Url: https://codereview.chromium.org/2801873004
Cr-Commit-Position: refs/heads/master@{#464854}

[modify] https://crrev.com/b4c7f73c1d4ee45836f07d959d7af1e4a12cdcc7/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkActivity.java

Labels: -Pri-3 Merge-Request-58 Merge-Request-59 Pri-2
I see what you're saying now from the CL - if the shell version doesn't update then we won't ever update webapks? That's pretty bad. Requesting merge of above (1-line change) to branches.
Project Member

Comment 4 by sheriffbot@chromium.org, Apr 18 2017

Labels: -Merge-Request-59 Hotlist-Merge-Approved Merge-Approved-59
Your change meets the bar and is auto-approved for M59. Please go ahead and merge the CL to branch 3071 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), gkihumba@(ChromeOS), Abdul Syed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 5 by sheriffbot@chromium.org, Apr 18 2017

Labels: -Merge-Request-58 Merge-Review-58 Hotlist-Merge-Review
This bug requires manual review: We are only 6 days from stable.
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Review-58 Merge-Rejected-58
Had this been requested sooner, we could have taken it, but as is it is too late.
Ok, but if we respin the build, can we reconsider this? THe codepath is only reached if the user opens a WebAPK so risk is low. 
Project Member

Comment 8 by bugdroid1@chromium.org, Apr 20 2017

Labels: -merge-approved-59 merge-merged-3071
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ac860834850b2f851e5e31dacfcec4542cf97dbc

commit ac860834850b2f851e5e31dacfcec4542cf97dbc
Author: Peter Kotwicz <pkotwicz@google.com>
Date: Thu Apr 20 18:03:09 2017

Merge 59 [Android WebAPK] Fix regression by https://codereview.chromium.org/2725813004/

https://codereview.chromium.org/2725813004/ caused
WebappDataStorage#updateTimeOfLastCheckForUpdatedWebManifest() to be called each
time that the WebAPK is launched effectively disabling WebAPK updates.

BUG= 709226 

Review-Url: https://codereview.chromium.org/2801873004
Cr-Commit-Position: refs/heads/master@{#464854}
(cherry picked from commit b4c7f73c1d4ee45836f07d959d7af1e4a12cdcc7)

Review-Url: https://codereview.chromium.org/2827373002 .
Cr-Commit-Position: refs/branch-heads/3071@{#89}
Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641}

[modify] https://crrev.com/ac860834850b2f851e5e31dacfcec4542cf97dbc/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkActivity.java

Status: Fixed (was: Assigned)
Tentatively marking bug fixed, please let us know if there's a respin for 58 as it is quite low risk

Sign in to add a comment