Convert //sandbox to use std::unique_ptr

Convert //sandbox to use std::unique_ptr

BUG=554298
Committed: https://crrev.com/d521b63cb0fe3d78ed64bbba2c4cfacfeb7ebfd3
Cr-Commit-Position: refs/heads/master@{#385445}

[Mostyn Bramley-Moore, 2016-04-01 19:35:22]

The CQ bit was checked by mostynb@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2016-04-01 19:35:44]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1849323003/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1849323003/1

[Mostyn Bramley-Moore, 2016-04-01 19:36:57]

mostynb@opera.com changed reviewers:
+ cpu@chromium.org

[Mostyn Bramley-Moore, 2016-04-01 19:36:57]

@cpu: please take a look.

[commit-bot: I haz the power, 2016-04-01 20:02:42]

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

[commit-bot: I haz the power, 2016-04-01 20:02:43]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)

[Mostyn Bramley-Moore, 2016-04-01 20:22:19]

The CQ bit was checked by mostynb@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2016-04-01 20:23:01]

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

[commit-bot: I haz the power, 2016-04-01 21:32:58]

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

[commit-bot: I haz the power, 2016-04-01 21:32:59]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Mostyn Bramley-Moore, 2016-04-01 21:48:09]

The CQ bit was checked by mostynb@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2016-04-01 21:49:04]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1849323003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1849323003/40001

[commit-bot: I haz the power, 2016-04-01 23:50:35]

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

[commit-bot: I haz the power, 2016-04-01 23:50:36]

Dry run: This issue passed the CQ dry run.

[jln (very slow on Chromium), 2016-04-04 20:15:38]

Description was changed from

==========
Convert //sandbox to use std::unique_ptr

BUG=554298
==========

to

==========
Convert //sandbox to use std::unique_ptr

BUG=554298
==========

[jln (very slow on Chromium), 2016-04-04 20:15:38]

jln@chromium.org changed reviewers:
+ mdempsky@chromium.org

[jln (very slow on Chromium), 2016-04-04 20:16:32]

Matthew, would you have time to take a look?

(cpu or myself can rubberstamp if you're happy with it)

[Will Harris, 2016-04-04 20:20:24]

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

[Will Harris, 2016-04-04 20:20:25]

https://codereview.chromium.org/1849323003/diff/40001/sandbox/win/src/acl.cc
File sandbox/win/src/acl.cc (right):

https://codereview.chromium.org/1849323003/diff/40001/sandbox/win/src/acl.cc#...
sandbox/win/src/acl.cc:11: #include "base/memory/free_deleter.h"
general comment - why not include <memory> here and in other CC files - I
thought chromium style was to always explicit include everything even if you
know it's included by the header file for the cc file? (or maybe I am just wrong
about chromium style)

[Mostyn Bramley-Moore, 2016-04-04 20:48:35]

mostynb@opera.com changed reviewers:
+ dcheng@chromium.org

[Mostyn Bramley-Moore, 2016-04-04 20:48:36]

https://codereview.chromium.org/1849323003/diff/40001/sandbox/win/src/acl.cc
File sandbox/win/src/acl.cc (right):

https://codereview.chromium.org/1849323003/diff/40001/sandbox/win/src/acl.cc#...
sandbox/win/src/acl.cc:11: #include "base/memory/free_deleter.h"
On 2016/04/04 20:20:25, Will Harris wrote:
> general comment - why not include <memory> here and in other CC files - I
> thought chromium style was to always explicit include everything even if you
> know it's included by the header file for the cc file? (or maybe I am just
wrong
> about chromium style)

I can't find any discussion of this style preference in either the chromium
coding style or google c++ style guide document, but maybe it's there and I
missed it.

My preference is to skip the extra include for now (this change shouldn't
regress  the include-what-you-use situation), and consider applying IWYU more
widely/consistently across the tree later if the tool becomes mature enough.

Maybe dcheng has an opinion?

[mdempsky, 2016-04-04 20:53:56]

lgtm for sandbox/linux

[dcheng, 2016-04-04 20:58:56]

On 2016/04/04 at 20:48:36, mostynb wrote:
> https://codereview.chromium.org/1849323003/diff/40001/sandbox/win/src/acl.cc
> File sandbox/win/src/acl.cc (right):
> 
>
https://codereview.chromium.org/1849323003/diff/40001/sandbox/win/src/acl.cc#...
> sandbox/win/src/acl.cc:11: #include "base/memory/free_deleter.h"
> On 2016/04/04 20:20:25, Will Harris wrote:
> > general comment - why not include <memory> here and in other CC files - I
> > thought chromium style was to always explicit include everything even if you
> > know it's included by the header file for the cc file? (or maybe I am just
wrong
> > about chromium style)
> 
> I can't find any discussion of this style preference in either the chromium
coding style or google c++ style guide document, but maybe it's there and I
missed it.
> 
> My preference is to skip the extra include for now (this change shouldn't
regress  the include-what-you-use situation), and consider applying IWYU more
widely/consistently across the tree later if the tool becomes mature enough.
> 
> Maybe dcheng has an opinion?

The general rule is if the header file for an implementation file includes it,
then you can assume it and not repeat the #include. The nuances of actually
interpreting this usually end up depending on the reviewer, which means there's
no actual consistency across the codebase. Do what seems reasonable.

