Make ComponentInstaller refcounted.

Make ComponentInstaller refcounted.

Before this CL, component installers were leaked in almost all cases. If we allow uninstalling components (see https://codereview.chromium.org/879993005/), we need to fix those leaks.

BUG=436459
Committed: https://crrev.com/810e60f487982e8edc208b78b0b4b2b2a12a77a0
Cr-Commit-Position: refs/heads/master@{#314701}

[Bernhard Bauer, 2015-02-04 11:28:04]

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

[Bernhard Bauer, 2015-02-04 11:28:04]

As discussed, please review.

[Sorin Jianu, 2015-02-04 18:35:50]

lgtm

Nice, thank you!

[Bernhard Bauer, 2015-02-04 21:54:45]

bauerb@chromium.org changed reviewers:
+ agl@chromium.org, thestig@chromium.org

[Bernhard Bauer, 2015-02-04 21:54:46]

Adding OWNERS.
Lei: c/b/browser_process_impl.*
Adam: c/b/n/crl_set_fetcher.h

[Lei Zhang, 2015-02-04 22:21:52]

Can you elaborate on how they are leaking? Looking at the
|pnacl_component_installer_| example, we never pass or release the pointer, so
how does it get leaked?

https://codereview.chromium.org/897873002/diff/1/chrome/browser/browser_proce...
File chrome/browser/browser_process_impl.cc (right):

https://codereview.chromium.org/897873002/diff/1/chrome/browser/browser_proce...
chrome/browser/browser_process_impl.cc:881: if (!crl_set_fetcher_)
Can we keep the .get() for
https://code.google.com/p/chromium/issues/detail?id=110610 ?

[Bernhard Bauer, 2015-02-04 22:50:55]

On 2015/02/04 22:21:52, Lei Zhang wrote:
> Can you elaborate on how they are leaking? Looking at the
> |pnacl_component_installer_| example, we never pass or release the pointer, so
> how does it get leaked?

Ha, the component installers in browser_process_impl are among the few cases
where the installer *isn't* leaked :-) Other components
(PNaClComponentInstaller, RecoveryComponentInstaller,
SwiftShaderComponentInstaller, and everything using DefaultComponentInstaller)
are leaked though.

The model where the client owns the component installer doesn't work in general,
because an installer might be busy at any given point, so the owner can't
actually destroy it whenever it wants, and that really means by definition that
it doesn't actually own it. It only works if the component is destroyed very
late (e.g. at browser_process shutdown).

https://codereview.chromium.org/897873002/diff/1/chrome/browser/browser_proce...
File chrome/browser/browser_process_impl.cc (right):

https://codereview.chromium.org/897873002/diff/1/chrome/browser/browser_proce...
chrome/browser/browser_process_impl.cc:881: if (!crl_set_fetcher_)
On 2015/02/04 22:21:52, Lei Zhang wrote:
> Can we keep the .get() for
> https://code.google.com/p/chromium/issues/detail?id=110610 ?

I thought this is already fixed (see https://codereview.chromium.org/759503002),
so we're using the bool conversion here?

[Lei Zhang, 2015-02-04 23:39:57]

browser_process_impl.* lgtm

https://codereview.chromium.org/897873002/diff/1/chrome/browser/browser_proce...
File chrome/browser/browser_process_impl.cc (right):

https://codereview.chromium.org/897873002/diff/1/chrome/browser/browser_proce...
chrome/browser/browser_process_impl.cc:881: if (!crl_set_fetcher_)
On 2015/02/04 22:50:55, Bernhard Bauer wrote:
> On 2015/02/04 22:21:52, Lei Zhang wrote:
> > Can we keep the .get() for
> > https://code.google.com/p/chromium/issues/detail?id=110610 ?
> 
> I thought this is already fixed (see
https://codereview.chromium.org/759503002),
> so we're using the bool conversion here?

Err, yes. n/m.

[agl, 2015-02-04 23:46:05]

LGTM for CRLSet change.

[Bernhard Bauer, 2015-02-04 23:58:13]

New patchsets have been uploaded after l-g-t-m from
sorin@chromium.org,thestig@chromium.org,agl@chromium.org

[Bernhard Bauer, 2015-02-05 00:01:20]

Patchset #2 (id:20001) has been deleted

[Bernhard Bauer, 2015-02-05 00:10:25]

Patchset #2 (id:40001) has been deleted

[Bernhard Bauer, 2015-02-05 00:11:04]

The CQ bit was checked by bauerb@chromium.org

[commit-bot: I haz the power, 2015-02-05 00:11:43]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/897873002/60001

[commit-bot: I haz the power, 2015-02-05 01:09:39]

Committed patchset #2 (id:60001)

[commit-bot: I haz the power, 2015-02-05 01:10:25]

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