Making GenericScopedHandle capable of handling handle types other than HANDLE.

Making GenericScopedHandle capable of handling handle types other than HANDLE.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=124980

[alexeypa (please no reviews), 2012-03-03 17:00:00]

Please, take a look.

[alexeypa (please no reviews), 2012-03-03 17:01:55]

+brettw

[gene, 2012-03-03 17:45:34]

lgtm for printing

[brettw, 2012-03-05 18:14:27]

lgtm

[commit-bot: I haz the power, 2012-03-05 18:21:57]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/alexeypa@chromium.org/9582045/3001

[Vitaly Buka (NO REVIEWS), 2012-03-05 18:52:14]

lgtm

http://codereview.chromium.org/9582045/diff/3001/chrome/service/cloud_print/p...
File chrome/service/cloud_print/print_system_win.cc (left):

http://codereview.chromium.org/9582045/diff/3001/chrome/service/cloud_print/p...
chrome/service/cloud_print/print_system_win.cc:36: class
PrinterChangeHandleTraits {
: pubic HandleTraits

http://codereview.chromium.org/9582045/diff/3001/printing/backend/win_helper.h
File printing/backend/win_helper.h (left):

http://codereview.chromium.org/9582045/diff/3001/printing/backend/win_helper....
printing/backend/win_helper.h:21: class PrinterHandleTraits {
PrinterHandleTraits : public HandleTraits {

 CloseHandle

}

[alexeypa (please no reviews), 2012-03-05 18:56:58]

http://codereview.chromium.org/9582045/diff/3001/chrome/service/cloud_print/p...
File chrome/service/cloud_print/print_system_win.cc (left):

http://codereview.chromium.org/9582045/diff/3001/chrome/service/cloud_print/p...
chrome/service/cloud_print/print_system_win.cc:36: class
PrinterChangeHandleTraits {
On 2012/03/05 18:52:14, Vitaly Buka wrote:
> : pubic HandleTraits

No, why? HandleTraits and PrinterChangeHandleTraits are unrelated. They both
happen to use HANDLE as the raw handle type but it does not make them related.

http://codereview.chromium.org/9582045/diff/3001/printing/backend/win_helper.h
File printing/backend/win_helper.h (left):

http://codereview.chromium.org/9582045/diff/3001/printing/backend/win_helper....
printing/backend/win_helper.h:21: class PrinterHandleTraits {
On 2012/03/05 18:52:14, Vitaly Buka wrote:
> PrinterHandleTraits : public HandleTraits {
> 
>  CloseHandle
> 
> }

The same thing, trait classes are unrelated in this case.

[brettw, 2012-03-05 18:58:43]

http://codereview.chromium.org/9582045/diff/3001/chrome/service/cloud_print/p...
File chrome/service/cloud_print/print_system_win.cc (left):

http://codereview.chromium.org/9582045/diff/3001/chrome/service/cloud_print/p...
chrome/service/cloud_print/print_system_win.cc:36: class
PrinterChangeHandleTraits {
I would not necessarily expect that the traits classes derive from each other in
this type of situation, so I think the existing code is fine.

[Vitaly Buka (NO REVIEWS), 2012-03-05 19:09:11]

I believe HANDLE for printer and HANDLE for Semaphore are related in the same
way as Semaphore HANDLE and file HANDLE. Just different close function. If you
use same class for the second, nothing wrong with inheritance for the first.

On 2012/03/05 18:56:58, alexeypa wrote:
>
http://codereview.chromium.org/9582045/diff/3001/chrome/service/cloud_print/p...
> File chrome/service/cloud_print/print_system_win.cc (left):
> 
>
http://codereview.chromium.org/9582045/diff/3001/chrome/service/cloud_print/p...
> chrome/service/cloud_print/print_system_win.cc:36: class
> PrinterChangeHandleTraits {
> On 2012/03/05 18:52:14, Vitaly Buka wrote:
> > : pubic HandleTraits
> 
> No, why? HandleTraits and PrinterChangeHandleTraits are unrelated. They both
> happen to use HANDLE as the raw handle type but it does not make them related.
> 
> http://codereview.chromium.org/9582045/diff/3001/printing/backend/win_helper.h
> File printing/backend/win_helper.h (left):
> 
>
http://codereview.chromium.org/9582045/diff/3001/printing/backend/win_helper....
> printing/backend/win_helper.h:21: class PrinterHandleTraits {
> On 2012/03/05 18:52:14, Vitaly Buka wrote:
> > PrinterHandleTraits : public HandleTraits {
> > 
> >  CloseHandle
> > 
> > }
> 
> The same thing, trait classes are unrelated in this case.

[Vitaly Buka (NO REVIEWS), 2012-03-05 19:14:19]

BTW. Its traits. it does note create relation between ScopedHandle and
ScopedPrinterHandle anyway.

But it's a matter of taste. Up to you.

lgtm

On 2012/03/05 19:09:11, Vitaly Buka wrote:
> I believe HANDLE for printer and HANDLE for Semaphore are related in the same
> way as Semaphore HANDLE and file HANDLE. Just different close function. If you
> use same class for the second, nothing wrong with inheritance for the first.
> 
> On 2012/03/05 18:56:58, alexeypa wrote:
> >
>
http://codereview.chromium.org/9582045/diff/3001/chrome/service/cloud_print/p...
> > File chrome/service/cloud_print/print_system_win.cc (left):
> > 
> >
>
http://codereview.chromium.org/9582045/diff/3001/chrome/service/cloud_print/p...
> > chrome/service/cloud_print/print_system_win.cc:36: class
> > PrinterChangeHandleTraits {
> > On 2012/03/05 18:52:14, Vitaly Buka wrote:
> > > : pubic HandleTraits
> > 
> > No, why? HandleTraits and PrinterChangeHandleTraits are unrelated. They both
> > happen to use HANDLE as the raw handle type but it does not make them
related.
> > 
> >
http://codereview.chromium.org/9582045/diff/3001/printing/backend/win_helper.h
> > File printing/backend/win_helper.h (left):
> > 
> >
>
http://codereview.chromium.org/9582045/diff/3001/printing/backend/win_helper....
> > printing/backend/win_helper.h:21: class PrinterHandleTraits {
> > On 2012/03/05 18:52:14, Vitaly Buka wrote:
> > > PrinterHandleTraits : public HandleTraits {
> > > 
> > >  CloseHandle
> > > 
> > > }
> > 
> > The same thing, trait classes are unrelated in this case.

[commit-bot: I haz the power, 2012-03-05 20:07:12]

Change committed as 124980