[Will Harris, 2016-04-04 21:05:57]

by the golden dcheng rule there is one #include missing?

otherwise lgtm for sandbox/win

https://codereview.chromium.org/1849323003/diff/40001/sandbox/win/src/address...
File sandbox/win/src/address_sanitizer_test.cc (right):

https://codereview.chromium.org/1849323003/diff/40001/sandbox/win/src/address...
sandbox/win/src/address_sanitizer_test.cc:5: #include <stdio.h>
memory.h

[mdempsky, 2016-04-04 21:40:34]

mdempsky@chromium.org changed reviewers:
+ rsesek@chromium.org

[mdempsky, 2016-04-04 21:40:35]

sandbox/mac LGTM too (though I'm only a sandbox/linux OWNER)

rsesek/cpu: Please review (or RSLGTM if you trust me :)) for sandbox/mac.

[Robert Sesek, 2016-04-04 21:43:05]

sandbox/mac LGTM

[jln (very slow on Chromium), 2016-04-04 23:04:42]

overall LGTM then based on previous reviews.

[Mostyn Bramley-Moore, 2016-04-05 07:11:00]

Thanks.

https://codereview.chromium.org/1849323003/diff/40001/sandbox/win/src/address...
File sandbox/win/src/address_sanitizer_test.cc (right):

https://codereview.chromium.org/1849323003/diff/40001/sandbox/win/src/address...
sandbox/win/src/address_sanitizer_test.cc:5: #include <stdio.h>
On 2016/04/04 21:05:57, Will Harris wrote:
> memory.h

Done.

[Mostyn Bramley-Moore, 2016-04-05 07:11:11]

The CQ bit was checked by mostynb@opera.com

[Mostyn Bramley-Moore, 2016-04-05 07:11:12]

The patchset sent to the CQ was uploaded after l-g-t-m from jln@chromium.org,
rsesek@chromium.org, mdempsky@chromium.org, wfh@chromium.org
Link to the patchset: https://codereview.chromium.org/1849323003/#ps60001
(title: "add missing include")

[commit-bot: I haz the power, 2016-04-05 07:11:27]

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

[commit-bot: I haz the power, 2016-04-05 07:22:08]

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

[commit-bot: I haz the power, 2016-04-05 07:22:10]

Try jobs failed on following builders:
  chromeos_amd64-generic_chromium_compile_only_ng on tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromeos_x86-generic_chromium_compile_only_ng on tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_x86-ge...)

[Mostyn Bramley-Moore, 2016-04-05 14:31:16]

The CQ bit was checked by mostynb@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2016-04-05 14:31:29]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1849323003/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1849323003/80001

[commit-bot: I haz the power, 2016-04-05 14:50:19]

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

[commit-bot: I haz the power, 2016-04-05 14:50:20]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Mostyn Bramley-Moore, 2016-04-05 21:36:36]

The CQ bit was checked by mostynb@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2016-04-05 21:37:12]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1849323003/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1849323003/100001

[commit-bot: I haz the power, 2016-04-05 22:47:40]

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

[commit-bot: I haz the power, 2016-04-05 22:47:42]

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

[commit-bot: I haz the power, 2016-04-05 22:47:43]

Dry run: This issue passed the CQ dry run.

[Mostyn Bramley-Moore, 2016-04-05 22:59:19]

mostynb@opera.com changed reviewers:
+ hidehiko@chromium.org, rickyz@chromium.org

[Mostyn Bramley-Moore, 2016-04-05 22:59:58]

@rickyz: can you please take a look at mojo/shell/runner/host/linux_sandbox.h ?
@hidehiko: can you please look at
components/nacl/loader/nonsfi/nonsfi_sandbox_unittest.cc ?

[rickyz (no longer on Chrome), 2016-04-05 23:00:52]

rickyz@chromium.org changed reviewers:
+ jln@chromium.org

[rickyz (no longer on Chrome), 2016-04-05 23:00:53]

mojo/shell/runner/host/linux_sandbox.h lgtm, thanks!

[hidehiko, 2016-04-06 13:05:31]

components/nacl/loader/nonsfi/ lgtm. Thank you for clean up!

[Mostyn Bramley-Moore, 2016-04-06 13:16:04]

The CQ bit was checked by mostynb@opera.com

[Mostyn Bramley-Moore, 2016-04-06 13:16:04]

The patchset sent to the CQ was uploaded after l-g-t-m from jln@chromium.org,
rsesek@chromium.org, mdempsky@chromium.org, wfh@chromium.org
Link to the patchset: https://codereview.chromium.org/1849323003/#ps100001
(title: "fixup nonsfi_sandbox_unittest.cc")

[commit-bot: I haz the power, 2016-04-06 13:16:14]

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

[commit-bot: I haz the power, 2016-04-06 13:22:16]

Description was changed from

==========
Convert //sandbox to use std::unique_ptr

BUG=554298
==========

to

==========
Convert //sandbox to use std::unique_ptr

BUG=554298
==========

[commit-bot: I haz the power, 2016-04-06 13:22:18]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-04-06 13:23:55]

Description was changed from

==========
Convert //sandbox to use std::unique_ptr

BUG=554298
==========

to

==========
Convert //sandbox to use std::unique_ptr

BUG=554298

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

[commit-bot: I haz the power, 2016-04-06 13:23:56]

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