Update CUPS settings UI to allow USB printers to be added via discovery.

chrome/browser/resources/settings/printing_page/cups_add_printer_dialog.js

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 /**
  * @fileoverview 'settings-cups-add-printer-dialog' includes multiple dialogs to
  * set up a new CUPS printer.
  * Subdialogs include:
  * - 'add-printer-discovery-dialog' is a dialog showing discovered printers on
  *   the network that are available for setup.
@@ skipping 32 matching lines @@
     discoveredPrinters: {
       type: Array,
     },
 
     /** @type {!CupsPrinterInfo} */
     selectedPrinter: {
       type: Object,
       notify: true,
     },
 
+    /** @type {!CupsPrinterInfo} */
+    newPrinter: {
+      type: Object,
+      notify: true,

[xdai1, 2017/04/24 21:42:12]

I think put the new property newPrinter in manufacturer-model-dialog might make
more sense. 

Previously the flow is discovery-dialog->configuring-dialog and
manual-add-printer-dialog->manufacturer-model-dialog->configuring-dialog. Now
the flow changed. So I think some of the codes might need to be revisited to
make sure they still make sense. For example, openConfiguringPrinterDialog_()
may not work as expected after your change.

[Carlson, 2017/05/01 20:59:49]

Sorry, I don't really understand what you mean by putting newPrinter in
manufacturer-model-dialog.  

I think this really means I don't fully understand how Polymer data binding
works.  So I'm going to describe what I'm trying to do, and hopefully you can
point me at the right way to do it.  :)

When I have the discovery dialog open, and the user selects a printer, I need to
get the information about the selected printer over to the manufacturer-model
dialog, because the user needs to supply more information before we can set up
the discovered printer.  (Eventually we'll be in a situation where we only want
to go to the manu/model dialog if the printer setup fails.  However, right now
we only discover USB devices that are not configured, and since USB printer
configuration is attempted automatically when the device is plugged in, the only
way a USB printer shows up in the discovery list is if configuration failed and
we need more information to set it up).

So what I want to have happen is that add-printer-manufacturer-model's
'newPrinter' property is set to whatever the discovery-dialog's
'selectedPrinter' property was when we change dialogs.

The way (I understand) this works in this code is that we bind both
add-printer-discovery-dialog.newPrinter and
add-printer-manufacturer-model-dialog.newPrinter to
settings-cups-add-printer-dialog.newPrinter which makes them (effectively) the
same object.  (I gather there's some serialization and deserialization going on
behind the scenes, but I think that's not important here?)

So given how this works, I think I need the newPrinter property declared in all
three places -- the top level dialog and both the discovery and
manufacturer-model dialogs.  

(note that the previous version of this that I sent to you effectively bound
add-printer-discovery-dialog.selectedPrinter to newPrinter, but you correctly
pointed out that we only want to do this if the user has actually decided to add
a discovered printer, otherwise we mess up the state of newPrinter).

So what am I missing here?

[xdai1, 2017/05/02 17:25:08]

Sorry I didn't explain it in a clear way.

I understand what you were trying to do here and functionally it can do what you
want: data-binding works as expected as you described. However, IMO I think it
might be better to do it in another way.

Previously when I implemented this, the expected flow is when a user selected a
printer from discovery-dialog, it would jump directly to configuring-dialog.
manufacturer-model-dialog was only used in manual-add-printer flow. So
manufacturer-model-dialog only needs maintain the 'newPrinter' property from
manual-add-printer-dialog. 

Now we changed the flow: when a user selectes a printer from discovery-dialog,
it jumps to manufacturer-model-dialog and then to configuring-dialog. I think
instead of putting 'newPrinter' property in discovery-dialog, we should put
'selectedPrinter' property in manufacturer-model-dialog. This way,
discovery-dialog only maintains 'selectedPrinter' property (and that's the only
thing it should know), and manufacturer-model-dialog maintains 'newPrinter'
property from manual-add-printer-dialog and 'selectedPrinter' property from
discovery-dialog, and the two properties are binded to the corresponding
properties in  settings-cups-add-printer-dialog. 

I think this way we don't mess up the two flows: auto-discovery flow and
manual-setup-flow and also keep two properties for the two flows:
selectedPrinter for auto-discovery-flow and newPrinter for manual-setup-flow.
What do you think?

+    },
+
     discovering_: {
       type: Boolean,
       value: true,
     },
   },
 
   /** @override */
   ready: function() {
     settings.CupsPrintersBrowserProxyImpl.getInstance().
         startDiscoveringPrinters();
@@ skipping 40 matching lines @@
     this.fire('open-manually-add-printer-dialog');
   },
 
   /** @private */
   onCancelTap_: function() {
     this.stopDiscoveringPrinters_();
     this.$$('add-printer-dialog').close();
   },
 
   /** @private */
