Enable the cups backend when --enable-native-cups is specified.

Enable the cups backend only when --enable-native-cups is specified.

CUPS printers will now be selectable in the print menus.

NOTE: This does not enable printing to said printers.

BUG=607668
TEST=Launch chrome with --enable-native-cups flag, open print preview,
change printers.  See that CUPS printers are listed as available
printers.

Committed: https://crrev.com/8ffe7751ea81f5d0d571999eccf2e8461a552dff
Cr-Commit-Position: refs/heads/master@{#402654}

[skau, 2016-04-29 22:19:56]

Description was changed from

==========
Enable the cups backend when --enable-native-cups is specified.

CUPS printers will now be selectable in the print menus.  NOTE: Acutally
printing is not enabled.

BUG=607668

TEST=Launch chrome with --enable-native-cups flag, open print preview,
change printers.  See that CUPS printers are listed as available
printers.
==========

to

==========
Enable the cups backend when --enable-native-cups is specified.

CUPS printers will now be selectable in the print menus.  NOTE: Acutally
printing is not enabled.

BUG=607668

TEST=Launch chrome with --enable-native-cups flag, open print preview,
change printers.  See that CUPS printers are listed as available
printers.
==========

[skau, 2016-04-29 22:19:56]

skau@chromium.org changed reviewers:
+ thestig@chromium.org

[skau, 2016-05-23 16:37:12]

skau@chromium.org changed reviewers:
+ briannorris@chromium.org

[skau, 2016-05-26 04:31:45]

The CQ bit was checked by skau@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-05-26 04:31:54]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1934013002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1934013002/20001

[commit-bot: I haz the power, 2016-05-26 04:45:08]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-05-26 04:45:09]

Dry run: Try jobs failed on following builders:
  chromeos_amd64-generic_chromium_compile_only_ng on tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)

[skau, 2016-06-28 18:18:50]

I've cleaned up this CL so that it retains the old behavior when use_cups is
false and the flag is off.  This will be submitted before I flip the build flag
in ChromeOS.

[skau, 2016-06-28 18:22:20]

skau@chromium.org changed reviewers:
+ adlr@chromium.org

[Lei Zhang, 2016-06-28 19:09:48]

There's a typo in the CL description.

https://codereview.chromium.org/1934013002/diff/40001/printing/DEPS
File printing/DEPS (right):

https://codereview.chromium.org/1934013002/diff/40001/printing/DEPS#newcode13
printing/DEPS:13: "+chrome/common",
Sorry, you can't do this. That's what the red try bots are telling you.
printing/ is the base layer and chrome/ builds on top of printing.

https://codereview.chromium.org/1934013002/diff/40001/printing/backend/print_...
File printing/backend/print_backend_chromeos.cc (right):

