Provide a callback for Component Updater OnDemand calls.

Provide a callback for Component Updater OnDemand calls.

The idea here is that there is no good way for a client to know
when a subsequent OnDemand call can be issued without getting
an Error::ERROR_UPDATE_IN_PROGRESS. This issue showed up when writing a
browser test for an unrelated component updater feature.

This change is mostly mechanical, especially for the existing users
of the OnDemand interface. All existing call sites pass a null callback
as an argument, therefore no new code paths are introduced with this change.

BUG=639180
Committed: https://crrev.com/4c520183cb626cf66bbfad7c58a7e3e128a8accc
Cr-Commit-Position: refs/heads/master@{#413178}

[Sorin Jianu, 2016-08-19 03:38:43]

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

[commit-bot: I haz the power, 2016-08-19 03:39:13]

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

[Sorin Jianu, 2016-08-19 03:40:32]

sorin@chromium.org changed reviewers:
+ bauerb@chromium.org, waffles@chromium.org

[Sorin Jianu, 2016-08-19 03:40:33]

Bernhard, please I need an owner's approval for the paths under chrome/browser.
Thank you!

[Bernhard Bauer, 2016-08-19 04:13:05]

https://codereview.chromium.org/2261533003/diff/1/chrome/browser/component_up...
File chrome/browser/component_updater/supervised_user_whitelist_installer.cc
(right):

https://codereview.chromium.org/2261533003/diff/1/chrome/browser/component_up...
chrome/browser/component_updater/supervised_user_whitelist_installer.cc:645:
updater->OnDemandUpdate(crx_id, ComponentUpdateService::CompletionCallback());
Could we DCHECK the result in a callback here? If you don't want to add the
overhead for a release build, feel free to pass the null callback if DCHECKs are
compiled out.

https://codereview.chromium.org/2261533003/diff/1/components/component_update...
File components/component_updater/component_updater_service.cc (right):

https://codereview.chromium.org/2261533003/diff/1/components/component_update...
components/component_updater/component_updater_service.cc:380: if
(static_cast<bool>(callback)) {
Just `if (callback)`? Or if you want to be explicit, !callback.is_null().

[Sorin Jianu, 2016-08-19 04:23:36]

thank you!

https://codereview.chromium.org/2261533003/diff/1/chrome/browser/component_up...
File chrome/browser/component_updater/supervised_user_whitelist_installer.cc
(right):

https://codereview.chromium.org/2261533003/diff/1/chrome/browser/component_up...
chrome/browser/component_updater/supervised_user_whitelist_installer.cc:645:
updater->OnDemandUpdate(crx_id, ComponentUpdateService::CompletionCallback());
On 2016/08/19 04:13:05, Bernhard Bauer wrote:
> Could we DCHECK the result in a callback here? If you don't want to add the
> overhead for a release build, feel free to pass the null callback if DCHECKs
are
> compiled out.

I will try to add the callback. I am not concerned about the overhead, I just
tried to keep the change mechanical for the existing callers, and hopefully,
people would take advantage of the callbacks.

https://codereview.chromium.org/2261533003/diff/1/components/component_update...
File components/component_updater/component_updater_service.cc (right):

https://codereview.chromium.org/2261533003/diff/1/components/component_update...
components/component_updater/component_updater_service.cc:380: if
(static_cast<bool>(callback)) {
On 2016/08/19 04:13:05, Bernhard Bauer wrote:
> Just `if (callback)`? Or if you want to be explicit, !callback.is_null().

Callback has an explicit conversion to bool. Will change the code to use the
null test.

[Sorin Jianu, 2016-08-19 04:40:55]

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

[commit-bot: I haz the power, 2016-08-19 04:41:15]

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

[Sorin Jianu, 2016-08-19 04:43:37]

Thank you, PTAL!

https://codereview.chromium.org/2261533003/diff/1/chrome/browser/component_up...
File chrome/browser/component_updater/supervised_user_whitelist_installer.cc
(right):

https://codereview.chromium.org/2261533003/diff/1/chrome/browser/component_up...
chrome/browser/component_updater/supervised_user_whitelist_installer.cc:645:
updater->OnDemandUpdate(crx_id, ComponentUpdateService::CompletionCallback());
On 2016/08/19 04:23:36, Sorin Jianu wrote:
> On 2016/08/19 04:13:05, Bernhard Bauer wrote:
> > Could we DCHECK the result in a callback here? If you don't want to add the
> > overhead for a release build, feel free to pass the null callback if DCHECKs
> are
> > compiled out.
> 
> I will try to add the callback. I am not concerned about the overhead, I just
> tried to keep the change mechanical for the existing callers, and hopefully,
> people would take advantage of the callbacks.

On a second thought, I prefer solving this in a future change list to keep the
mechanical aspect of this change. The DCHECK is mostly pedantic, as the existing
code returned a hardcoded true in the interesting case.

[Bernhard Bauer, 2016-08-19 04:45:24]

LGTM!

[commit-bot: I haz the power, 2016-08-19 06:18:03]

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

[commit-bot: I haz the power, 2016-08-19 06:18:04]

Dry run: This issue passed the CQ dry run.

[waffles, 2016-08-19 16:20:29]

lgtm

[Sorin Jianu, 2016-08-19 17:22:51]

The CQ bit was checked by sorin@chromium.org

[commit-bot: I haz the power, 2016-08-19 17:23:24]

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-08-19 17:28:53]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-08-19 17:31:50]

Description was changed from

==========
Provide a callback for Component Updater OnDemand calls.

The idea here is that there is no good way for a client to know
when a subsequent OnDemand call can be issued without getting
an Error::ERROR_UPDATE_IN_PROGRESS. This issue showed up when writing a
browser test for an unrelated component updater feature.

This change is mostly mechanical, especially for the existing users
of the OnDemand interface. All existing call sites pass a null callback
as an argument, therefore no new code paths are introduced with this change.

BUG=639180
==========

to

==========
Provide a callback for Component Updater OnDemand calls.

The idea here is that there is no good way for a client to know
when a subsequent OnDemand call can be issued without getting
an Error::ERROR_UPDATE_IN_PROGRESS. This issue showed up when writing a
browser test for an unrelated component updater feature.

This change is mostly mechanical, especially for the existing users
of the OnDemand interface. All existing call sites pass a null callback
as an argument, therefore no new code paths are introduced with this change.

BUG=639180

Committed: https://crrev.com/4c520183cb626cf66bbfad7c58a7e3e128a8accc
Cr-Commit-Position: refs/heads/master@{#413178}
==========

[commit-bot: I haz the power, 2016-08-19 17:31:51]

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