Makes the component installers return a Result instead of a bool.

Makes the component installers return a Result instead of a bool.

This allows conveying more error information from the component
installers to other layers of the component updater.

Before this change, the component install execution path could
only return one error. We are trying to understand the reasons
why the component installers fail.

BUG=615669
Committed: https://crrev.com/73769a84d74113a4a10e4152e63d219eb72f2630
Committed: https://crrev.com/2892f721ec000a72ace40512c19f8b2f2e512c37
Cr-Original-Commit-Position: refs/heads/master@{#429995}
Cr-Commit-Position: refs/heads/master@{#430333}

[Sorin Jianu, 2016-11-03 23:05:24]

The CQ bit was checked by [email protected] to run a CQ dry run

[Sorin Jianu, 2016-11-03 23:05:45]

[email protected] changed reviewers:
+ [email protected]

[Sorin Jianu, 2016-11-03 23:05:45]

PTAL thank you!

[commit-bot: I haz the power, 2016-11-03 23:06:05]

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

[waffles, 2016-11-04 00:43:56]

lgtm

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

https://codereview.chromium.org/2479633003/diff/1/chrome/browser/component_up...
chrome/browser/component_updater/file_type_policies_component_installer.cc:73:
return update_client::CrxInstaller::Result(0);  // Nothing custom here.
Elsewhere, we use InstallerError::None; should that be done here?

(I'm OK with 0 if you prefer.)

[Sorin Jianu, 2016-11-04 00:55:19]

Description was changed from

==========
Makes the component installers return a Result instead of a bool.

This allows conveying more error information from the component
installers to other layers of the component updater.

Before this change, the component install execution path could
only return one error. We are trying to understand the reasons
why the component installers fail.

BUG=615669
==========

to

==========
Makes the component installers return a Result instead of a bool.

This allows conveying more error information from the component
installers to other layers of the component updater.

Before this change, the component install execution path could
only return one error. We are trying to understand the reasons
why the component installers fail.

BUG=615669
==========

[Sorin Jianu, 2016-11-04 00:55:20]

[email protected] changed reviewers:
+ [email protected], [email protected], [email protected],
[email protected]

[Sorin Jianu, 2016-11-04 00:59:18]

Hi,
I need a bunch of owner's approval for changes that are mechanical, especially
the ones outside the component updater code itself.

agl@, PTAL for changes under chrome/browser/net
asargent@ PTAL for changes under extensions/
sdefresne@ PTAL for changes under ios/
pastarmovj@ PTAL for changes under chrome/browser/policies

Thank you so much!

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

https://codereview.chromium.org/2479633003/diff/1/chrome/browser/component_up...
chrome/browser/component_updater/file_type_policies_component_installer.cc:73:
return update_client::CrxInstaller::Result(0);  // Nothing custom here.
On 2016/11/04 00:43:56, waffles wrote:
> Elsewhere, we use InstallerError::None; should that be done here?
> 
> (I'm OK with 0 if you prefer.)

At similar call sites it uses the integral 0 since it avoids including the error
definition file.

[commit-bot: I haz the power, 2016-11-04 04:03:54]

The CQ bit was unchecked by [email protected]

[commit-bot: I haz the power, 2016-11-04 04:03:55]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[pastarmovj, 2016-11-04 10:05:02]

[email protected] changed reviewers:
+ [email protected]

[pastarmovj, 2016-11-04 10:05:03]

**/policy lgtm

[asargent_no_longer_on_chrome, 2016-11-04 15:55:15]

extensions lgtm

[agl, 2016-11-04 16:15:50]

LGTM

[sdefresne, 2016-11-04 16:27:52]

ios/ lgtm

[Sorin Jianu, 2016-11-04 17:39:18]

The CQ bit was checked by [email protected]

[commit-bot: I haz the power, 2016-11-04 17:39:55]

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-11-04 20:19:10]

Description was changed from

==========
Makes the component installers return a Result instead of a bool.

This allows conveying more error information from the component
installers to other layers of the component updater.

Before this change, the component install execution path could
only return one error. We are trying to understand the reasons
why the component installers fail.

BUG=615669
==========

to

==========
Makes the component installers return a Result instead of a bool.

This allows conveying more error information from the component
installers to other layers of the component updater.

Before this change, the component install execution path could
only return one error. We are trying to understand the reasons
why the component installers fail.

BUG=615669
==========

[commit-bot: I haz the power, 2016-11-04 20:19:12]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-11-04 20:21:57]

Description was changed from

==========
Makes the component installers return a Result instead of a bool.

This allows conveying more error information from the component
installers to other layers of the component updater.

Before this change, the component install execution path could
only return one error. We are trying to understand the reasons
why the component installers fail.

BUG=615669
==========

to

==========
Makes the component installers return a Result instead of a bool.

This allows conveying more error information from the component
installers to other layers of the component updater.

Before this change, the component install execution path could
only return one error. We are trying to understand the reasons
why the component installers fail.

BUG=615669

Committed: https://crrev.com/73769a84d74113a4a10e4152e63d219eb72f2630
Cr-Commit-Position: refs/heads/master@{#429995}
==========

[commit-bot: I haz the power, 2016-11-04 20:21:59]

Patchset 1 (id:??) landed as
https://crrev.com/73769a84d74113a4a10e4152e63d219eb72f2630
Cr-Commit-Position: refs/heads/master@{#429995}

[Avi (use Gerrit), 2016-11-04 20:44:26]

A revert of this CL (patchset #1 id:1) has been created in
https://codereview.chromium.org/2483513002/ by [email protected].

The reason for reverting is: Breaks official build.

http://build.chromium.org/p/chromium.chrome/builders/Google%20Chrome%20Mac/bu...

FAILED: obj/chrome/browser/browser/pepper_flash_component_installer.o 
/b/c/cipd/goma/gomacc ../../third_party/llvm-build/Release+Asserts/bin/clang++
-MMD -MF obj/chrome/...-inlines-hidden -std=c++11 -stdlib=libc++ -fno-rtti
-fno-exceptions -c
../../chrome/browser/component_updater/pepper_flash_component_installer.cc -o
obj/chrome/browser/browser/pepper_flash_component_installer.o
../../chrome/browser/component_updater/pepper_flash_component_installer.cc:162:8:
error: virtual function 'OnCustomInstall' has a different return type ('bool')
than the function it overrides (which has return type
'update_client::CrxInstaller::Result')
  bool OnCustomInstall(const base::DictionaryValue& manifest,
  ~~~~ ^
../../components/component_updater/default_component_installer.h:61:47: note:
overridden virtual function is here
  virtual update_client::CrxInstaller::Result OnCustomInstall(
          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^
1 error generated.
.

[findit-for-me, 2016-11-04 20:58:17]

FYI: Findit identified this CL at revision 429995 as the culprit for
failures in the build cycles as shown on:
https://findit-for-me.appspot.com/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3Itb...

[Sorin Jianu, 2016-11-04 21:31:08]

Fixed Flash build break. Not sure why the try bot run is not including the
official Chrome build bot.

[Sorin Jianu, 2016-11-04 21:41:30]

Description was changed from

==========
Makes the component installers return a Result instead of a bool.

This allows conveying more error information from the component
installers to other layers of the component updater.

Before this change, the component install execution path could
only return one error. We are trying to understand the reasons
why the component installers fail.

BUG=615669

Committed: https://crrev.com/73769a84d74113a4a10e4152e63d219eb72f2630
Cr-Commit-Position: refs/heads/master@{#429995}
==========

to

==========
Makes the component installers return a Result instead of a bool.

This allows conveying more error information from the component
installers to other layers of the component updater.

Before this change, the component install execution path could
only return one error. We are trying to understand the reasons
why the component installers fail.

BUG=615669

Committed: https://crrev.com/73769a84d74113a4a10e4152e63d219eb72f2630
Cr-Commit-Position: refs/heads/master@{#429995}
==========

[Sorin Jianu, 2016-11-04 21:41:42]

The CQ bit was checked by [email protected]

[Sorin Jianu, 2016-11-04 21:41:43]

The patchset sent to the CQ was uploaded after l-g-t-m from
[email protected], [email protected], [email protected],
[email protected], [email protected]
Link to the patchset: https://codereview.chromium.org/2479633003/#ps80001
(title: "rebase")

[Sorin Jianu, 2016-11-04 21:41:43]

The CQ bit was unchecked by [email protected]

[Sorin Jianu, 2016-11-04 21:41:50]

The CQ bit was checked by [email protected] to run a CQ dry run

[commit-bot: I haz the power, 2016-11-04 21:42:57]

Dry run: 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-11-04 23:06:42]

The CQ bit was unchecked by [email protected]

[commit-bot: I haz the power, 2016-11-04 23:06:44]

Dry run: This issue passed the CQ dry run.

[Sorin Jianu, 2016-11-07 17:51:43]

The CQ bit was checked by [email protected]

[Sorin Jianu, 2016-11-07 17:51:44]

The patchset sent to the CQ was uploaded after l-g-t-m from
[email protected], [email protected], [email protected],
[email protected], [email protected]
Link to the patchset: https://codereview.chromium.org/2479633003/#ps80001
(title: "rebase")

[commit-bot: I haz the power, 2016-11-07 17:52:10]

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-11-07 19:00:13]

Description was changed from

==========
Makes the component installers return a Result instead of a bool.

This allows conveying more error information from the component
installers to other layers of the component updater.

Before this change, the component install execution path could
only return one error. We are trying to understand the reasons
why the component installers fail.

BUG=615669

Committed: https://crrev.com/73769a84d74113a4a10e4152e63d219eb72f2630
Cr-Commit-Position: refs/heads/master@{#429995}
==========

to

==========
Makes the component installers return a Result instead of a bool.

This allows conveying more error information from the component
installers to other layers of the component updater.

Before this change, the component install execution path could
only return one error. We are trying to understand the reasons
why the component installers fail.

BUG=615669

Committed: https://crrev.com/73769a84d74113a4a10e4152e63d219eb72f2630
Cr-Commit-Position: refs/heads/master@{#429995}
==========

[commit-bot: I haz the power, 2016-11-07 19:00:14]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-11-07 19:03:30]

Description was changed from

==========
Makes the component installers return a Result instead of a bool.

This allows conveying more error information from the component
installers to other layers of the component updater.

Before this change, the component install execution path could
only return one error. We are trying to understand the reasons
why the component installers fail.

BUG=615669

Committed: https://crrev.com/73769a84d74113a4a10e4152e63d219eb72f2630
Cr-Commit-Position: refs/heads/master@{#429995}
==========

to

==========
Makes the component installers return a Result instead of a bool.

This allows conveying more error information from the component
installers to other layers of the component updater.

Before this change, the component install execution path could
only return one error. We are trying to understand the reasons
why the component installers fail.

BUG=615669

Committed: https://crrev.com/73769a84d74113a4a10e4152e63d219eb72f2630
Committed: https://crrev.com/2892f721ec000a72ace40512c19f8b2f2e512c37
Cr-Original-Commit-Position: refs/heads/master@{#429995}
Cr-Commit-Position: refs/heads/master@{#430333}
==========

[commit-bot: I haz the power, 2016-11-07 19:03:31]

Patchset 5 (id:??) landed as
https://crrev.com/2892f721ec000a72ace40512c19f8b2f2e512c37
Cr-Commit-Position: refs/heads/master@{#430333}