Print Preview: Remembering last used printer.

Print Preview: Remembering last used printer.

BUG=85081
TEST=Open print preview tab, print a doc. Reopen, the last used printer should be pre-selected.


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

[dpapad, 2011-06-07 01:07:44]

I have tested only on Linux so far.

http://codereview.chromium.org/7042034/diff/1/chrome/browser/resources/print_...
File chrome/browser/resources/print_preview.js (right):

http://codereview.chromium.org/7042034/diff/1/chrome/browser/resources/print_...
chrome/browser/resources/print_preview.js:372: function setPrinters(printers) {
Since we already know the default printer, there is no longer need to include
this info here.

http://codereview.chromium.org/7042034/diff/1001/chrome/browser/ui/webui/prin...
File chrome/browser/ui/webui/print_preview_handler.cc (right):

http://codereview.chromium.org/7042034/diff/1001/chrome/browser/ui/webui/prin...
chrome/browser/ui/webui/print_preview_handler.cc:182: << default_printer_string;
This is only for debugging purposes.

[dpapad, 2011-06-07 02:11:20]

On 2011/06/07 01:07:44, dpapad wrote:
> I have tested only on Linux so far.
> 
>
http://codereview.chromium.org/7042034/diff/1/chrome/browser/resources/print_...
> File chrome/browser/resources/print_preview.js (right):
> 
>
http://codereview.chromium.org/7042034/diff/1/chrome/browser/resources/print_...
> chrome/browser/resources/print_preview.js:372: function setPrinters(printers)
{
> Since we already know the default printer, there is no longer need to include
> this info here.
> 
>
http://codereview.chromium.org/7042034/diff/1001/chrome/browser/ui/webui/prin...
> File chrome/browser/ui/webui/print_preview_handler.cc (right):
> 
>
http://codereview.chromium.org/7042034/diff/1001/chrome/browser/ui/webui/prin...
> chrome/browser/ui/webui/print_preview_handler.cc:182: <<
default_printer_string;
> This is only for debugging purposes.

Tested on Windows too.

[Lei Zhang, 2011-06-08 00:41:59]

http://codereview.chromium.org/7042034/diff/10001/chrome/browser/ui/webui/pri...
File chrome/browser/ui/webui/print_preview_handler.h (right):

http://codereview.chromium.org/7042034/diff/10001/chrome/browser/ui/webui/pri...
chrome/browser/ui/webui/print_preview_handler.h:104: static std::string
last_used_printer_;
I don't think you are allowed to use non-POD static variable.

http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Static_and_Glo...

http://codereview.chromium.org/7042034/diff/10001/printing/backend/print_back...
File printing/backend/print_backend_cups.cc (right):

http://codereview.chromium.org/7042034/diff/10001/printing/backend/print_back...
printing/backend/print_backend_cups.cc:162: return dest ?
std::string(dest->name) : std::string();
I'm glad you found a reliable way to get the default printer! However, let's
write it to handle the "2" version of the CUPS function as well:

  cups_dest_t *dests;
  int num_dests = GetDests(&dests);
  cups_dest_t* dest = cupsGetDest(NULL, NULL, num_dests, dests);
  return dest ? std::string(dest->name) : std::string();

Also, please retain a comment to indicate cupsGetDefault() returns the wrong
result.

[dpapad, 2011-06-08 16:33:56]

http://codereview.chromium.org/7042034/diff/10001/chrome/browser/ui/webui/pri...
File chrome/browser/ui/webui/print_preview_handler.h (right):

http://codereview.chromium.org/7042034/diff/10001/chrome/browser/ui/webui/pri...
chrome/browser/ui/webui/print_preview_handler.h:104: static std::string
last_used_printer_;
On 2011/06/08 00:41:59, Lei Zhang wrote:
> I don't think you are allowed to use non-POD static variable.
> 
>
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Static_and_Glo...

Done.

http://codereview.chromium.org/7042034/diff/10001/printing/backend/print_back...
File printing/backend/print_backend_cups.cc (right):

http://codereview.chromium.org/7042034/diff/10001/printing/backend/print_back...
printing/backend/print_backend_cups.cc:162: return dest ?
std::string(dest->name) : std::string();
On 2011/06/08 00:41:59, Lei Zhang wrote:
> I'm glad you found a reliable way to get the default printer! However, let's
> write it to handle the "2" version of the CUPS function as well:
> 
>   cups_dest_t *dests;
>   int num_dests = GetDests(&dests);
>   cups_dest_t* dest = cupsGetDest(NULL, NULL, num_dests, dests);
>   return dest ? std::string(dest->name) : std::string();
> 
> Also, please retain a comment to indicate cupsGetDefault() returns the wrong
> result.

Done.

[Lei Zhang, 2011-06-08 20:04:18]

LGTM

[commit-bot: I haz the power, 2011-06-08 21:06:43]

Change committed as 88399