Implement Printer Preference Persistence (Step 1/3)

Implement Printer Preference Persistence (Step 1/3)

Introduce new "Preferences" for printer overlays were added in 

PrefNames:  
- printing.page.header.left  
- printing.page.header.center 
- printing.page.header.right 
- printing.page.footer.left 
- printing.page.footer.center 
- printing.page.footer.right

The prefs file will look like this:   

"printing": {
   "page": {
      "header": {
         "left": " ",
         "center": " ",
         "right": " "
      },
      "footer": {
         "left": "{date}",
         "center": " ",
         "right": " "
      }
   }
}

The steps for Printer Preferences:

Step 1:
 Introduce some sort of persistence for header/footer which is located in the Preference file.

Step 2:
 Allow PrinterQuery to read those stored preferences and setup PrinterSettings so that it will apply it when printing.

Step 3:
 Create a UI for the print preference, so that the users will easily change those printing settings.

NOTE: I tested the following approach while doing a simple Hack with PrintJob (if you see the previous patch), and it works great, with some bad after effect. It was done for testing purposes. So I hope we are going in the right direction.

BUG=947
(http://crbug.com/947)

[Mohamed Mansour (USE mhm), 2009-04-05 04:11:34]

Hey guys, please review this, I learned quite a bit about how the Print
infrastructure goes, but still fuzzy, I really need your advice.

It works, but don't know if this is the approach you were looking for.

[M-A Ruel, 2009-04-06 20:51:41]

Sorry for the delay. I like the change, still some things to fix though.

http://codereview.chromium.org/60118/diff/1/4
File chrome/browser/printing/page_overlays.cc (right):

http://codereview.chromium.org/60118/diff/1/4#newcode113
Line 113: break;
Add for each switch case:
default:
  NOTREACHED();
  break;

http://codereview.chromium.org/60118/diff/1/3
File chrome/browser/printing/page_overlays.h (right):

http://codereview.chromium.org/60118/diff/1/3#newcode43
Line 43: std::wstring& input) const;
Don't make it const if it modify the object. We need to modify the place where
it keeps a "const" instance and make it non-const instead.

http://codereview.chromium.org/60118/diff/1/3#newcode69
Line 69: mutable std::wstring top_left;
Don't set that mutable

http://codereview.chromium.org/60118/diff/1/8
File chrome/browser/printing/print_view_manager.cc (right):

http://codereview.chromium.org/60118/diff/1/8#newcode289
Line 289: FilePath user_data_dir;
I'd rather have the PrintViewManager constructor modified to accept a Profile*
pointer:
See WebContents constructor. It initializes printing_(*this), replace that with
printing_(*this, profile->GetOriginalProfile()). We don't want the Incognito
profile for printing AFAIK, unless we start logging anything.

http://codereview.chromium.org/60118/diff/1/7
File chrome/browser/printing/print_view_manager.h (right):

http://codereview.chromium.org/60118/diff/1/7#newcode31
Line 31: static void RegisterUserPrefs(PrefService* prefs);
Please document.

http://codereview.chromium.org/60118/diff/1/10
File chrome/common/pref_names.cc (right):

http://codereview.chromium.org/60118/diff/1/10#newcode239
Line 239: const wchar_t kPrinterOverlayTopLeft[] = L"printer.overlay.top_left";
I'm unsure about the names. I had a discussion with Sverrir and we're still
unsure. :)
Do you mind using:
print.header.left, print.footer.center

or
print.page_setup.footer.right

or

printing.page.footer.right

I don't know, please comment.

[Mohamed Mansour (USE mhm), 2009-04-07 02:32:11]

Please review, we decided to split this into 3 sections. This would be the first
section. Updated the description as well to reflect the new changes.

I prefer printing.page.footer.right, it seems elegant, and will allow us to add
more future properties under the printing item.

[M-A Ruel, 2009-04-07 15:42:58]

http://codereview.chromium.org/60118/diff/1002/1004
File chrome/browser/printing/page_overlays.cc (right):

http://codereview.chromium.org/60118/diff/1002/1004#newcode95
Line 95: bottom_left = input;
Add default: at this level too.
You forgot the break; statement, this code just doesn't work at all.
As a consequence, write an unit test for it.

http://codereview.chromium.org/60118/diff/1002/1003
File chrome/browser/printing/page_overlays.h (right):

http://codereview.chromium.org/60118/diff/1002/1003#newcode42
Line 42: void SetOverlay(HorizontalPosition x, VerticalPosition y,
Please comment.

[Mohamed Mansour (USE mhm), 2009-04-07 23:50:03]

On 2009/04/07 15:42:58, M-A wrote:
> As a consequence, write an unit test for it.

Gee, your right, sorry! Stupid mistake :< Please double check. I have testes
NOTREACHED occasions, which worked, but I can't include them in the unittest, I
don't know how to test for NOTREACHED.

Please re-review.

[M-A Ruel, 2009-04-09 16:37:23]

Committed as revision 13426

[Sverrir, 2009-04-09 16:58:01]

LGTM