[Android WebAPK] Change WebAPK update into two phases

[Android WebAPK] Change WebAPK update into two phases

This CL splits WebAPK updates into two phases:
Phase #1: Compute the proto to send to the WebAPK Minting Server
Phase #2: Send the proto to send to the WebAPK Minting Server and talk to
Google Play

Phase #1 puts the data needed to do an update into an easily serializable
format. This enables a follow up CL to put the output of Phase #1 into a
org.chromium.components.background_task_scheduler.TaskInfo os.Bundle for use
with BackgroundTaskScheduler

BUG=713655
Review-Url: https://codereview.chromium.org/2933783002
Cr-Commit-Position: refs/heads/master@{#480255}
Committed: https://chromium.googlesource.com/chromium/src/+/9940e1d1afe38894e862a18d605b1a64ecb4c2c2

[pkotwicz, 2017-06-12 03:48:32]

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

[pkotwicz, 2017-06-12 03:48:32]

Xi, can you please take a look?

[pkotwicz, 2017-06-12 03:49:42]

Description was changed from

==========
[Android WebAPK] Change WebAPK update into two phases

This CL splits WebAPK updates into two phases:
Phase #1: Compute the proto to send to the WebAPK Minting Server
Phase #2: Send the proto to send to the WebAPK Minting Server and talk to
Google Play

The goal of the split is that Phase #1 puts data needed to do an update into
an easily serializable form. This enables a follow up CL to put the output
of Phase #1 into a
org.chromium.components.background_task_scheduler.TaskInfo os.Bundle
for use with BackgroundTaskScheduler

BUG=713655
==========

to

==========
[Android WebAPK] Change WebAPK update into two phases

This CL splits WebAPK updates into two phases:
Phase #1: Compute the proto to send to the WebAPK Minting Server
Phase #2: Send the proto to send to the WebAPK Minting Server and talk to
Google Play

Phase #1 puts the data needed to do an update into an easily serializable
format. This enables a follow up CL to put the output of Phase #1 into a
org.chromium.components.background_task_scheduler.TaskInfo os.Bundle for use
with BackgroundTaskScheduler

BUG=713655
==========

[Xi Han, 2017-06-12 19:07:28]

lgtm

[pkotwicz, 2017-06-12 19:26:13]

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

[pkotwicz, 2017-06-12 19:26:14]

Dominick for OWNERS

[dominickn, 2017-06-13 06:42:01]

On 2017/06/12 19:26:14, pkotwicz wrote:
> Dominick for OWNERS

In the queue - it's a fairly big CL so it's taking a little longer than hoped.

[dominickn, 2017-06-14 00:14:23]

Looks pretty good at first pass. I'll do one more in case I missed anything.

https://codereview.chromium.org/2933783002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkUpdateManager.java
(right):

https://codereview.chromium.org/2933783002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkUpdateManager.java:187:
protected void buildProto(final WebApkInfo info, String bestIconUrl, boolean
isManifestStale) {
Call this buildProtoAndScheduleUpdate?

https://codereview.chromium.org/2933783002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkUpdateManager.java:196:
iconHashes[i] = iconHash != null ? iconHash : "";
Nit: (iconHash == null) ? "" : iconHash might be clearer (I know you just moved
this code from elsewhere). At the very least the brackets would be good.

https://codereview.chromium.org/2933783002/diff/1/chrome/browser/android/weba...
File chrome/browser/android/webapk/webapk_install_service.h (right):

https://codereview.chromium.org/2933783002/diff/1/chrome/browser/android/weba...
chrome/browser/android/webapk/webapk_install_service.h:11: #include <vector>
#include <memory> for std::unique_ptr

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

https://codereview.chromium.org/2933783002/diff/1/chrome/browser/android/weba...
chrome/browser/android/webapk/webapk_installer.cc:6: 
#include <utility> for std::move
#include <memory> for std::unique_ptr

https://codereview.chromium.org/2933783002/diff/1/chrome/browser/android/weba...
chrome/browser/android/webapk/webapk_installer.cc:176: new
std::vector<uint8_t>);
Use base::MakeUnique from base/memory/ptr_utils.h instead of bare new.

https://codereview.chromium.org/2933783002/diff/1/chrome/browser/android/weba...
chrome/browser/android/webapk/webapk_installer.cc:223: // staic
Minor nit: "static"

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

https://codereview.chromium.org/2933783002/diff/1/chrome/browser/android/weba...
chrome/browser/android/webapk/webapk_installer_unittest.cc:101: ShortcutInfo
info(GURL::EmptyGURL());
Use GURL() (EmptyGURL() should only be used when you need a reference returned).

https://codereview.chromium.org/2933783002/diff/1/chrome/browser/android/weba...
chrome/browser/android/webapk/webapk_installer_unittest.cc:120: new
std::vector<uint8_t>(serialized_proto.begin(),
base::MakeUnique

https://codereview.chromium.org/2933783002/diff/1/chrome/browser/android/weba...
chrome/browser/android/webapk/webapk_installer_unittest.cc:217:
webapk_request_.reset(new webapk::WebApk);
base::MakeUnique

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

https://codereview.chromium.org/2933783002/diff/1/chrome/browser/android/weba...
chrome/browser/android/webapk/webapk_update_manager.cc:7: #include <jni.h>
#include <memory>
#include <vector>

https://codereview.chromium.org/2933783002/diff/1/chrome/browser/android/weba...
chrome/browser/android/webapk/webapk_update_manager.cc:150: new
std::vector<uint8_t>);
base::MakeUnique

[pkotwicz, 2017-06-15 01:57:22]

Patchset #2 (id:20001) has been deleted

[pkotwicz, 2017-06-15 02:25:20]

Dominick, can you please take another look? I think that I have addressed all of
your comments

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

https://codereview.chromium.org/2933783002/diff/1/chrome/browser/android/weba...
chrome/browser/android/webapk/webapk_installer.cc:6: 
Added include for <utility>

webapk_installer.h already includes <memory>

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

https://codereview.chromium.org/2933783002/diff/1/chrome/browser/android/weba...
chrome/browser/android/webapk/webapk_installer_unittest.cc:101: ShortcutInfo
info(GURL::EmptyGURL());
Ok. This has to be

ShortcutInfo info((GURL()));

to avoid the following error:
"parentheses were disambiguated as a function declaration
[-Werror,-Wvexing-parse]"

[dominickn, 2017-06-15 03:35:05]

lgtm, thanks

[pkotwicz, 2017-06-15 19:50:17]

Patchset #5 (id:100001) has been deleted

[pkotwicz, 2017-06-15 19:50:31]

Patchset #4 (id:80001) has been deleted

[pkotwicz, 2017-06-15 21:47:19]

Patchset #4 (id:120001) has been deleted

[pkotwicz, 2017-06-15 22:14:44]

The CQ bit was checked by pkotwicz@chromium.org

[pkotwicz, 2017-06-15 22:14:45]

The patchset sent to the CQ was uploaded after l-g-t-m from hanxi@chromium.org,
dominickn@chromium.org
Link to the patchset: https://codereview.chromium.org/2933783002/#ps140001
(title: "Merge branch 'master' into background_updates0")

[commit-bot: I haz the power, 2017-06-15 22:15:35]

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-06-15 22:46:02]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-15 22:46:03]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[pkotwicz, 2017-06-16 00:17:57]

