Convert //chrome/installer from scoped_ptr to std::unique_ptr

Convert //chrome/installer from scoped_ptr to std::unique_ptr

BUG=554298
R=thestig@chromium.org

Committed: https://crrev.com/f5d315cecedf2bc83f7499d9c4311215db51ecc6
Cr-Commit-Position: refs/heads/master@{#386997}

[Lei Zhang, 2016-04-13 01:22:42]

Got red bots still.

https://codereview.chromium.org/1878313003/diff/1/chrome/installer/mini_insta...
File chrome/installer/mini_installer/decompress.cc (right):

https://codereview.chromium.org/1878313003/diff/1/chrome/installer/mini_insta...
chrome/installer/mini_installer/decompress.cc:88: std::unique_ptr<wchar_t>
path(Utf8ToWide(pszFile));
IWYU

[dcheng, 2016-04-13 01:24:32]

https://codereview.chromium.org/1878313003/diff/1/chrome/installer/mini_insta...
File chrome/installer/mini_installer/decompress.cc (right):

https://codereview.chromium.org/1878313003/diff/1/chrome/installer/mini_insta...
chrome/installer/mini_installer/decompress.cc:88: std::unique_ptr<wchar_t>
path(Utf8ToWide(pszFile));
On 2016/04/13 at 01:22:42, Lei Zhang wrote:
> IWYU

This file currently has a local copy of scoped_ptr. I'm guessing it's to try to
avoid pulling the C++ standard library, but I'm not sure. Either way, this was
the primary cause of redness: PTAL

[dcheng, 2016-04-13 01:24:59]

https://codereview.chromium.org/1878313003/diff/1/chrome/installer/mini_insta...
File chrome/installer/mini_installer/decompress.cc (right):

https://codereview.chromium.org/1878313003/diff/1/chrome/installer/mini_insta...
chrome/installer/mini_installer/decompress.cc:88: std::unique_ptr<wchar_t>
path(Utf8ToWide(pszFile));
On 2016/04/13 at 01:24:31, dcheng wrote:
> On 2016/04/13 at 01:22:42, Lei Zhang wrote:
> > IWYU
> 
> This file currently has a local copy of scoped_ptr. I'm guessing it's to try
to avoid pulling the C++ standard library, but I'm not sure. Either way, this
was the primary cause of redness: PTAL

(I restored this to use the locally defined scoped_ptr, sorry that's not clear
from the first message)

[Lei Zhang, 2016-04-13 01:44:16]

lgtm

https://codereview.chromium.org/1878313003/diff/1/chrome/installer/mini_insta...
File chrome/installer/mini_installer/decompress.cc (right):

https://codereview.chromium.org/1878313003/diff/1/chrome/installer/mini_insta...
chrome/installer/mini_installer/decompress.cc:88: std::unique_ptr<wchar_t>
path(Utf8ToWide(pszFile));
On 2016/04/13 01:24:59, dcheng wrote:
> On 2016/04/13 at 01:24:31, dcheng wrote:
> > On 2016/04/13 at 01:22:42, Lei Zhang wrote:
> > > IWYU
> > 
> > This file currently has a local copy of scoped_ptr. I'm guessing it's to try
> to avoid pulling the C++ standard library, but I'm not sure. Either way, this
> was the primary cause of redness: PTAL
> 
> (I restored this to use the locally defined scoped_ptr, sorry that's not clear
> from the first message)

I have a feeling mini_installer is self-contained and does not want to depend on
base/ or stdlib.

[dcheng, 2016-04-13 01:46:45]

The CQ bit was checked by dcheng@chromium.org

[commit-bot: I haz the power, 2016-04-13 01:46:53]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1878313003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1878313003/20001

[Will Harris, 2016-04-13 02:02:17]

Sorry but I think it's prudent to get l-g-t-m from chrome/installer owners
before committing this.

[Will Harris, 2016-04-13 02:02:21]

The CQ bit was unchecked by wfh@chromium.org

[Will Harris, 2016-04-13 02:06:23]

Description was changed from

==========
Convert //chrome/installer from scoped_ptr to std::unique_ptr

BUG=554298
R=thestig@chromium.org
==========

to

==========
Convert //chrome/installer from scoped_ptr to std::unique_ptr

BUG=554298
R=thestig@chromium.org
==========

[Will Harris, 2016-04-13 02:06:23]

wfh@chromium.org changed reviewers:
+ grt@chromium.org

[dcheng, 2016-04-13 02:22:28]

wfh@, are there guidelines for when I need a review specifically from a
//chrome/installer OWNER?

This CL is extremely boring:
- scoped_ptr (from base) is already a typedef for std::unique_ptr, so there's no
behavior change.
- duplicate copies of scoped_ptr haven't been changed
- it doesn't add any new dependencies (since scoped_ptr is a typedef, the
dependency on <memory> is already there)

