Enable Native CUPS printing on Chrome OS.

Enable Native CUPS printing on Chrome OS.

Flips the polarity of the --enable-native-cups flag to --disable-native-cups.

BUG=583155
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2475813002
Cr-Commit-Position: refs/heads/master@{#462954}
Committed: https://chromium.googlesource.com/chromium/src/+/a9afc02163b48dbb93c9a343630096b8f45aec71

[skau, 2016-11-03 23:03:35]

skau@chromium.org changed reviewers:
+ justincarlson@chromium.org, xdai@chromium.org

[skau, 2016-11-03 23:03:36]

Assuming we land everything in time.

[xdai1, 2016-11-03 23:16:11]

On 2016/11/03 23:03:36, skau wrote:
> Assuming we land everything in time.

When do you plan to land this CL? And if we're going to enable CUPS by default,
why do we still need this flag? I think we can remove the flag directly?

[skau, 2017-03-16 23:21:14]

On 2016/11/03 23:16:11, xdai1 wrote:
> On 2016/11/03 23:03:36, skau wrote:
> > Assuming we land everything in time.
> 
> When do you plan to land this CL? And if we're going to enable CUPS by
default,
> why do we still need this flag? I think we can remove the flag directly?

We want to retain the ability to disable the feature in case we need to pull it
at the last minute.

[skau, 2017-03-16 23:39:10]

Description was changed from

==========
Enable Native CUPS printing on Chrome OS.

Flips the polarity of the --enable-native-cups flag to --disable-native-cups.

BUG=583155
==========

to

==========
Enable Native CUPS printing on Chrome OS.

Flips the polarity of the --enable-native-cups flag to --disable-native-cups.

BUG=583155
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[skau, 2017-03-16 23:56:18]

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

[commit-bot: I haz the power, 2017-03-16 23:56:45]

Dry run: 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, 2017-03-17 00:52:30]

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

[commit-bot: I haz the power, 2017-03-17 00:52:31]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[skau, 2017-03-29 23:24:39]

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

[commit-bot: I haz the power, 2017-03-29 23:25:34]

Dry run: 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, 2017-03-30 00:36:41]

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

[commit-bot: I haz the power, 2017-03-30 00:36:42]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[skau, 2017-03-30 19:43:09]

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

[commit-bot: I haz the power, 2017-03-30 19:44:10]

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

[skau, 2017-03-30 19:51:42]

I've rebased this change and am running the buildbots.  I think we're in a good
position to flip the flag for M59 and get it into dev channel.

[commit-bot: I haz the power, 2017-03-30 21:14:02]

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

[commit-bot: I haz the power, 2017-03-30 21:14:04]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[adlr1, 2017-03-30 23:10:06]

adlr@google.com changed reviewers:
+ adlr@google.com

[adlr1, 2017-03-30 23:10:06]

This CL is surprisingly big. Is changing 'enable' to 'disable' everywhere the
convention?

[skau, 2017-03-30 23:40:37]

The polarity for all the conditions needs to be flipped so that's most of
the changes.  I could've left the javascript alone but that seems confusing.

On Thu, Mar 30, 2017 at 4:10 PM <adlr@google.com> wrote:

> This CL is surprisingly big. Is changing 'enable' to 'disable' everywhere
> the
> convention?
>
> https://codereview.chromium.org/2475813002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[skau, 2017-03-31 21:35:36]

Ping for review?

[chromium-reviews, 2017-03-31 21:40:26]

LGTM but would like someone who works on Chrome more than I do to approve
as well.

On Fri, Mar 31, 2017 at 2:35 PM <skau@chromium.org> wrote:

> Ping for review?
>
> https://codereview.chromium.org/2475813002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Carlson, 2017-03-31 21:45:53]

lgtm

https://codereview.chromium.org/2475813002/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/printer_detector/printer_detector_factory.cc
(right):

https://codereview.chromium.org/2475813002/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/printer_detector/printer_detector_factory.cc:50: return
PrinterDetector::CreateCups(Profile::FromBrowserContext(context))
Probably slightly better to flip the contents of the if and else blocks here
instead of inverting the condition.  if (!disabled) is a quasi-double-negative.

[xdai1, 2017-03-31 22:22:53]

lgtm

[skau, 2017-03-31 23:29:08]

