[ios] Adds code for Omaha and the upgrade center.

[ios] Adds code for Omaha and the upgrade center.

BUG=663349
Committed: https://crrev.com/e2d6faa121e5638c84bbaf482141f17ebfb15920
Cr-Commit-Position: refs/heads/master@{#438607}

[rohitrao (ping after 24h), 2016-12-14 02:59:44]

The CQ bit was checked by rohitrao@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-14 03:00:09]

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

[rohitrao (ping after 24h), 2016-12-14 03:00:26]

rohitrao@chromium.org changed reviewers:
+ brettw@chromium.org, sdefresne@chromium.org

[rohitrao (ping after 24h), 2016-12-14 03:00:26]

+brettw for the libxml DEPS addition

+sdefresne for everything else

[commit-bot: I haz the power, 2016-12-14 03:59:22]

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

[commit-bot: I haz the power, 2016-12-14 03:59:22]

Dry run: This issue passed the CQ dry run.

[sdefresne, 2016-12-14 11:50:57]

lgtm

All comments should probably be addressed in a followup CL so that they don't
block upstreaming.

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/DEPS
File ios/chrome/browser/DEPS (right):

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/DEPS#new...
ios/chrome/browser/DEPS:95: "+libxml/xmlwriter.h",
Is this the only DEPS addition we missed?

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/omaha/om...
File ios/chrome/browser/omaha/omaha_service.h (right):

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/omaha/om...
ios/chrome/browser/omaha/omaha_service.h:33: // called from the io thread.
nit: io -> IO

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/omaha/om...
ios/chrome/browser/omaha/omaha_service.h:40: // Start the service. Also set the
|URLRequestContextGetter| necessary to
Should it be "Starts" (I'm terrible at English grammar, but the other comments
use the "s" form of the verbs)?

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/omaha/om...
ios/chrome/browser/omaha/omaha_service.h:101: // Send a ping to the Omaha
server.
"Sends"?

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/omaha/om...
ios/chrome/browser/omaha/omaha_service.h:108: // Persisting states of the
service.
"Persists state of the service."?

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/omaha/om...
ios/chrome/browser/omaha/omaha_service.h:121: // Returns the xml representation
of the ping message to send to the Omaha
nit: XML

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/omaha/om...
ios/chrome/browser/omaha/omaha_service.h:154: net::URLRequestContextGetter*
request_context_getter_;  // weak
Remove "// weak" (if it were strong, then a std::unique_ptr<> or shared_ptr<>
would be used).

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/omaha/om...
File ios/chrome/browser/omaha/omaha_service.mm (right):

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/omaha/om...
ios/chrome/browser/omaha/omaha_service.mm:119: // XML Parser for the server
response.
nit: "XML parser for the server response."

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/omaha/om...
ios/chrome/browser/omaha/omaha_service.mm:121: @public
This is the default, so you can remove.

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/omaha/om...
ios/chrome/browser/omaha/omaha_service.mm:136: -
(id)initWithAppId:(NSString*)appId;
nit: id -> instancetype

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/omaha/om...
ios/chrome/browser/omaha/omaha_service.mm:149: -
(id)initWithAppId:(NSString*)appId {
nit: id -> instancetype

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/omaha/om...
ios/chrome/browser/omaha/omaha_service.mm:235: updateInformation_.reset(new
UpgradeRecommendedDetails);
nit: can you change to use base::MakeUnique

  updateInformation_ = base::MakeUnique<UpgradeRecommendedDetails>();

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/omaha/om...
ios/chrome/browser/omaha/omaha_service.mm:613: // Handle success
nit: can you add a dot at the end of comment? "Handle success."

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/omaha/om...
ios/chrome/browser/omaha/omaha_service.mm:641:
std::unique_ptr<base::DictionaryValue> result(new base::DictionaryValue);
nit: can you change to use base::MakeUnique

  auto result = base::MakeUnique<base::DictionaryValue>();

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/upgrade/...
File ios/chrome/browser/upgrade/upgrade_center.h (right):

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/upgrade/...
ios/chrome/browser/upgrade/upgrade_center.h:19: @required
nit: @required is the default, please remove

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/upgrade/...
ios/chrome/browser/upgrade/upgrade_center.h:24: 
nit: remove blank line

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/upgrade/...
ios/chrome/browser/upgrade/upgrade_center.h:32: // Register a client for the
UpgradeCenter. The clients are not retained,
nit: Registers?

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/upgrade/...
ios/chrome/browser/upgrade/upgrade_center.h:36: // Unregister a client.
nit: Unregisters?

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/upgrade/...
ios/chrome/browser/upgrade/upgrade_center.h:40: // created. The infobar will not
be created if is already there or if there is
nit: "if is" -> "if it is"

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/upgrade/...
File ios/chrome/browser/upgrade/upgrade_center.mm (right):

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/upgrade/...
ios/chrome/browser/upgrade/upgrade_center.mm:32: // Create infobars on all tabs.
nit: Creates?

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/upgrade/...
ios/chrome/browser/upgrade/upgrade_center.mm:34: // Remove all the infobars.
nit: Removes?

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/upgrade/...
ios/chrome/browser/upgrade/upgrade_center.mm:39: // Return YES if the infobar
should be shown.
nit: Returns?

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/upgrade/...
ios/chrome/browser/upgrade/upgrade_center.mm:41: // Return YES if the last
version signaled by a server side service is more
nit: Returns?

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/upgrade/...
ios/chrome/browser/upgrade/upgrade_center.mm:216: #ifndef NDEBUG
nit: I think all "#ifndef NDEBUG" should be changed to "#if DCHECK_IS_ON()" as
we can build release with DCHECK.

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/upgrade/...
ios/chrome/browser/upgrade/upgrade_center.mm:248: // Return YES if the infobar
should be shown.
Comment is already at the top of the file where the method is forward-declared,
remove from here.

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/upgrade/...
ios/chrome/browser/upgrade/upgrade_center.mm:299: // For the client to call when
-showUpgrade: is called or when a new tab is
The comment is already in the header file, please remove from the
implementation.

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/upgrade/...
ios/chrome/browser/upgrade/upgrade_center.mm:315:
std::unique_ptr<UpgradeInfoBarDelegate> infobarDelegate(
nit: use base::MakeUnique

  auto infobarDelegate = base::MakeUnique<UpgradeInfoBarDelegate>();

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/upgrade/...
ios/chrome/browser/upgrade/upgrade_center.mm:332: // Callback when an infobar is
closed, for any reason. Perform upgrade is set to
Comment is already at the top of the file when the method is forward-declared,
remove from here.

[rohitrao (ping after 24h), 2016-12-14 13:25:46]

The CQ bit was checked by rohitrao@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-14 13:25:56]

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

[rohitrao (ping after 24h), 2016-12-14 13:26:33]

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/DEPS
File ios/chrome/browser/DEPS (right):

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/DEPS#new...
ios/chrome/browser/DEPS:95: "+libxml/xmlwriter.h",
On 2016/12/14 11:50:55, sdefresne wrote:
> Is this the only DEPS addition we missed?

I believe so.  Will double-check the next time I do a dry run.

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/omaha/om...
File ios/chrome/browser/omaha/omaha_service.h (right):

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/omaha/om...
ios/chrome/browser/omaha/omaha_service.h:33: // called from the io thread.
On 2016/12/14 11:50:55, sdefresne wrote:
> nit: io -> IO

Done.

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/omaha/om...
ios/chrome/browser/omaha/omaha_service.h:40: // Start the service. Also set the
|URLRequestContextGetter| necessary to
On 2016/12/14 11:50:55, sdefresne wrote:
> Should it be "Starts" (I'm terrible at English grammar, but the other comments
> use the "s" form of the verbs)?

Done.

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/omaha/om...
ios/chrome/browser/omaha/omaha_service.h:101: // Send a ping to the Omaha
server.
On 2016/12/14 11:50:55, sdefresne wrote:
> "Sends"?

Done.

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/omaha/om...
ios/chrome/browser/omaha/omaha_service.h:108: // Persisting states of the
service.
On 2016/12/14 11:50:55, sdefresne wrote:
> "Persists state of the service."?

Done.

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/omaha/om...
ios/chrome/browser/omaha/omaha_service.h:121: // Returns the xml representation
of the ping message to send to the Omaha
On 2016/12/14 11:50:55, sdefresne wrote:
> nit: XML

Done.

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/omaha/om...
ios/chrome/browser/omaha/omaha_service.h:154: net::URLRequestContextGetter*
request_context_getter_;  // weak
On 2016/12/14 11:50:55, sdefresne wrote:
> Remove "// weak" (if it were strong, then a std::unique_ptr<> or shared_ptr<>
> would be used).

Done.

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/omaha/om...
File ios/chrome/browser/omaha/omaha_service.mm (right):

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/omaha/om...
ios/chrome/browser/omaha/omaha_service.mm:119: // XML Parser for the server
response.
On 2016/12/14 11:50:56, sdefresne wrote:
> nit: "XML parser for the server response."

Done.

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/omaha/om...
ios/chrome/browser/omaha/omaha_service.mm:121: @public
On 2016/12/14 11:50:56, sdefresne wrote:
> This is the default, so you can remove.

Done.

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/omaha/om...
ios/chrome/browser/omaha/omaha_service.mm:136: -
(id)initWithAppId:(NSString*)appId;
On 2016/12/14 11:50:56, sdefresne wrote:
> nit: id -> instancetype

This is some old code =)

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/omaha/om...
ios/chrome/browser/omaha/omaha_service.mm:149: -
(id)initWithAppId:(NSString*)appId {
On 2016/12/14 11:50:56, sdefresne wrote:
> nit: id -> instancetype

Done.

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/omaha/om...
ios/chrome/browser/omaha/omaha_service.mm:235: updateInformation_.reset(new
UpgradeRecommendedDetails);
On 2016/12/14 11:50:56, sdefresne wrote:
> nit: can you change to use base::MakeUnique
> 
>   updateInformation_ = base::MakeUnique<UpgradeRecommendedDetails>();

Done.

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/omaha/om...
ios/chrome/browser/omaha/omaha_service.mm:613: // Handle success
On 2016/12/14 11:50:56, sdefresne wrote:
> nit: can you add a dot at the end of comment? "Handle success."

Done.

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/omaha/om...
ios/chrome/browser/omaha/omaha_service.mm:641:
std::unique_ptr<base::DictionaryValue> result(new base::DictionaryValue);
On 2016/12/14 11:50:55, sdefresne wrote:
> nit: can you change to use base::MakeUnique
> 
>   auto result = base::MakeUnique<base::DictionaryValue>();

Done.

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/upgrade/...
File ios/chrome/browser/upgrade/upgrade_center.h (right):

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/upgrade/...
ios/chrome/browser/upgrade/upgrade_center.h:19: @required
On 2016/12/14 11:50:56, sdefresne wrote:
> nit: @required is the default, please remove

Done.

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/upgrade/...
ios/chrome/browser/upgrade/upgrade_center.h:24: 
On 2016/12/14 11:50:56, sdefresne wrote:
> nit: remove blank line

Added a blank line above instead.

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/upgrade/...
ios/chrome/browser/upgrade/upgrade_center.h:32: // Register a client for the
UpgradeCenter. The clients are not retained,
On 2016/12/14 11:50:56, sdefresne wrote:
> nit: Registers?

Done.

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/upgrade/...
ios/chrome/browser/upgrade/upgrade_center.h:36: // Unregister a client.
On 2016/12/14 11:50:56, sdefresne wrote:
> nit: Unregisters?

Done.

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/upgrade/...
ios/chrome/browser/upgrade/upgrade_center.h:40: // created. The infobar will not
be created if is already there or if there is
On 2016/12/14 11:50:56, sdefresne wrote:
> nit: "if is" -> "if it is"

Done.

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/upgrade/...
File ios/chrome/browser/upgrade/upgrade_center.mm (right):

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/upgrade/...
ios/chrome/browser/upgrade/upgrade_center.mm:32: // Create infobars on all tabs.
On 2016/12/14 11:50:56, sdefresne wrote:
> nit: Creates?

Done.

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/upgrade/...
ios/chrome/browser/upgrade/upgrade_center.mm:34: // Remove all the infobars.
On 2016/12/14 11:50:56, sdefresne wrote:
> nit: Removes?

Done.

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/upgrade/...
ios/chrome/browser/upgrade/upgrade_center.mm:39: // Return YES if the infobar
should be shown.
On 2016/12/14 11:50:56, sdefresne wrote:
> nit: Returns?

Done.

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/upgrade/...
ios/chrome/browser/upgrade/upgrade_center.mm:41: // Return YES if the last
version signaled by a server side service is more
On 2016/12/14 11:50:56, sdefresne wrote:
> nit: Returns?

Done.

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/upgrade/...
ios/chrome/browser/upgrade/upgrade_center.mm:216: #ifndef NDEBUG
On 2016/12/14 11:50:56, sdefresne wrote:
> nit: I think all "#ifndef NDEBUG" should be changed to "#if DCHECK_IS_ON()" as
> we can build release with DCHECK.

Noted.  Not changing in this CL.

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/upgrade/...
ios/chrome/browser/upgrade/upgrade_center.mm:248: // Return YES if the infobar
should be shown.
On 2016/12/14 11:50:56, sdefresne wrote:
> Comment is already at the top of the file where the method is
forward-declared,
> remove from here.

Done.

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/upgrade/...
ios/chrome/browser/upgrade/upgrade_center.mm:299: // For the client to call when
-showUpgrade: is called or when a new tab is
On 2016/12/14 11:50:56, sdefresne wrote:
> The comment is already in the header file, please remove from the
> implementation.

Done.

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/upgrade/...
ios/chrome/browser/upgrade/upgrade_center.mm:315:
std::unique_ptr<UpgradeInfoBarDelegate> infobarDelegate(
On 2016/12/14 11:50:56, sdefresne wrote:
> nit: use base::MakeUnique
> 
>   auto infobarDelegate = base::MakeUnique<UpgradeInfoBarDelegate>();

Done.

https://codereview.chromium.org/2568003005/diff/1/ios/chrome/browser/upgrade/...
ios/chrome/browser/upgrade/upgrade_center.mm:332: // Callback when an infobar is
closed, for any reason. Perform upgrade is set to
On 2016/12/14 11:50:56, sdefresne wrote:
> Comment is already at the top of the file when the method is forward-declared,
> remove from here.

Done.

[commit-bot: I haz the power, 2016-12-14 14:37:48]

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

[commit-bot: I haz the power, 2016-12-14 14:37:49]

Dry run: This issue passed the CQ dry run.

[brettw, 2016-12-14 20:16:10]

deps lgtm, I didn't look at the rest.

[rohitrao (ping after 24h), 2016-12-14 20:16:47]

The CQ bit was checked by rohitrao@chromium.org

[rohitrao (ping after 24h), 2016-12-14 20:16:48]

The patchset sent to the CQ was uploaded after l-g-t-m from
sdefresne@chromium.org
Link to the patchset: https://codereview.chromium.org/2568003005/#ps20001
(title: "Review.")

[commit-bot: I haz the power, 2016-12-14 20:17:26]

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, 2016-12-14 20:25:51]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1481746607959820,
"parent_rev": "d2cbdeb947886bc6f9162e1e317f325001294032", "commit_rev":
"4cbb1e799fcd4b5037613ed8ff7a7c1a18f8f701"}

[commit-bot: I haz the power, 2016-12-14 20:26:26]

Description was changed from

==========
[ios] Adds code for Omaha and the upgrade center.

BUG=663349
==========

to

==========
[ios] Adds code for Omaha and the upgrade center.

BUG=663349

Review-Url: https://codereview.chromium.org/2568003005
==========

[commit-bot: I haz the power, 2016-12-14 20:26:27]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-12-14 20:30:35]

Description was changed from

==========
[ios] Adds code for Omaha and the upgrade center.

BUG=663349

Review-Url: https://codereview.chromium.org/2568003005
==========

to

==========
[ios] Adds code for Omaha and the upgrade center.

BUG=663349

Committed: https://crrev.com/e2d6faa121e5638c84bbaf482141f17ebfb15920
Cr-Commit-Position: refs/heads/master@{#438607}
==========

[commit-bot: I haz the power, 2016-12-14 20:30:36]

Patchset 2 (id:??) landed as
https://crrev.com/e2d6faa121e5638c84bbaf482141f17ebfb15920
Cr-Commit-Position: refs/heads/master@{#438607}