Don't download identical APKs on update.

Don't download identical APKs on update.

When we update a certified WebAPK whose Clank-fetched content no longer matches
its Harpoon-fetched content, Clank will always think the WebAPK is out of date,
and check for a new update from the WAM at every opportunity.

To mitigate this, server returns an empty download URL in the Response proto.
It tells the client _not_ to download the APK from the gstatic URL that the
server provides, because it will be exactly identical to the one already
installed on the device. This will save the bulk of the bandwidth usage for
unnecessary update.

BUG=680131
Review-Url: https://codereview.chromium.org/2632243004
Cr-Commit-Position: refs/heads/master@{#449688}
Committed: https://chromium.googlesource.com/chromium/src/+/345600684d7a9a0a31298d165e3e7d7deab34076

[Xi Han, 2017-01-16 21:42:08]

hanxi@chromium.org changed reviewers:
+ pkotwicz@chromium.org

[Xi Han, 2017-01-16 21:42:08]

Hi Peter, could you please take a look? Thanks!

[pkotwicz, 2017-01-17 02:37:10]

Your CL looks good. Can you also implement exponential backoff in this CL?
http://crbug.com/680128

https://codereview.chromium.org/2632243004/diff/1/chrome/browser/android/weba...
File chrome/browser/android/webapk/webapk_installer.cc (right):

https://codereview.chromium.org/2632243004/diff/1/chrome/browser/android/weba...
chrome/browser/android/webapk/webapk_installer.cc:421: //
https://crbug.com/680131. Don't download identical APKs on update.
How about: "The server sends an empty URL if the server does not have a newer
WebAPK to update to."

[Xi Han, 2017-01-17 16:45:12]

Hi Peter, I will implement the other bug in another CL. PTAL, thanks!

https://codereview.chromium.org/2632243004/diff/1/chrome/browser/android/weba...
File chrome/browser/android/webapk/webapk_installer.cc (right):

https://codereview.chromium.org/2632243004/diff/1/chrome/browser/android/weba...
chrome/browser/android/webapk/webapk_installer.cc:421: //
https://crbug.com/680131. Don't download identical APKs on update.
On 2017/01/17 02:37:10, pkotwicz wrote:
> How about: "The server sends an empty URL if the server does not have a newer
> WebAPK to update to."
> 

Done.

[Xi Han, 2017-01-27 20:12:59]

Hi Peter, do you have any concern of this CL?

[pkotwicz, 2017-01-27 22:56:00]

I am holding off on reviewing this CL till we handle all of the launch blocking
bugs (Yes I am slightly lazy). I think that fixing this bug crbug.com/658498 is
more important than fixing this bug.

[pkotwicz, 2017-02-10 02:50:31]

pkotwicz@chromium.org changed reviewers:
+ dominickn@chromium.org

[pkotwicz, 2017-02-10 02:50:31]

LGTM with nits

Off to Dominick!

Dominick, this CL does the setup for https://codereview.chromium.org/2641973003/

https://codereview.chromium.org/2632243004/diff/20001/chrome/browser/android/...
File chrome/browser/android/webapk/webapk.proto (right):

https://codereview.chromium.org/2632243004/diff/20001/chrome/browser/android/...
chrome/browser/android/webapk/webapk.proto:24: // WebAPK it already has.
How about: "If the URL is empty, the client should not ..."
"this" -> "the URL"

WAM -> "WebAPK Server" (Unless you want to check in a glossary somewhere in the
codebase 😉)

Remove "already"

https://codereview.chromium.org/2632243004/diff/20001/chrome/browser/android/...
File chrome/browser/android/webapk/webapk_installer_unittest.cc (right):

https://codereview.chromium.org/2632243004/diff/20001/chrome/browser/android/...
chrome/browser/android/webapk/webapk_installer_unittest.cc:428: // an empty
download URL.
Nits:
"updates" -> "an update"
"succeeded" -> "succeeds"

Can you please add:
"
The WebAPK server sends an empty download URL when:
- The server is unable to update the WebAPK in the way that the client
requested.
AND
- The most up to date version of the WebAPK on the server is identical to the
one installed on the client.
"

[dominickn, 2017-02-10 03:00:11]

lgtm

Out of curiosity, WebAPKs are still updated by Chrome even if they're installed
by Play? Or does this only apply with non-Play WebAPKs?

[Xi Han, 2017-02-10 18:21:09]

> Out of curiosity, WebAPKs are still updated by Chrome even if they're
installed
> by Play? Or does this only apply with non-Play WebAPKs?

Yes, WebAPKs are still updated by Chrome. This is because WebAPKs aren't listed
in Play Store, so Play Store won't auto update the WebAPKs. Chrome calls the
same install API provided by Play to trigger WebAPK's updates.

https://codereview.chromium.org/2632243004/diff/20001/chrome/browser/android/...
File chrome/browser/android/webapk/webapk.proto (right):

https://codereview.chromium.org/2632243004/diff/20001/chrome/browser/android/...
chrome/browser/android/webapk/webapk.proto:24: // WebAPK it already has.
On 2017/02/10 02:50:31, pkotwicz wrote:
> How about: "If the URL is empty, the client should not ..."
> "this" -> "the URL"
> 
> WAM -> "WebAPK Server" (Unless you want to check in a glossary somewhere in
the
> codebase 😉)
> 
> Remove "already"

Done.

https://codereview.chromium.org/2632243004/diff/20001/chrome/browser/android/...
File chrome/browser/android/webapk/webapk_installer_unittest.cc (right):

https://codereview.chromium.org/2632243004/diff/20001/chrome/browser/android/...
chrome/browser/android/webapk/webapk_installer_unittest.cc:428: // an empty
download URL.
On 2017/02/10 02:50:31, pkotwicz wrote:
> Nits:
> "updates" -> "an update"
> "succeeded" -> "succeeds"
> 
> Can you please add:
> "
> The WebAPK server sends an empty download URL when:
> - The server is unable to update the WebAPK in the way that the client
> requested.
> AND
> - The most up to date version of the WebAPK on the server is identical to the
> one installed on the client.
> "

Done.

[Xi Han, 2017-02-10 18:27:58]

The CQ bit was checked by hanxi@chromium.org

[Xi Han, 2017-02-10 18:27:59]

The patchset sent to the CQ was uploaded after l-g-t-m from
dominickn@chromium.org, pkotwicz@chromium.org
Link to the patchset: https://codereview.chromium.org/2632243004/#ps40001
(title: "Nits.")

[commit-bot: I haz the power, 2017-02-10 18:28:40]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-10 19:29:40]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1486751278701680,
"parent_rev": "525b0d69b50807ab7c9901f52acf858c0e5e1ba8", "commit_rev":
"345600684d7a9a0a31298d165e3e7d7deab34076"}