The CQ bit was checked by pkotwicz@chromium.org

[commit-bot: I haz the power, 2017-06-16 00:18:20]

CQ is trying da patch.

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

[pkotwicz, 2017-06-16 00:39:27]

The CQ bit was checked by pkotwicz@chromium.org

[pkotwicz, 2017-06-16 00:39:27]

The patchset sent to the CQ was uploaded after l-g-t-m from
dominickn@chromium.org, hanxi@chromium.org
Link to the patchset: https://codereview.chromium.org/2933783002/#ps160001
(title: "Merge branch 'master' into background_updates0")

[commit-bot: I haz the power, 2017-06-16 00:39:37]

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-06-16 00:46:48]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-16 00:46:49]

Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)

[pkotwicz, 2017-06-16 23:56:42]

The CQ bit was checked by pkotwicz@chromium.org

[pkotwicz, 2017-06-16 23:56:42]

The patchset sent to the CQ was uploaded after l-g-t-m from
dominickn@chromium.org, hanxi@chromium.org
Link to the patchset: https://codereview.chromium.org/2933783002/#ps180001
(title: "Merge branch 'master' into background_updates0")

[commit-bot: I haz the power, 2017-06-16 23:57:14]

CQ is trying da patch.

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

[pkotwicz, 2017-06-17 00:49:21]

The CQ bit was unchecked by pkotwicz@chromium.org

[pkotwicz, 2017-06-17 00:51:31]

The CQ bit was checked by pkotwicz@chromium.org

[commit-bot: I haz the power, 2017-06-17 00:51:50]

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-06-17 00:55:44]

CQ is committing da patch.

Bot data: {"patchset_id": 180001, "attempt_start_ts": 1497660691484730,
"parent_rev": "72d18c6ae485173d6a01ec35ca8b0847aa8bea9d", "commit_rev":
"9940e1d1afe38894e862a18d605b1a64ecb4c2c2"}

[commit-bot: I haz the power, 2017-06-17 00:55:57]

Description was changed from

==========
[Android WebAPK] Change WebAPK update into two phases

This CL splits WebAPK updates into two phases:
Phase #1: Compute the proto to send to the WebAPK Minting Server
Phase #2: Send the proto to send to the WebAPK Minting Server and talk to
Google Play

Phase #1 puts the data needed to do an update into an easily serializable
format. This enables a follow up CL to put the output of Phase #1 into a
org.chromium.components.background_task_scheduler.TaskInfo os.Bundle for use
with BackgroundTaskScheduler

BUG=713655
==========

to

==========
[Android WebAPK] Change WebAPK update into two phases

This CL splits WebAPK updates into two phases:
Phase #1: Compute the proto to send to the WebAPK Minting Server
Phase #2: Send the proto to send to the WebAPK Minting Server and talk to
Google Play

Phase #1 puts the data needed to do an update into an easily serializable
format. This enables a follow up CL to put the output of Phase #1 into a
org.chromium.components.background_task_scheduler.TaskInfo os.Bundle for use
with BackgroundTaskScheduler

BUG=713655

Review-Url: https://codereview.chromium.org/2933783002
Cr-Commit-Position: refs/heads/master@{#480255}
Committed:
https://chromium.googlesource.com/chromium/src/+/9940e1d1afe38894e862a18d605b...
==========

[commit-bot: I haz the power, 2017-06-17 00:55:59]

Committed patchset #6 (id:180001) as
https://chromium.googlesource.com/chromium/src/+/9940e1d1afe38894e862a18d605b...