-  switchToConfiguringDialog_: function() {
+  switchToManufacturerDialog_: function() {
     this.stopDiscoveringPrinters_();
+
+    // Copy relevant fields over from selected printer,
+    // clear the rest.
+    this.newPrinter = {
+      printerAddress: this.selectedPrinter.printerAddress,
+      printerDescription: this.selectedPrinter.printerDescription,
+      printerId: this.selectedPrinter.printerId,
+      printerName: this.selectedPrinter.printerName,
+      printerProtocol: this.selectedPrinter.printerProtocol,
+      printerQueue: this.selectedPrinter.printerQueue,
+      printerStatus: this.selectedPrinter.printerStatus,
+      printerManufacturer: '',
+      printerModel: '',
+      printerPPDPath: '',
+    };
     this.$$('add-printer-dialog').close();
-    this.fire('open-configuring-printer-dialog');
+    this.fire('open-manufacturer-model-dialog');
   },
 });
 
 Polymer({
   is: 'add-printer-manually-dialog',
 
   properties: {
     /** @type {!CupsPrinterInfo} */
     newPrinter: {
       type: Object,
@@ skipping 198 matching lines @@
     newPrinter: {
       type: Object,
     },
 
     /** @type {boolean} whether the new printer setup is failed. */
     setupFailed: {
       type: Boolean,
       value: false,
     },
 
+    /** @type {boolean} whether we're doing manufacturer/model in
+        a manual flow (as opposed to a discovery flow). */

[xdai1, 2017/04/24 21:42:12]

The format should be
/**
 * @type {boolean} comment...
 *     (comment to be continued...)
 */

[Carlson, 2017/05/01 20:59:49]

done.

+    inManualFlow: {

[xdai1, 2017/04/24 21:42:12]

this should be a private property.

[Carlson, 2017/05/01 20:59:49]

Done.

+      type: Boolean,
+      value: false,
+    },
+
     configuringDialogTitle: String,
 
     /** @private {string} */
     previousDialog_: String,
 
     /** @private {string} */
     currentDialog_: String,
 
     /** @private {boolean} */
     showDiscoveryDialog_: Boolean,
@@ skipping 17 matching lines @@
   },
 
   /** @override */
   ready: function() {
     this.addWebUIListener('on-add-cups-printer', this.onAddPrinter_.bind(this));
   },
 
   /** Opens the Add printer discovery dialog. */
   open: function() {
     this.resetData_();
-    this.switchDialog_(
-        '', AddPrinterDialogs.MANUALLY, 'showManuallyAddDialog_');
+    this.switchDialog_('', AddPrinterDialogs.DISCOVERY, 'showDiscoveryDialog_');
   },
 
   /**
    * Reset all the printer data in the Add printer flow.
    * @private
    */
   resetData_: function() {
     if (this.selectedPrinter)
       this.selectedPrinter = this.getEmptyPrinter_();
     if (this.newPrinter)
@@ skipping 47 matching lines @@
           loadTimeData.getString('addPrintersManuallyTitle');
       settings.CupsPrintersBrowserProxyImpl.getInstance().addCupsPrinter(
           this.newPrinter);
     }
   },
 
   /** @private */
   openManufacturerModelDialog_: function() {
     this.switchDialog_(this.currentDialog_, AddPrinterDialogs.MANUFACTURER,
                        'showManufacturerDialog_');
+    this.inManualFlow = (this.previousDialog_ == AddPrinterDialogs.MANUALLY);
   },
 
   /** @private */
   configuringDialogClosed_: function() {
     if (this.previousDialog_ == AddPrinterDialogs.MANUALLY) {
       this.switchDialog_(
           this.currentDialog_, this.previousDialog_, 'showManuallyAddDialog_');
     } else if (this.previousDialog_ == AddPrinterDialogs.MANUFACTURER) {
       this.switchDialog_(
           this.currentDialog_, this.previousDialog_, 'showManufacturerDialog_');
@@ skipping 43 matching lines @@
   onAddPrinter_: function(success, printerName) {
     this.$$('add-printer-configuring-dialog').close();
     if (success)
       return;
 
     if (this.previousDialog_ == AddPrinterDialogs.MANUFACTURER) {
       this.setupFailed = true;
     }
   },
 });