https://codereview.chromium.org/1934013002/diff/40001/printing/backend/print_...
printing/backend/print_backend_chromeos.cc:82: if
(base::CommandLine::ForCurrentProcess()->HasSwitch(
Given that PrintBackend::CreateInstance() has many implementations, changing its
signature is a pain. Assuming |kEnableNativeCups| is not going to be around
forever, maybe just add a static method to set a global boolean in this file.

Set the global from chrome/ to break the dependency.

https://codereview.chromium.org/1934013002/diff/40001/printing/backend/print_...
printing/backend/print_backend_chromeos.cc:84: #ifdef USE_CUPS
nit: #if defined(USE_CUPS)

https://codereview.chromium.org/1934013002/diff/40001/printing/backend/print_...
printing/backend/print_backend_chromeos.cc:85: std::string print_server_url_str,
cups_blocking;
nit: one decl per line

[skau - Gooooogle, 2016-06-28 20:11:37]

Description was changed from

==========
Enable the cups backend when --enable-native-cups is specified.

CUPS printers will now be selectable in the print menus.  NOTE: Acutally
printing is not enabled.

BUG=607668

TEST=Launch chrome with --enable-native-cups flag, open print preview,
change printers.  See that CUPS printers are listed as available
printers.
==========

to

==========
Enable the cups backend only when --enable-native-cups is specified.

CUPS printers will now be selectable in the print menus.  

NOTE: This does not enable printing to said printers.

BUG=607668

TEST=Launch chrome with --enable-native-cups flag, open print preview,
change printers.  See that CUPS printers are listed as available
printers.
==========

[skau, 2016-06-28 21:30:28]

I've moved the flag check into the print_preview_handler and that seems to have
made the trybots happy.  PTAL.

Thanks for the help.

[skau, 2016-06-28 21:40:15]

https://codereview.chromium.org/1934013002/diff/40001/printing/DEPS
File printing/DEPS (right):

https://codereview.chromium.org/1934013002/diff/40001/printing/DEPS#newcode13
printing/DEPS:13: "+chrome/common",
On 2016/06/28 19:09:47, Lei Zhang wrote:
> Sorry, you can't do this. That's what the red try bots are telling you.
> printing/ is the base layer and chrome/ builds on top of printing.

Thanks. I've moved it to webui

https://codereview.chromium.org/1934013002/diff/40001/printing/backend/print_...
File printing/backend/print_backend_chromeos.cc (right):

https://codereview.chromium.org/1934013002/diff/40001/printing/backend/print_...
printing/backend/print_backend_chromeos.cc:82: if
(base::CommandLine::ForCurrentProcess()->HasSwitch(
On 2016/06/28 19:09:47, Lei Zhang wrote:
> Given that PrintBackend::CreateInstance() has many implementations, changing
its
> signature is a pain. Assuming |kEnableNativeCups| is not going to be around
> forever, maybe just add a static method to set a global boolean in this file.
> 
> Set the global from chrome/ to break the dependency.

Done.

https://codereview.chromium.org/1934013002/diff/40001/printing/backend/print_...
printing/backend/print_backend_chromeos.cc:84: #ifdef USE_CUPS
On 2016/06/28 19:09:47, Lei Zhang wrote:
> nit: #if defined(USE_CUPS)

Done.

https://codereview.chromium.org/1934013002/diff/40001/printing/backend/print_...
printing/backend/print_backend_chromeos.cc:85: std::string print_server_url_str,
cups_blocking;
On 2016/06/28 19:09:47, Lei Zhang wrote:
> nit: one decl per line

Done.

[Lei Zhang, 2016-06-28 22:27:35]

https://codereview.chromium.org/1934013002/diff/100001/chrome/browser/ui/webu...
File chrome/browser/ui/webui/print_preview/print_preview_handler.cc (right):

https://codereview.chromium.org/1934013002/diff/100001/chrome/browser/ui/webu...
chrome/browser/ui/webui/print_preview/print_preview_handler.cc:430:
printing::PrintBackend::SetNativeCupsEnabled(
We probably don't need to set the same value every time we enumerate printers.
How about we stick it in ChromeBrowserMainPartsLinux::PostMainMessageLoopStart()
and just call it once early on?

BTW, is the plan that |kEnableNativeCups| will eventually become always true?

https://codereview.chromium.org/1934013002/diff/100001/printing/backend/print...
File printing/backend/print_backend.h (right):

https://codereview.chromium.org/1934013002/diff/100001/printing/backend/print...
printing/backend/print_backend.h:133: static bool native_cups_enabled;
You can just declare this in an anonymous namespace in the .cc file. Since most
Chromium .cc files contain only one class impl, you are not losing out on any
encapsulation. This can also be infinitely complex and you won't have to expose
what it is in the header.

https://codereview.chromium.org/1934013002/diff/100001/printing/backend/print...
File printing/backend/print_backend_chromeos.cc (right):

https://codereview.chromium.org/1934013002/diff/100001/printing/backend/print...
printing/backend/print_backend_chromeos.cc:10: #if defined(USE_CUPS)
#includes inside an #if goes in their own section below the main list.

https://codereview.chromium.org/1934013002/diff/100001/printing/backend/print...
File printing/backend/print_backend_cups.cc (right):

https://codereview.chromium.org/1934013002/diff/100001/printing/backend/print...
printing/backend/print_backend_cups.cc:10: #include <string>
Chromium code separates C headers from C++ headers. (google3 prefers the other
way)

[skau, 2016-06-28 23:58:19]

https://codereview.chromium.org/1934013002/diff/100001/chrome/browser/ui/webu...
File chrome/browser/ui/webui/print_preview/print_preview_handler.cc (right):

https://codereview.chromium.org/1934013002/diff/100001/chrome/browser/ui/webu...
chrome/browser/ui/webui/print_preview/print_preview_handler.cc:430:
printing::PrintBackend::SetNativeCupsEnabled(
On 2016/06/28 22:27:35, Lei Zhang wrote:
> We probably don't need to set the same value every time we enumerate printers.
> How about we stick it in
ChromeBrowserMainPartsLinux::PostMainMessageLoopStart()
> and just call it once early on?
> 
I've moved it to ChromeBrowserMainChromeos since this is only for Chromeos.

> BTW, is the plan that |kEnableNativeCups| will eventually become always true?
Yes.  The plan is to remove the flag once implementation of the backend is
complete and verified.

https://codereview.chromium.org/1934013002/diff/100001/printing/backend/print...
File printing/backend/print_backend.h (right):

https://codereview.chromium.org/1934013002/diff/100001/printing/backend/print...
printing/backend/print_backend.h:133: static bool native_cups_enabled;
On 2016/06/28 22:27:35, Lei Zhang wrote:
> You can just declare this in an anonymous namespace in the .cc file. Since
most
> Chromium .cc files contain only one class impl, you are not losing out on any
> encapsulation. This can also be infinitely complex and you won't have to
expose
> what it is in the header.

Done.

https://codereview.chromium.org/1934013002/diff/100001/printing/backend/print...
File printing/backend/print_backend_chromeos.cc (right):

https://codereview.chromium.org/1934013002/diff/100001/printing/backend/print...
printing/backend/print_backend_chromeos.cc:10: #if defined(USE_CUPS)
On 2016/06/28 22:27:35, Lei Zhang wrote:
> #includes inside an #if goes in their own section below the main list.

Done.

https://codereview.chromium.org/1934013002/diff/100001/printing/backend/print...
File printing/backend/print_backend_cups.cc (right):

https://codereview.chromium.org/1934013002/diff/100001/printing/backend/print...
printing/backend/print_backend_cups.cc:10: #include <string>
On 2016/06/28 22:27:35, Lei Zhang wrote:
> Chromium code separates C headers from C++ headers. (google3 prefers the other
> way)

Done.

[Lei Zhang, 2016-06-29 00:22:31]

lgtm with some more nits

https://codereview.chromium.org/1934013002/diff/120001/printing/backend/print...
File printing/backend/print_backend.cc (right):

https://codereview.chromium.org/1934013002/diff/120001/printing/backend/print...
printing/backend/print_backend.cc:8: static bool native_cups_enabled = false;
You don't need static in an anonymous namespace.

https://codereview.chromium.org/1934013002/diff/120001/printing/backend/print...
printing/backend/print_backend.cc:8: static bool native_cups_enabled = false;
g_native_cups_enabled

https://codereview.chromium.org/1934013002/diff/120001/printing/backend/print...
printing/backend/print_backend.cc:47: bool PrintBackend::GetNativeCupsEnabled()
{
Add: // static
above this line, same for SetNativeCupsEnabled().

https://codereview.chromium.org/1934013002/diff/120001/printing/backend/print...
File printing/backend/print_backend_chromeos.cc (right):

https://codereview.chromium.org/1934013002/diff/120001/printing/backend/print...
printing/backend/print_backend_chromeos.cc:82: if
(PrintBackend::GetNativeCupsEnabled()) {
Does it work if you omit PrintBackend:: ?

[skau, 2016-06-29 00:40:17]

https://codereview.chromium.org/1934013002/diff/120001/printing/backend/print...
File printing/backend/print_backend.cc (right):

https://codereview.chromium.org/1934013002/diff/120001/printing/backend/print...
printing/backend/print_backend.cc:8: static bool native_cups_enabled = false;
On 2016/06/29 00:22:31, Lei Zhang wrote:
> You don't need static in an anonymous namespace.

Done.

https://codereview.chromium.org/1934013002/diff/120001/printing/backend/print...
printing/backend/print_backend.cc:8: static bool native_cups_enabled = false;
On 2016/06/29 00:22:31, Lei Zhang wrote:
> You don't need static in an anonymous namespace.

Done.

https://codereview.chromium.org/1934013002/diff/120001/printing/backend/print...
printing/backend/print_backend.cc:47: bool PrintBackend::GetNativeCupsEnabled()
{
On 2016/06/29 00:22:31, Lei Zhang wrote:
> Add: // static
> above this line, same for SetNativeCupsEnabled().

Done.

https://codereview.chromium.org/1934013002/diff/120001/printing/backend/print...
File printing/backend/print_backend_chromeos.cc (right):

https://codereview.chromium.org/1934013002/diff/120001/printing/backend/print...
printing/backend/print_backend_chromeos.cc:82: if
(PrintBackend::GetNativeCupsEnabled()) {
On 2016/06/29 00:22:31, Lei Zhang wrote:
> Does it work if you omit PrintBackend:: ?

It does!

[skau, 2016-06-29 00:41:39]

The CQ bit was checked by skau@chromium.org

[skau, 2016-06-29 00:41:40]

The patchset sent to the CQ was uploaded after l-g-t-m from thestig@chromium.org
Link to the patchset: https://codereview.chromium.org/1934013002/#ps40002
(title: "Select the printing backends at runtime via --enable-native-cups")

[commit-bot: I haz the power, 2016-06-29 00:42:50]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-06-29 02:10:47]

Description was changed from

==========
Enable the cups backend only when --enable-native-cups is specified.

CUPS printers will now be selectable in the print menus.  

NOTE: This does not enable printing to said printers.

BUG=607668

TEST=Launch chrome with --enable-native-cups flag, open print preview,
change printers.  See that CUPS printers are listed as available
printers.
==========

to

==========
Enable the cups backend only when --enable-native-cups is specified.

CUPS printers will now be selectable in the print menus.  

NOTE: This does not enable printing to said printers.

BUG=607668

TEST=Launch chrome with --enable-native-cups flag, open print preview,
change printers.  See that CUPS printers are listed as available
printers.
==========

[commit-bot: I haz the power, 2016-06-29 02:10:48]

Committed patchset #8 (id:40002)

[commit-bot: I haz the power, 2016-06-29 02:13:30]

Description was changed from

==========
Enable the cups backend only when --enable-native-cups is specified.

CUPS printers will now be selectable in the print menus.  

NOTE: This does not enable printing to said printers.

BUG=607668

TEST=Launch chrome with --enable-native-cups flag, open print preview,
change printers.  See that CUPS printers are listed as available
printers.
==========

to

==========
Enable the cups backend only when --enable-native-cups is specified.

CUPS printers will now be selectable in the print menus.

NOTE: This does not enable printing to said printers.

BUG=607668

TEST=Launch chrome with --enable-native-cups flag, open print preview,
change printers.  See that CUPS printers are listed as available
printers.

Committed: https://crrev.com/8ffe7751ea81f5d0d571999eccf2e8461a552dff
Cr-Commit-Position: refs/heads/master@{#402654}
==========

[commit-bot: I haz the power, 2016-06-29 02:13:32]

Patchset 8 (id:??) landed as
https://crrev.com/8ffe7751ea81f5d0d571999eccf2e8461a552dff
Cr-Commit-Position: refs/heads/master@{#402654}