Use scoped_refptr and RefCountedThreadSafe for TargetPolicy.

Use scoped_refptr and RefCountedThreadSafe for TargetPolicy.

BUG=549319

[rickyz (no longer on Chrome), 2015-09-29 06:52:40]

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

[rickyz (no longer on Chrome), 2015-10-05 19:42:15]

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

[Will Harris, 2015-10-05 23:30:59]

This CL confuses me a bit: TargetPolicy (base class of PolicyBase) already
implements its own refcounting via AddRef() and Release() which this CL intends
to replace?

If so, why not remove all the refcounting in the base class? It seems we are
mixing scoped_refptr with the TargetPolicy ref counting here? Maybe I need to
fully understand what the intent here is.

https://codereview.chromium.org/1378523002/diff/20001/sandbox/win/sandbox_poc...
File sandbox/win/sandbox_poc/main_ui_window.h (right):

https://codereview.chromium.org/1378523002/diff/20001/sandbox/win/sandbox_poc...
sandbox/win/sandbox_poc/main_ui_window.h:8: #include <windows.h>
nit: needed?

https://codereview.chromium.org/1378523002/diff/20001/sandbox/win/sandbox_poc...
sandbox/win/sandbox_poc/main_ui_window.h:12: #include "base/macros.h"
nit: needed?

https://codereview.chromium.org/1378523002/diff/20001/sandbox/win/src/target_...
File sandbox/win/src/target_process.h (right):

https://codereview.chromium.org/1378523002/diff/20001/sandbox/win/src/target_...
sandbox/win/src/target_process.h:11: #include "base/memory/ref_counted.h"
nit: needed?

[rickyz (no longer on Chrome), 2015-10-08 21:36:51]

On 2015/10/05 at 23:30:59, wfh wrote:
> This CL confuses me a bit: TargetPolicy (base class of PolicyBase) already
implements its own refcounting via AddRef() and Release() which this CL intends
to replace?
scoped_refptr is kind of weird in that it's designed to support classes which
implement their own AddRef/Release methods, as long as the follow the same
conventions as RefCounted (like starting the refcount at 0). 

> If so, why not remove all the refcounting in the base class? It seems we are
mixing scoped_refptr with the TargetPolicy ref counting here? Maybe I need to
fully understand what the intent here is.
I'm leaving the refcounting in (even though we don't currently use it) because I
eventually want to use it - specifically, once I invert the ownership between
TargetPolicy and TargetProcess, I plan to have each TargetPolicy ref the
corresponding TargetPolicy.

[rickyz (no longer on Chrome), 2015-10-28 22:29:06]

On 2015/10/08 at 21:36:51, rickyz wrote:
> On 2015/10/05 at 23:30:59, wfh wrote:
> > This CL confuses me a bit: TargetPolicy (base class of PolicyBase) already
implements its own refcounting via AddRef() and Release() which this CL intends
to replace?
> scoped_refptr is kind of weird in that it's designed to support classes which
implement their own AddRef/Release methods, as long as the follow the same
conventions as RefCounted (like starting the refcount at 0). 
> 
> > If so, why not remove all the refcounting in the base class? It seems we are
mixing scoped_refptr with the TargetPolicy ref counting here? Maybe I need to
fully understand what the intent here is.
> I'm leaving the refcounting in (even though we don't currently use it) because
I eventually want to use it - specifically, once I invert the ownership between
TargetPolicy and TargetProcess, I plan to have each TargetPolicy ref the
corresponding TargetPolicy.

Friendly ping :-)

[rickyz (no longer on Chrome), 2015-11-18 22:28:11]

Description was changed from

==========
Use scoped_refptr for PolicyBase.
==========

to

==========
Use scoped_refptr and RefCountedThreadSafe for TargetPolicy.

BUG=549319
==========

[rickyz (no longer on Chrome), 2015-11-18 22:36:37]

Patchset #1 (id:1) has been deleted

[rickyz (no longer on Chrome), 2015-11-18 22:36:55]

Patchset #1 (id:20001) has been deleted

[rickyz (no longer on Chrome), 2015-11-18 22:37:01]

Patchset #1 (id:40001) has been deleted

[rickyz (no longer on Chrome), 2015-11-18 22:39:07]

Per discussion on chromium-dev and offline, I've changed TargetPolicy to use
RefCountedThreadSafe instead of implementing its own refcounting. This CL now
depends on https://codereview.chromium.org/1456663003 and
https://codereview.chromium.org/1460903002, which unify PolicyBase and
TargetPolicy.