Reland "Fix sandbox::PolicyBase leak"

Reland "Fix sandbox::PolicyBase leak"

StartSandboxedProcess was missing a Release call, so this was always
leaking. Use scoped_refptr instead to fix that and prevent it from
happening in the future.

BUG=663019
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng

Review-Url: https://codereview.chromium.org/2646043002
Cr-Commit-Position: refs/heads/master@{#448371}
Committed: https://chromium.googlesource.com/chromium/src/+/17e392b12a3b8a59fc3ca9265d1d89cda2a5509d

[jbauman, 2017-01-19 23:57:27]

Description was changed from

==========
Reland "Fix sandbox::PolicyBase leak"

StartSandboxedProcess was missing a Release call, so this was always
leaking. Use scoped_refptr instead to fix that and prevent it from
happening in the future.

BUG=663019
==========

to

==========
Reland "Fix sandbox::PolicyBase leak"

StartSandboxedProcess was missing a Release call, so this was always
leaking. Use scoped_refptr instead to fix that and prevent it from
happening in the future.

BUG=663019
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
==========

[jbauman, 2017-01-19 23:57:33]

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

[commit-bot: I haz the power, 2017-01-19 23:58:32]

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, 2017-01-20 01:21:33]

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

[commit-bot: I haz the power, 2017-01-20 01:21:34]

Dry run: This issue passed the CQ dry run.

[jbauman, 2017-01-24 00:08:08]

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

[jbauman, 2017-01-24 00:08:09]

The only change from the previously landed version is that it leaks the
PolicyBase when there's no job object created.

[Will Harris, 2017-01-24 19:45:30]

On 2017/01/24 00:08:09, jbauman wrote:
> The only change from the previously landed version is that it leaks the
> PolicyBase when there's no job object created.

Thanks for doing this.

I'm seeing a few other small changes and it's really hard to compare vs
crrev.com/2316333003 - would it be possible to upload 2316333003 as a base
patchset then this on top? I'll keep looking to see if I can find the changes in
the meantime, there don't look like many

[Will Harris, 2017-01-24 19:47:32]

I found all the changes. will review just these.

https://codereview.chromium.org/2646043002/diff/1/sandbox/win/src/broker_serv...
File sandbox/win/src/broker_services.cc (right):

https://codereview.chromium.org/2646043002/diff/1/sandbox/win/src/broker_serv...
sandbox/win/src/broker_services.cc:462: policy_base->AddRef();
note: new change

https://codereview.chromium.org/2646043002/diff/1/sandbox/win/tests/common/co...
File sandbox/win/tests/common/controller.cc (right):

https://codereview.chromium.org/2646043002/diff/1/sandbox/win/tests/common/co...
sandbox/win/tests/common/controller.cc:242: if (release_policy_in_run_)
note: new change

[jbauman, 2017-02-02 00:39:18]

Ok, uploaded the original (except rebased very slightly) as patch set 2 and the
new version as patch set 3.

[Will Harris, 2017-02-02 00:42:13]

lgtm. sorry for slow review.

I think you need a reviewer for content/common/sandbox_init_win.cc as it seems
there is a bug in the OWNERS file that this is not included in the win sandbox
owners.

[jbauman, 2017-02-02 00:48:32]

jbauman@chromium.org changed reviewers:
+ jam@chromium.org

[jbauman, 2017-02-02 00:48:32]

jam@: content/ OWNERS review please

[Will Harris, 2017-02-02 00:50:58]

On 2017/02/02 00:42:13, Will Harris wrote:
> lgtm. sorry for slow review.
> 
> I think you need a reviewer for content/common/sandbox_init_win.cc as it seems
> there is a bug in the OWNERS file that this is not included in the win sandbox
> owners.

and the owners file is fixed in crrev.com/2676473002

[jam, 2017-02-06 18:40:52]

On 2017/02/02 00:48:32, jbauman wrote:
> jam@: content/ OWNERS review please

lgtm

[Will Harris, 2017-02-06 18:50:56]

The CQ bit was checked by wfh@chromium.org

[commit-bot: I haz the power, 2017-02-06 18:51:41]

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, 2017-02-06 20:23:24]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1486407056711430,
"parent_rev": "17c45669733a9c6acc7f3a0d095a92f118eb0e2a", "commit_rev":
"17e392b12a3b8a59fc3ca9265d1d89cda2a5509d"}

[commit-bot: I haz the power, 2017-02-06 20:23:57]

Description was changed from

==========
Reland "Fix sandbox::PolicyBase leak"

StartSandboxedProcess was missing a Release call, so this was always
leaking. Use scoped_refptr instead to fix that and prevent it from
happening in the future.

BUG=663019
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
==========

to

==========
Reland "Fix sandbox::PolicyBase leak"

StartSandboxedProcess was missing a Release call, so this was always
leaking. Use scoped_refptr instead to fix that and prevent it from
happening in the future.

BUG=663019
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng

Review-Url: https://codereview.chromium.org/2646043002
Cr-Commit-Position: refs/heads/master@{#448371}
Committed:
https://chromium.googlesource.com/chromium/src/+/17e392b12a3b8a59fc3ca9265d1d...
==========

[commit-bot: I haz the power, 2017-02-06 20:24:03]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/17e392b12a3b8a59fc3ca9265d1d...