Include driver infor into PrinterBasicInfo.

Driver related info would be included into PrinterBasicInfo::options so it would be uploaded to cloud print backend.


BUG=108194
TEST=none

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

[gene, 2012-03-21 21:07:22]

https://chromiumcodereview.appspot.com/9812002/diff/3003/printing/backend/pri...
File printing/backend/print_backend_win.cc (right):

https://chromiumcodereview.appspot.com/9812002/diff/3003/printing/backend/pri...
printing/backend/print_backend_win.cc:79:
InitBasicPrinterInfo(&printer_info[index],
Can you make InitBasicPrinterInfo to take printer name and fill in
PrinterBasicInfo?

https://chromiumcodereview.appspot.com/9812002/diff/3003/printing/backend/pri...
printing/backend/print_backend_win.cc:182: return
GetDriverInfo(GetDriverInfo6(printer, &driver_info_buffer));
Let's make GetDriverInfo to take printer name?

https://chromiumcodereview.appspot.com/9812002/diff/3003/printing/backend/win...
File printing/backend/win_helper.cc (right):

https://chromiumcodereview.appspot.com/9812002/diff/3003/printing/backend/win...
printing/backend/win_helper.cc:277: buffer->reset(new BYTE[bytes_needed]);
Let's check bytes need != 0, before trying to allocate buffer

https://chromiumcodereview.appspot.com/9812002/diff/3003/printing/backend/win...
printing/backend/win_helper.cc:284: if (!bytes_needed)
This looks unnecessary

https://chromiumcodereview.appspot.com/9812002/diff/3003/printing/backend/win...
printing/backend/win_helper.cc:296: buffer->reset(new BYTE[bytes_needed]);
Let's check bytes need != 0, before trying to allocate buffer

https://chromiumcodereview.appspot.com/9812002/diff/3003/printing/backend/win...
printing/backend/win_helper.cc:303: if (!bytes_needed)
This looks unnecessary

https://chromiumcodereview.appspot.com/9812002/diff/3003/printing/backend/win...
File printing/backend/win_helper.h (right):

https://chromiumcodereview.appspot.com/9812002/diff/3003/printing/backend/win...
printing/backend/win_helper.h:128: PRINTING_EXPORT const PRINTER_INFO_2*
GetPrinterInfo2(
Could you please create a simple class representing PRINTER_INFO_2? Current API
is pretty confusing.

https://chromiumcodereview.appspot.com/9812002/diff/3003/printing/backend/win...
printing/backend/win_helper.h:132: PRINTING_EXPORT const DRIVER_INFO_6*
GetDriverInfo6(
Could you please create a simple class representing DRIVER_INFO_6? Current API
is pretty confusing.

[Vitaly Buka (NO REVIEWS), 2012-03-21 21:23:01]

https://chromiumcodereview.appspot.com/9812002/diff/3003/printing/backend/pri...
File printing/backend/print_backend_win.cc (right):

https://chromiumcodereview.appspot.com/9812002/diff/3003/printing/backend/pri...
printing/backend/print_backend_win.cc:79:
InitBasicPrinterInfo(&printer_info[index],
need to talk about that

On 2012/03/21 21:07:22, gene wrote:
> Can you make InitBasicPrinterInfo to take printer name and fill in
> PrinterBasicInfo?

https://chromiumcodereview.appspot.com/9812002/diff/3003/printing/backend/win...
File printing/backend/win_helper.cc (right):

https://chromiumcodereview.appspot.com/9812002/diff/3003/printing/backend/win...
printing/backend/win_helper.cc:277: buffer->reset(new BYTE[bytes_needed]);
On 2012/03/21 21:07:22, gene wrote:
> Let's check bytes need != 0, before trying to allocate buffer

Done.

https://chromiumcodereview.appspot.com/9812002/diff/3003/printing/backend/win...
printing/backend/win_helper.cc:284: if (!bytes_needed)
On 2012/03/21 21:07:22, gene wrote:
> This looks unnecessary

Done.

https://chromiumcodereview.appspot.com/9812002/diff/3003/printing/backend/win...
printing/backend/win_helper.cc:296: buffer->reset(new BYTE[bytes_needed]);
On 2012/03/21 21:07:22, gene wrote:
> Let's check bytes need != 0, before trying to allocate buffer

Done.

https://chromiumcodereview.appspot.com/9812002/diff/3003/printing/backend/win...
printing/backend/win_helper.cc:303: if (!bytes_needed)
On 2012/03/21 21:07:22, gene wrote:
> This looks unnecessary

Done.

https://chromiumcodereview.appspot.com/9812002/diff/3003/printing/backend/win...
File printing/backend/win_helper.h (right):

https://chromiumcodereview.appspot.com/9812002/diff/3003/printing/backend/win...
printing/backend/win_helper.h:128: PRINTING_EXPORT const PRINTER_INFO_2*
GetPrinterInfo2(
need to talk about that

On 2012/03/21 21:07:22, gene wrote:
> Could you please create a simple class representing PRINTER_INFO_2? Current
API
> is pretty confusing.

https://chromiumcodereview.appspot.com/9812002/diff/3003/printing/backend/win...
printing/backend/win_helper.h:132: PRINTING_EXPORT const DRIVER_INFO_6*
GetDriverInfo6(
need to talk about that

On 2012/03/21 21:07:22, gene wrote:
> Could you please create a simple class representing DRIVER_INFO_6? Current API
> is pretty confusing.

[Vitaly Buka (NO REVIEWS), 2012-03-21 22:16:15]

Please review again.

[gene, 2012-03-21 22:53:08]

almost there :)
A few comments below

https://chromiumcodereview.appspot.com/9812002/diff/4020/printing/backend/pri...
File printing/backend/print_backend_consts.cc (right):

https://chromiumcodereview.appspot.com/9812002/diff/4020/printing/backend/pri...
printing/backend/print_backend_consts.cc:11: const char kDriverInfoTagName[] =
"driverinfo";
"system_driverinfo" ???
I would like to have something that lass likely to cause collisions with printer
settings

https://chromiumcodereview.appspot.com/9812002/diff/4020/printing/backend/win...
File printing/backend/win_helper.cc (right):

https://chromiumcodereview.appspot.com/9812002/diff/4020/printing/backend/win...
printing/backend/win_helper.cc:68: const PRINTER_INFO_2* GetPrinterInfo2(HANDLE
printer,
Could you please add a small comment that this function will return a pointer to
the structure that is located inside buffer? I am just worried about someone
reusing this function without fully understanding its semantic.

https://chromiumcodereview.appspot.com/9812002/diff/4020/printing/backend/win...
printing/backend/win_helper.cc:78: if (!bytes_needed || !buffer->get())
you probably don't need this anymore

https://chromiumcodereview.appspot.com/9812002/diff/4020/printing/backend/win...
printing/backend/win_helper.cc:97: if (!bytes_needed || !buffer->get())
you probably don't need this anymore

https://chromiumcodereview.appspot.com/9812002/diff/4020/printing/backend/win...
printing/backend/win_helper.cc:320: if (!info2)
duplicate

https://chromiumcodereview.appspot.com/9812002/diff/4020/printing/backend/win...
printing/backend/win_helper.cc:342: std::string driver_info;
move this definition down where it first used (for loop)

[Vitaly Buka (NO REVIEWS), 2012-03-21 23:03:10]

https://chromiumcodereview.appspot.com/9812002/diff/4020/printing/backend/pri...
File printing/backend/print_backend_consts.cc (right):

https://chromiumcodereview.appspot.com/9812002/diff/4020/printing/backend/pri...
printing/backend/print_backend_consts.cc:11: const char kDriverInfoTagName[] =
"driverinfo";
On 2012/03/21 22:53:09, gene wrote:
> "system_driverinfo" ???
> I would like to have something that lass likely to cause collisions with
printer
> settings

Done.

https://chromiumcodereview.appspot.com/9812002/diff/4020/printing/backend/win...
File printing/backend/win_helper.cc (right):

https://chromiumcodereview.appspot.com/9812002/diff/4020/printing/backend/win...
printing/backend/win_helper.cc:68: const PRINTER_INFO_2* GetPrinterInfo2(HANDLE
printer,
On 2012/03/21 22:53:09, gene wrote:
> Could you please add a small comment that this function will return a pointer
to
> the structure that is located inside buffer? I am just worried about someone
> reusing this function without fully understanding its semantic.

Done.

https://chromiumcodereview.appspot.com/9812002/diff/4020/printing/backend/win...
printing/backend/win_helper.cc:78: if (!bytes_needed || !buffer->get())
On 2012/03/21 22:53:09, gene wrote:
> you probably don't need this anymore

Done.

https://chromiumcodereview.appspot.com/9812002/diff/4020/printing/backend/win...
printing/backend/win_helper.cc:97: if (!bytes_needed || !buffer->get())
On 2012/03/21 22:53:09, gene wrote:
> you probably don't need this anymore

Done.

https://chromiumcodereview.appspot.com/9812002/diff/4020/printing/backend/win...
printing/backend/win_helper.cc:320: if (!info2)
On 2012/03/21 22:53:09, gene wrote:
> duplicate

Done.

Common! Compiler should fix that :-).

https://chromiumcodereview.appspot.com/9812002/diff/4020/printing/backend/win...
printing/backend/win_helper.cc:342: std::string driver_info;
On 2012/03/21 22:53:09, gene wrote:
> move this definition down where it first used (for loop)
 I use that as empty string to return.

[gene, 2012-03-21 23:18:41]

lgtm

[commit-bot: I haz the power, 2012-03-21 23:25:49]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vitalybuka@chromium.org/9812002/12015

[commit-bot: I haz the power, 2012-03-21 23:26:06]

Presubmit check for 9812002-12015 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit Warnings **
Found line ending with white spaces in:

***************
printing/backend/win_helper.cc, line 367
***************

Presubmit checks took 1.8s to calculate.

Was the presubmit check useful? Please send feedback & hate mail to
maruel@chromium.org!

[commit-bot: I haz the power, 2012-03-21 23:32:54]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vitalybuka@chromium.org/9812002/3019

[commit-bot: I haz the power, 2012-03-22 02:21:13]

Change committed as 128141