[CUPS] Implement the "Printers" section in chrome://settings.

[CUPS] Implement the "Printers" section in chrome://settings.

The CUPS printer setup UI is currently implemented under MD Settings.

Because we are planning to launch CUPS with M56 and CrOS MD Settings
will launch in a following milestone, we implement the "Printers" section
in current chrome://settings to link to the CUPS printer MD settings UI
from the existing chrome://settings.

Note: This change is temporary and will be reverted later in M57.

BUG=658928, 626752
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/b0e2e70344284572abfdf5c5dc2aa15fc3a32559
Cr-Commit-Position: refs/heads/master@{#430039}

[xdai1, 2016-11-02 20:33:19]

Description was changed from

==========
[CUPS] Implement the "Printers" section in chrome://settings.

The CUPS printer setup UI is currently implemented under MD Settings.

Because we are planning to launch CUPS with M56 and CrOS MD Settings
will launch in a following milestone, we implement the "Printers" section
in current chrome://settings to link to the CUPS printer MD settings UI
from the existing chrome://settings.

Note: This change is temporary and will be reverted later in M57.

BUG=658928, 626752
==========

to

==========
[CUPS] Implement the "Printers" section in chrome://settings.

The CUPS printer setup UI is currently implemented under MD Settings.

Because we are planning to launch CUPS with M56 and CrOS MD Settings
will launch in a following milestone, we implement the "Printers" section
in current chrome://settings to link to the CUPS printer MD settings UI
from the existing chrome://settings.

Note: This change is temporary and will be reverted later in M57.

BUG=658928, 626752
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[xdai1, 2016-11-02 20:40:02]

Patchset #1 (id:1) has been deleted

[xdai1, 2016-11-02 20:42:00]

xdai@chromium.org changed reviewers:
+ michaelpg@chromium.org, stevenjb@chromium.org

[xdai1, 2016-11-02 20:42:00]

stevenjb@, michaelpg@, could you help review this CL please? Thanks!

[xdai1, 2016-11-02 20:45:32]

On 2016/11/02 20:42:00, xdai1 wrote:
> stevenjb@, michaelpg@, could you help review this CL please? Thanks!

FYI: see
https://docs.google.com/presentation/d/1z9tntdvKHNloLrs2zyjLWeNzo9eDl6qVUA3e1...
for the mock. Thanks!

[xdai1, 2016-11-03 23:04:02]

On 2016/11/02 20:45:32, xdai1 wrote:
> On 2016/11/02 20:42:00, xdai1 wrote:
> > stevenjb@, michaelpg@, could you help review this CL please? Thanks!
> 
> FYI: see
>
https://docs.google.com/presentation/d/1z9tntdvKHNloLrs2zyjLWeNzo9eDl6qVUA3e1...
> for the mock. Thanks!

kindly ping? Could you help review this CL please? The feature freeze date is
tomorrow. Thanks!

[stevenjb, 2016-11-04 00:01:28]

lgtm

https://codereview.chromium.org/2464403003/diff/20001/chrome/browser/resource...
File chrome/browser/resources/options/browser_options.html (right):

https://codereview.chromium.org/2464403003/diff/20001/chrome/browser/resource...
chrome/browser/resources/options/browser_options.html:765: <!--TODO(xdai):
Remove this "cups-printers-section" section in M57. -->
Why will we remove just this section in M57? Eventually all of the options/ code
will be removed when we switch to the new Settings UI, but we don't need a TODO
for each section, unless there is another reason (in which case reference a
crbug).

https://codereview.chromium.org/2464403003/diff/20001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/options/browser_options_handler.cc (right):

https://codereview.chromium.org/2464403003/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/options/browser_options_handler.cc:250: #endif  //
defined(OS_CHROMEOS)
Move this down to large existing OS_CHROMEOS section below.

https://codereview.chromium.org/2464403003/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/options/browser_options_handler.cc:270: #endif  //
defined(OS_CHROMEOS)
These too.

[xdai1, 2016-11-04 00:07:41]

stevenjb@, I've addressed your comments. Please take another look. Thanks!

https://codereview.chromium.org/2464403003/diff/20001/chrome/browser/resource...
File chrome/browser/resources/options/browser_options.html (right):

https://codereview.chromium.org/2464403003/diff/20001/chrome/browser/resource...
chrome/browser/resources/options/browser_options.html:765: <!--TODO(xdai):
Remove this "cups-printers-section" section in M57. -->
On 2016/11/04 00:01:28, stevenjb wrote:
> Why will we remove just this section in M57? Eventually all of the options/
code
> will be removed when we switch to the new Settings UI, but we don't need a
TODO
> for each section, unless there is another reason (in which case reference a
> crbug).
> 

I see. Removed the TODO.

https://codereview.chromium.org/2464403003/diff/20001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/options/browser_options_handler.cc (right):

https://codereview.chromium.org/2464403003/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/options/browser_options_handler.cc:250: #endif  //
defined(OS_CHROMEOS)
On 2016/11/04 00:01:28, stevenjb wrote:
> Move this down to large existing OS_CHROMEOS section below.

In line 239, it says "keep these in alphabetical order"...

https://codereview.chromium.org/2464403003/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/options/browser_options_handler.cc:270: #endif  //
defined(OS_CHROMEOS)
On 2016/11/04 00:01:28, stevenjb wrote:
> These too.

ditto.

[stevenjb, 2016-11-04 00:27:17]

https://codereview.chromium.org/2464403003/diff/20001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/options/browser_options_handler.cc (right):

https://codereview.chromium.org/2464403003/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/options/browser_options_handler.cc:250: #endif  //
defined(OS_CHROMEOS)
On 2016/11/04 00:07:41, xdai1 wrote:
> On 2016/11/04 00:01:28, stevenjb wrote:
> > Move this down to large existing OS_CHROMEOS section below.
> 
> In line 239, it says "keep these in alphabetical order"...

There is a separate alphebetized section for OS_CHROMEOS starting around line
400. Otherwise there would be a huge number of #ifdef's :)

[xdai1, 2016-11-04 05:29:45]

stevenjb@, I've addressed your comments. Thanks for your review.

https://codereview.chromium.org/2464403003/diff/20001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/options/browser_options_handler.cc (right):

https://codereview.chromium.org/2464403003/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/options/browser_options_handler.cc:250: #endif  //
defined(OS_CHROMEOS)
On 2016/11/04 00:27:17, stevenjb wrote:
> On 2016/11/04 00:07:41, xdai1 wrote:
> > On 2016/11/04 00:01:28, stevenjb wrote:
> > > Move this down to large existing OS_CHROMEOS section below.
> > 
> > In line 239, it says "keep these in alphabetical order"...
> 
> There is a separate alphebetized section for OS_CHROMEOS starting around line
> 400. Otherwise there would be a huge number of #ifdef's :)

Ah. I see. Didn't noticed that there is a seprate section:) Addressed.

[michaelpg, 2016-11-04 17:58:08]

lgtm

https://codereview.chromium.org/2464403003/diff/60001/chrome/browser/resource...
File chrome/browser/resources/options/browser_options.html (right):

https://codereview.chromium.org/2464403003/diff/60001/chrome/browser/resource...
chrome/browser/resources/options/browser_options.html:771:
i18n-values="href:cupsPrintLearnMoreURL">
nit: indent 4 spaces

[stevenjb, 2016-11-04 18:08:52]

lgtm++

[xdai1, 2016-11-04 18:19:20]

michaelpg@, stevenjb@, thanks for the review!

https://codereview.chromium.org/2464403003/diff/60001/chrome/browser/resource...
File chrome/browser/resources/options/browser_options.html (right):

https://codereview.chromium.org/2464403003/diff/60001/chrome/browser/resource...
chrome/browser/resources/options/browser_options.html:771:
i18n-values="href:cupsPrintLearnMoreURL">
On 2016/11/04 17:58:08, michaelpg wrote:
> nit: indent 4 spaces

Done.

[xdai1, 2016-11-04 18:19:24]

The CQ bit was checked by xdai@chromium.org

[xdai1, 2016-11-04 18:19:24]

The patchset sent to the CQ was uploaded after l-g-t-m from
stevenjb@chromium.org, michaelpg@chromium.org
Link to the patchset: https://codereview.chromium.org/2464403003/#ps80001
(title: "Address michaelpg@'s comment.")

[commit-bot: I haz the power, 2016-11-04 18:19: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, 2016-11-04 22:38:22]

Committed patchset #4 (id:80001)

[commit-bot: I haz the power, 2016-11-04 22:40:00]

Description was changed from

==========
[CUPS] Implement the "Printers" section in chrome://settings.

The CUPS printer setup UI is currently implemented under MD Settings.

Because we are planning to launch CUPS with M56 and CrOS MD Settings
will launch in a following milestone, we implement the "Printers" section
in current chrome://settings to link to the CUPS printer MD settings UI
from the existing chrome://settings.

Note: This change is temporary and will be reverted later in M57.

BUG=658928, 626752
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
[CUPS] Implement the "Printers" section in chrome://settings.

The CUPS printer setup UI is currently implemented under MD Settings.

Because we are planning to launch CUPS with M56 and CrOS MD Settings
will launch in a following milestone, we implement the "Printers" section
in current chrome://settings to link to the CUPS printer MD settings UI
from the existing chrome://settings.

Note: This change is temporary and will be reverted later in M57.

BUG=658928, 626752
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/b0e2e70344284572abfdf5c5dc2aa15fc3a32559
Cr-Commit-Position: refs/heads/master@{#430039}
==========

[commit-bot: I haz the power, 2016-11-04 22:40:01]

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