skau@chromium.org changed reviewers:
+ jwd@chromium.org, stevenjb@chromium.org, thestig@chromium.org

[skau, 2017-03-31 23:29:09]

Thanks for the reviews.  Adding OWNERS now.

stevenjb:
chrome/browser/resources/*
chrome/browser/ui/webui/options/*
chrome/browser/ui/webui/settings/*

thestig:
chrome/browser/ui/webui/print_preview/*

jwd:
tools/metrics/histograms/histograms.xml

https://codereview.chromium.org/2475813002/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/printer_detector/printer_detector_factory.cc
(right):

https://codereview.chromium.org/2475813002/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/printer_detector/printer_detector_factory.cc:50: return
PrinterDetector::CreateCups(Profile::FromBrowserContext(context))
On 2017/03/31 21:45:52, Carlson wrote:
> Probably slightly better to flip the contents of the if and else blocks here
> instead of inverting the condition.  if (!disabled) is a
quasi-double-negative.

Done.

[Lei Zhang, 2017-03-31 23:35:34]

Very exciting! chrome/ lgtm

https://codereview.chromium.org/2475813002/diff/120001/chrome/common/chrome_s...
File chrome/common/chrome_switches.h (right):

https://codereview.chromium.org/2475813002/diff/120001/chrome/common/chrome_s...
chrome/common/chrome_switches.h:277: extern const char kEnableLoginScreenApps[];
nit: Can you sort this section? Ditto in the .cc file.

[skau, 2017-04-01 01:02:47]

Thanks for the review.  I am also excited!

https://codereview.chromium.org/2475813002/diff/120001/chrome/common/chrome_s...
File chrome/common/chrome_switches.h (right):

https://codereview.chromium.org/2475813002/diff/120001/chrome/common/chrome_s...
chrome/common/chrome_switches.h:277: extern const char kEnableLoginScreenApps[];
On 2017/03/31 23:35:34, Lei Zhang wrote:
> nit: Can you sort this section? Ditto in the .cc file.

Done.

[skau, 2017-04-01 01:02:54]

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

[commit-bot: I haz the power, 2017-04-01 01:03:30]

Dry run: 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, 2017-04-01 02:30:13]

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

[commit-bot: I haz the power, 2017-04-01 02:30:14]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[chromium-reviews, 2017-04-03 15:16:50]

🤗

On Fri, Mar 31, 2017 at 6:02 PM, <skau@chromium.org> wrote:

> Thanks for the review. I am also excited!
>
>
> https://codereview.chromium.org/2475813002/diff/120001/
> chrome/common/chrome_switches.h
> File chrome/common/chrome_switches.h (right):
>
> https://codereview.chromium.org/2475813002/diff/120001/
> chrome/common/chrome_switches.h#newcode277
> chrome/common/chrome_switches.h:277: extern const char
> kEnableLoginScreenApps[];
> On 2017/03/31 23:35:34, Lei Zhang wrote:
> > nit: Can you sort this section? Ditto in the .cc file.
>
> Done.
>
> https://codereview.chromium.org/2475813002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[skau, 2017-04-03 18:28:51]

The CQ bit was checked by skau@chromium.org

[skau, 2017-04-03 18:28:52]

The patchset sent to the CQ was uploaded after l-g-t-m from xdai@chromium.org,
justincarlson@chromium.org, thestig@chromium.org
Link to the patchset: https://codereview.chromium.org/2475813002/#ps160001
(title: "rebase")

[commit-bot: I haz the power, 2017-04-03 18:30:03]

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

[skau, 2017-04-03 18:40:41]

Ping for review? :)

stevenjb:
chrome/browser/resources/options/browser_options.js 
chrome/browser/ui/webui/options/browser_options_handler.cc

jwd:
tools/metrics/histograms/histograms.xml

[commit-bot: I haz the power, 2017-04-03 18:47:43]

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

[commit-bot: I haz the power, 2017-04-03 18:47:44]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[stevenjb, 2017-04-03 18:53:44]

lgtm

[skau, 2017-04-04 21:14:14]

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

[skau, 2017-04-04 21:15:08]

Ping for review?  Just one more file.

jwd@: tools/metrics/histograms/histograms.xml

[commit-bot: I haz the power, 2017-04-04 21:15:09]

Dry run: 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, 2017-04-04 22:51:09]

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

[commit-bot: I haz the power, 2017-04-04 22:51:11]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[skau, 2017-04-05 19:37:12]

skau@chromium.org changed reviewers:
+ holte@chromium.org
- jwd@chromium.org

[skau, 2017-04-05 19:37:13]

holte: Would you mind approving tools/metrics/histograms/histograms.xml?  You
reviewed the original value of enable-native-cups in
https://codereview.chromium.org/1932903002

[Steven Holte, 2017-04-06 17:48:47]

lgtm

https://codereview.chromium.org/2475813002/diff/160001/tools/metrics/histogra...
File tools/metrics/histograms/histograms.xml (left):

https://codereview.chromium.org/2475813002/diff/160001/tools/metrics/histogra...
tools/metrics/histograms/histograms.xml:100668: -  <int value="-535208779"
label="enable-native-cups"/>
You might actually want to leave this in, if it is still reported by old
clients.

[skau, 2017-04-06 18:02:40]

Thanks for the reviews!

https://codereview.chromium.org/2475813002/diff/160001/tools/metrics/histogra...
File tools/metrics/histograms/histograms.xml (left):

https://codereview.chromium.org/2475813002/diff/160001/tools/metrics/histogra...
tools/metrics/histograms/histograms.xml:100668: -  <int value="-535208779"
label="enable-native-cups"/>
On 2017/04/06 17:48:47, Steven Holte wrote:
> You might actually want to leave this in, if it is still reported by old
> clients.

Thanks.  I'll remove it a few versions after we remove the flag.

[skau, 2017-04-06 18:02:54]

The CQ bit was checked by skau@chromium.org

[skau, 2017-04-06 18:02:55]

The patchset sent to the CQ was uploaded after l-g-t-m from
stevenjb@chromium.org, holte@chromium.org, thestig@chromium.org,
xdai@chromium.org, justincarlson@chromium.org
Link to the patchset: https://codereview.chromium.org/2475813002/#ps180001
(title: "git cl set-commit")

[commit-bot: I haz the power, 2017-04-06 18:04:22]

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, 2017-04-06 19:57:29]

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

[commit-bot: I haz the power, 2017-04-06 19:57:31]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[skau, 2017-04-06 21:15:58]

As far as I can tell, the test flaked.  I'm going to run it again.

[skau, 2017-04-06 21:16:08]

The CQ bit was checked by skau@chromium.org

[commit-bot: I haz the power, 2017-04-06 21:18:14]

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, 2017-04-06 23:28:39]

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

[commit-bot: I haz the power, 2017-04-06 23:28:40]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[skau, 2017-04-07 18:04:11]

The CQ bit was checked by skau@chromium.org

[skau, 2017-04-07 18:04:12]

The patchset sent to the CQ was uploaded after l-g-t-m from
stevenjb@chromium.org, holte@chromium.org, thestig@chromium.org,
xdai@chromium.org, justincarlson@chromium.org
Link to the patchset: https://codereview.chromium.org/2475813002/#ps220001
(title: "rebase")

[commit-bot: I haz the power, 2017-04-07 18:04:51]

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, 2017-04-07 19:18:53]

CQ is committing da patch.

Bot data: {"patchset_id": 220001, "attempt_start_ts": 1491588251859770,
"parent_rev": "6a6df4ec7e64dfc890c6450cd4a86e39f6754da1", "commit_rev":
"a9afc02163b48dbb93c9a343630096b8f45aec71"}

[commit-bot: I haz the power, 2017-04-07 19:19:46]

Description was changed from

==========
Enable Native CUPS printing on Chrome OS.

Flips the polarity of the --enable-native-cups flag to --disable-native-cups.

BUG=583155
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Enable Native CUPS printing on Chrome OS.

Flips the polarity of the --enable-native-cups flag to --disable-native-cups.

BUG=583155
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2475813002
Cr-Commit-Position: refs/heads/master@{#462954}
Committed:
https://chromium.googlesource.com/chromium/src/+/a9afc02163b48dbb93c9a3436300...
==========

[commit-bot: I haz the power, 2017-04-07 19:19:53]

Committed patchset #12 (id:220001) as
https://chromium.googlesource.com/chromium/src/+/a9afc02163b48dbb93c9a3436300...