Replacing VerifierTraits with DummyVerifierTraits for non-kernel handles.

Replacing VerifierTraits with DummyVerifierTraits for non-kernel handles.
Current implicit assumption for VerifierTraits when it is using ActiveVerifier seems to be that these are all one category of handles, espcially kernel handles as it is hooking up CloseHandle API. HDC/Printer Handle/Device Info handle all different from kernel handles.

BUG=475872
R=cpu@chromium.org,grt@chromium.org,wfh@chromium.org,vitalybuka@chromium.org,keybuk@chromium.org, armansito@chromium.org, rpaquay@chromium.org

Committed: https://crrev.com/832ccd5cc5904f848c0eb6a41e46b1d2cb2f92cc
Cr-Commit-Position: refs/heads/master@{#328402}

[grt (UTC plus 2), 2015-05-04 19:42:45]

If these handle-like things aren't kernel handles and don't make use of the
verifier, can they be contained in scoped_ptr<FooHandle, FooHandleDeleter>
instances instead? This will make it it easy to have a "only use ScopedHandle
for kernel handles that use our handle tracking juju" policy.

[Will Harris, 2015-05-04 19:48:56]

I don't think this code is buggy at the moment, as the Traits define the
handle-type specific operations (CloseHandle, IsHandleValid, NullHandle) while
the Verifier defines the tracking operations which operate opaquely on the
HANDLE type.

I think what goes wrong is when Verifier part starts levying expectations on the
handle type - e.g. in https://codereview.chromium.org/1113063004/ where it makes
the base::win::VerifierTraits expect that the HANDLE passed is non-DC.

I'm not sure of the best solution here, but perhaps moving the
HANDLE_FLAG_PROTECT_FROM_CLOSE code into the base::win::HandleTraits
implementation, since that is already specific to kernel HANDLES.

[grt (UTC plus 2), 2015-05-04 19:56:12]

On 2015/05/04 19:48:56, Will Harris wrote:
> I don't think this code is buggy at the moment, as the Traits define the
> handle-type specific operations (CloseHandle, IsHandleValid, NullHandle) while
> the Verifier defines the tracking operations which operate opaquely on the
> HANDLE type.

Is there a reason to have ScopedHandle with DummyTraits? What does it buy you
over scoped_ptr with a custom deleter?

[Shrikant Kelkar, 2015-05-04 20:10:35]

On 2015/05/04 19:56:12, grt wrote:
> On 2015/05/04 19:48:56, Will Harris wrote:
> > I don't think this code is buggy at the moment, as the Traits define the
> > handle-type specific operations (CloseHandle, IsHandleValid, NullHandle)
while
> > the Verifier defines the tracking operations which operate opaquely on the
> > HANDLE type.
> 
> Is there a reason to have ScopedHandle with DummyTraits? What does it buy you
> over scoped_ptr with a custom deleter?

Yes, @grt what you are saying makes sense. I am not aware about history of
GenericScopedHandle usage, but lot of instances seem to use GenericScopedHandle
with DummyVerifierTraits and provide their own implementation of HandleTraits,
so I just kept it in same spirit.

[Will Harris, 2015-05-04 20:13:05]

Spoke to shrikant - agree with his assessment here, considering the namespaces
for these types of handles is different and potentially overlapping. lgtm.

[Vitaly Buka (NO REVIEWS), 2015-05-04 22:12:22]

lgtm

[grt (UTC plus 2), 2015-05-04 22:17:53]

rubberstam lgtm, though I think scoped_ptr should be preferred in general.

[Shrikant Kelkar, 2015-05-05 18:06:59]

The CQ bit was checked by shrikant@chromium.org

[commit-bot: I haz the power, 2015-05-05 18:10:08]

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

[Shrikant Kelkar, 2015-05-05 18:14:40]

The CQ bit was unchecked by shrikant@chromium.org

[Shrikant Kelkar, 2015-05-05 20:28:12]

shrikant@chromium.org changed reviewers:
+ armansito@chromium.org

[Shrikant Kelkar, 2015-05-05 20:29:31]

+armansito for BLE

[Shrikant Kelkar, 2015-05-05 20:31:47]

shrikant@chromium.org changed reviewers:
+ rpaquay@chromium.org

[Shrikant Kelkar, 2015-05-05 20:32:21]

+rpaquay as per keybuk@

[cpu_(ooo_6.6-7.5), 2015-05-05 20:52:10]

lgtm

[cpu_(ooo_6.6-7.5), 2015-05-05 20:52:17]

The CQ bit was checked by cpu@chromium.org

[commit-bot: I haz the power, 2015-05-05 20:53:26]

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

[rpaquay, 2015-05-05 20:57:52]

lgtm

[commit-bot: I haz the power, 2015-05-05 21:09:19]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2015-05-05 21:10:03]

Patchset 1 (id:??) landed as
https://crrev.com/832ccd5cc5904f848c0eb6a41e46b1d2cb2f92cc
Cr-Commit-Position: refs/heads/master@{#328402}