[commit-bot: I haz the power, 2017-02-10 19:30:17]

Description was changed from

==========
Don't download identical APKs on update.

When we update a certified WebAPK whose Clank-fetched content no longer matches
its Harpoon-fetched content, Clank will always think the WebAPK is out of date,
and check for a new update from the WAM at every opportunity.

To mitigate this, server returns an empty download URL in the Response proto.
It tells the client _not_ to download the APK from the gstatic URL that the
server provides, because it will be exactly identical to the one already
installed on the device. This will save the bulk of the bandwidth usage for
unnecessary update.

BUG=680131
==========

to

==========
Don't download identical APKs on update.

When we update a certified WebAPK whose Clank-fetched content no longer matches
its Harpoon-fetched content, Clank will always think the WebAPK is out of date,
and check for a new update from the WAM at every opportunity.

To mitigate this, server returns an empty download URL in the Response proto.
It tells the client _not_ to download the APK from the gstatic URL that the
server provides, because it will be exactly identical to the one already
installed on the device. This will save the bulk of the bandwidth usage for
unnecessary update.

BUG=680131

Review-Url: https://codereview.chromium.org/2632243004
Cr-Commit-Position: refs/heads/master@{#449688}
Committed:
https://chromium.googlesource.com/chromium/src/+/345600684d7a9a0a31298d165e3e...
==========

[commit-bot: I haz the power, 2017-02-10 19:30:21]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/345600684d7a9a0a31298d165e3e...