[Will Harris, 2016-04-13 02:26:53]

On 2016/04/13 02:22:28, dcheng wrote:
> wfh@, are there guidelines for when I need a review specifically from a
> //chrome/installer OWNER?
> 
> This CL is extremely boring:
> - scoped_ptr (from base) is already a typedef for std::unique_ptr, so there's
no
> behavior change.
> - duplicate copies of scoped_ptr haven't been changed
> - it doesn't add any new dependencies (since scoped_ptr is a typedef, the
> dependency on <memory> is already there)

I seem to recall something special about mini_installer code, I think it has to
compile on other machines, perhaps the signing machines...

I think in general if you've taken the time to split a CL down to a particular
sub component then there's no harm in asking the owners of that component for
review?

If I'm correct about the mini_installer idiosyncrasies, it makes sense to have a
noparent rule in the OWNERS. I'll let grt@ comment on that.

[grt (UTC plus 2), 2016-04-13 15:27:45]

On 2016/04/13 02:26:53, Will Harris wrote:
> On 2016/04/13 02:22:28, dcheng wrote:
> > wfh@, are there guidelines for when I need a review specifically from a
> > //chrome/installer OWNER?
> > 
> > This CL is extremely boring:
> > - scoped_ptr (from base) is already a typedef for std::unique_ptr, so
there's
> no
> > behavior change.
> > - duplicate copies of scoped_ptr haven't been changed
> > - it doesn't add any new dependencies (since scoped_ptr is a typedef, the
> > dependency on <memory> is already there)
> 
> I seem to recall something special about mini_installer code, I think it has
to
> compile on other machines, perhaps the signing machines...

For the moment, mini_installer still needs to build with ancient toolchains.
This is being resolved by mmoss.

> I think in general if you've taken the time to split a CL down to a particular
> sub component then there's no harm in asking the owners of that component for
> review?
> 
> If I'm correct about the mini_installer idiosyncrasies, it makes sense to have
a
> noparent rule in the OWNERS. I'll let grt@ comment on that.

I wouldn't be opposed. Even once the toolchain issue is resolved, we don't want
to add dependencies to mini_installer.

[grt (UTC plus 2), 2016-04-13 15:29:48]

oh, and lgtm. thanks for updating setup and util.

[dcheng, 2016-04-13 15:32:48]

The CQ bit was checked by dcheng@chromium.org

[commit-bot: I haz the power, 2016-04-13 15:33:08]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1878313003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1878313003/20001

[commit-bot: I haz the power, 2016-04-13 15:39:58]

Description was changed from

==========
Convert //chrome/installer from scoped_ptr to std::unique_ptr

BUG=554298
R=thestig@chromium.org
==========

to

==========
Convert //chrome/installer from scoped_ptr to std::unique_ptr

BUG=554298
R=thestig@chromium.org
==========

[commit-bot: I haz the power, 2016-04-13 15:40:00]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-04-13 15:40:51]

Description was changed from

==========
Convert //chrome/installer from scoped_ptr to std::unique_ptr

BUG=554298
R=thestig@chromium.org
==========

to

==========
Convert //chrome/installer from scoped_ptr to std::unique_ptr

BUG=554298
R=thestig@chromium.org

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

[commit-bot: I haz the power, 2016-04-13 15:40:53]

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