Changing CloudPrintURLFetcher instantiation to be more testable

Changing printer job handler to be more testable

1. Instead of using "new" to create cloud print URL fetchers,
use protected virtual ::CreateCloudPrintURLFetcher()

2. Instead of having a private destructor, have a protected one,
allowing for sub-classing (useful for testing purposes)

BUG=


Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=181997

[Noam Samuel (WRONG ACCOUNT), 2013-02-08 19:32:09]

This change is part of an effort to add unit testing for PrinterJobHandler. It
does not directly change the behavior of PrinterJobHandler in production, but it
does allow fitting it into a unit test more easily.

[Vitaly Buka (NO REVIEWS), 2013-02-08 22:38:56]

https://codereview.chromium.org/12208089/diff/1/chrome/service/cloud_print/pr...
File chrome/service/cloud_print/printer_job_handler.cc (right):

https://codereview.chromium.org/12208089/diff/1/chrome/service/cloud_print/pr...
chrome/service/cloud_print/printer_job_handler.cc:700: CloudPrintURLFetcher*
PrinterJobHandler::CreateCloudPrintURLFetcher() {
Maybe also to use static factory for tests?

https://codereview.chromium.org/12208089/diff/1/chrome/service/cloud_print/pr...
File chrome/service/cloud_print/printer_job_handler.h (right):

https://codereview.chromium.org/12208089/diff/1/chrome/service/cloud_print/pr...
chrome/service/cloud_print/printer_job_handler.h:150: virtual
CloudPrintURLFetcher* CreateCloudPrintURLFetcher();
empty line before private

[Noam Samuel (WRONG ACCOUNT), 2013-02-11 22:44:55]

-- Created a CloudPrintURLFetcherFactory class that can be used to change the
cloud print URL fetcher instantiation to redirect it to a test version if
necessary. 

-- Modified PrinterJobHandler and JobStatusUpdater to use new-style
instantiation.

[Vitaly Buka (NO REVIEWS), 2013-02-11 22:46:52]

lgtm

[gene, 2013-02-11 23:34:05]

https://codereview.chromium.org/12208089/diff/5001/chrome/service/cloud_print...
File chrome/service/cloud_print/cloud_print_url_fetcher.cc (right):

https://codereview.chromium.org/12208089/diff/5001/chrome/service/cloud_print...
chrome/service/cloud_print/cloud_print_url_fetcher.cc:38: void
CloudPrintURLFetcher::set_factory(
Does this fit on one line?

https://codereview.chromium.org/12208089/diff/5001/chrome/service/cloud_print...
chrome/service/cloud_print/cloud_print_url_fetcher.cc:44:
CloudPrintURLFetcherFactory::CloudPrintURLFetcherFactory() {
Can we omit constructor/destructor here, and have a unittest to call set_factory
directly?

https://codereview.chromium.org/12208089/diff/5001/chrome/service/cloud_print...
chrome/service/cloud_print/cloud_print_url_fetcher.cc:86: bool
CloudPrintURLFetcher::IsSameRequest(
This should fit on one line.

https://codereview.chromium.org/12208089/diff/5001/chrome/service/cloud_print...
File chrome/service/cloud_print/cloud_print_url_fetcher.h (right):

https://codereview.chromium.org/12208089/diff/5001/chrome/service/cloud_print...
chrome/service/cloud_print/cloud_print_url_fetcher.h:28: 
one empty line is enough here

https://codereview.chromium.org/12208089/diff/5001/chrome/service/cloud_print...
chrome/service/cloud_print/cloud_print_url_fetcher.h:33:
CloudPrintURLFetcherFactory();
May be we can omit constructor/destructor here?

https://codereview.chromium.org/12208089/diff/5001/chrome/service/cloud_print...
chrome/service/cloud_print/cloud_print_url_fetcher.h:35: virtual
CloudPrintURLFetcher* CreateCloudPrintURLFetcher()=0;
add spaces around =. "() = 0;"

https://codereview.chromium.org/12208089/diff/5001/chrome/service/cloud_print...
chrome/service/cloud_print/cloud_print_url_fetcher.h:38: 
one empty line is enough here

https://codereview.chromium.org/12208089/diff/5001/chrome/service/cloud_print...
chrome/service/cloud_print/cloud_print_url_fetcher.h:110:
CloudPrintURLFetcher();
Constructor should probably be in protected section now.

https://codereview.chromium.org/12208089/diff/5001/chrome/service/cloud_print...
chrome/service/cloud_print/cloud_print_url_fetcher.h:153: static
CloudPrintURLFetcherFactory* factory();
Can you move this before declaring member variables?

https://codereview.chromium.org/12208089/diff/5001/chrome/service/cloud_print...
File chrome/service/cloud_print/printer_job_handler.cc (right):

https://codereview.chromium.org/12208089/diff/5001/chrome/service/cloud_print...
chrome/service/cloud_print/printer_job_handler.cc:700: 
you don't need new line here

[Noam Samuel (WRONG ACCOUNT), 2013-02-12 00:32:09]

Incorporated Gene's suggestions

[gene, 2013-02-12 02:30:58]

lgtm

[commit-bot: I haz the power, 2013-02-12 18:28:37]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/noamsml@google.com/12208089/7005

[commit-bot: I haz the power, 2013-02-12 20:39:14]

Change committed as 181997