[CUPS] Implement the CUPS printer PPD file picker in the manufacturer and model dialog.

[CUPS] Implement the CUPS printer PPD file picker in the manufacturer and model dialog.

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

Committed: https://crrev.com/9e0f12bd0fb775bc066936e2a1b891fa57b88dda
Cr-Commit-Position: refs/heads/master@{#419352}

[xdai1, 2016-09-10 00:33:40]

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

[xdai1, 2016-09-10 00:33:40]

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

[xdai1, 2016-09-10 00:35:15]

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

[commit-bot: I haz the power, 2016-09-10 00:35:47]

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, 2016-09-10 01:45:32]

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

[commit-bot: I haz the power, 2016-09-10 01:45:33]

Dry run: This issue passed the CQ dry run.

[xdai1, 2016-09-13 00:29:14]

Patchset #1 (id:1) has been deleted

[michaelpg, 2016-09-15 03:01:39]

Looks good with a few suggestions!

https://codereview.chromium.org/2332483002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/printing_page/cups_add_printer_dialog.js
(right):

https://codereview.chromium.org/2332483002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/printing_page/cups_add_printer_dialog.js:87:
printerPPDPath: '',
nit: keep alphabetized?

https://codereview.chromium.org/2332483002/diff/20001/chrome/browser/resource...
File
chrome/browser/resources/settings/printing_page/cups_printers_browser_proxy.js
(right):

https://codereview.chromium.org/2332483002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/printing_page/cups_printers_browser_proxy.js:21:
*   printerPPDPath: string
alpha?

https://codereview.chromium.org/2332483002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/printing_page/cups_printers_browser_proxy.js:56:
* @return {!Promise<string>} the full path of the printer PPD file.
capitalize description

https://codereview.chromium.org/2332483002/diff/20001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/settings/chromeos/cups_printers_handler.cc (right):

https://codereview.chromium.org/2332483002/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/settings/chromeos/cups_printers_handler.cc:125: if
(!PathService::Get(chrome::DIR_DEFAULT_DOWNLOADS, &downloads_path))
We probably want to use the user's preferred downloads directory, not the
default system downloads directory. See DownloadPrefs::DownloadPath().

[xdai1, 2016-09-16 21:19:32]

michaelpg@, I've addressed your comments. Please take another look, thanks!

https://codereview.chromium.org/2332483002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/printing_page/cups_add_printer_dialog.js
(right):

https://codereview.chromium.org/2332483002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/printing_page/cups_add_printer_dialog.js:87:
printerPPDPath: '',
On 2016/09/15 03:01:39, michaelpg wrote:
> nit: keep alphabetized?

Done.

https://codereview.chromium.org/2332483002/diff/20001/chrome/browser/resource...
File
chrome/browser/resources/settings/printing_page/cups_printers_browser_proxy.js
(right):

https://codereview.chromium.org/2332483002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/printing_page/cups_printers_browser_proxy.js:21:
*   printerPPDPath: string
On 2016/09/15 03:01:39, michaelpg wrote:
> alpha?

Done.

https://codereview.chromium.org/2332483002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/printing_page/cups_printers_browser_proxy.js:56:
* @return {!Promise<string>} the full path of the printer PPD file.
On 2016/09/15 03:01:39, michaelpg wrote:
> capitalize description

Done.

https://codereview.chromium.org/2332483002/diff/20001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/settings/chromeos/cups_printers_handler.cc (right):

https://codereview.chromium.org/2332483002/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/settings/chromeos/cups_printers_handler.cc:125: if
(!PathService::Get(chrome::DIR_DEFAULT_DOWNLOADS, &downloads_path))
On 2016/09/15 03:01:39, michaelpg wrote:
> We probably want to use the user's preferred downloads directory, not the
> default system downloads directory. See DownloadPrefs::DownloadPath().

Done.

[michaelpg, 2016-09-16 23:22:32]

lgtm

[xdai1, 2016-09-16 23:31:05]

Thanks for your review!

[xdai1, 2016-09-16 23:31:09]

The CQ bit was checked by xdai@chromium.org

[commit-bot: I haz the power, 2016-09-16 23:32:23]

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-09-16 23:42:45]

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

[commit-bot: I haz the power, 2016-09-16 23:42:47]

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

[xdai1, 2016-09-16 23:48:59]

The CQ bit was checked by xdai@chromium.org

[xdai1, 2016-09-16 23:49:00]

The patchset sent to the CQ was uploaded after l-g-t-m from
michaelpg@chromium.org
Link to the patchset: https://codereview.chromium.org/2332483002/#ps60001
(title: "Fix the failed closure_compilation trybot.")

[commit-bot: I haz the power, 2016-09-16 23:49:39]

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-09-17 00:56:36]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2016-09-17 00:58:03]

Description was changed from

==========
[CUPS] Implement the CUPS printer PPD file picker in the manufacturer and model
dialog.

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

to

==========
[CUPS] Implement the CUPS printer PPD file picker in the manufacturer and model
dialog.

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

Committed: https://crrev.com/9e0f12bd0fb775bc066936e2a1b891fa57b88dda
Cr-Commit-Position: refs/heads/master@{#419352}
==========

[commit-bot: I haz the power, 2016-09-17 00:58:04]

Patchset 3 (id:??) landed as
https://crrev.com/9e0f12bd0fb775bc066936e2a1b891fa57b88dda
Cr-Commit-Position: refs/heads/master@{#419352}