Update PPDFile to PpdReference. Change the fields to a sufficient set to implement QuirksServer lo…

chromeos/printing/printer_configuration.h

 // 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.
 
 #ifndef CHROMEOS_PRINTING_PRINTER_CONFIGURATION_H_
 #define CHROMEOS_PRINTING_PRINTER_CONFIGURATION_H_
 
 #include <memory>
 #include <string>
 
 #include "base/macros.h"
 #include "chromeos/chromeos_export.h"
 
 namespace chromeos {
 
 class CHROMEOS_EXPORT Printer {
  public:
-  // Represents a Postscript Printer Description with which the printer was
-  // setup.
-  struct PPDFile {
-    // Identifier from the quirks server. -1 otherwise.
-    int id = -1;
+  // Information needed to find the PPD file for this printer.
+  struct PpdReference {
+    // If non-empty, this is the url of a specific ppd the user has specified

[Lei Zhang, 2016/10/11 23:13:28]

nit: Can we consistently refer to "PPD" as "PPD" ?

[Carlson, 2016/10/11 23:28:02]

Done.  (I think I got them all?  There are a few instances in other files in
this directory that I didn't touch.)

[Lei Zhang, 2016/10/11 23:36:24]

No worries if you didn't.

+    // for use with this printer.
+    std::string user_supplied_ppd_url;
 
-    std::string file_name;
-
-    // If there is a file with a later version on the quirks server, that file
-    // should be used. The default value is 0.
-    int version_number = 0;
-
-    // This will be true if the file was retrived from the quirks server.
-    // Otherwise, the file was saved to disk by the user.
-    bool from_quirks_server = false;
+    // The *effective* manufacturer and model of this printer, in other words,
+    // the manufacturer and model of the printer for which we grab a ppd.  This
+    // doesn’t have to (but can) be the actual manufacturer/model of the
+    // printer.  We should always try to fill these fields in, even if we don’t
+    // think they’ll be needed, as they provide a fallback mechanism for
+    // finding a ppd.
+    std::string effective_manufacturer;
+    std::string effective_model;
   };
 
   // Constructs a printer object that is completely empty.
   Printer();
 
   // Constructs a printer object with an id.
   explicit Printer(const std::string& id);
 
   // Copy constructor and assignment.
   explicit Printer(const Printer& printer);
@@ skipping 18 matching lines @@
   void set_manufacturer(const std::string& manufacturer) {
     manufacturer_ = manufacturer;
   }
 
   const std::string& model() const { return model_; }
   void set_model(const std::string& model) { model_ = model; }
 
   const std::string& uri() const { return uri_; }
   void set_uri(const std::string& uri) { uri_ = uri; }
 
-  const PPDFile& ppd() const { return ppd_; }
-  void SetPPD(std::unique_ptr<PPDFile> ppd);
+  const PpdReference& ppd_reference() const { return ppd_reference_; }

[Lei Zhang, 2016/10/11 23:13:28]

Some places do:
const Foo* foo() const;
Foo* foo();

And the compiler will pick the right one when you call foo().

[Carlson, 2016/10/11 23:28:02]

I understand the appeal of that approach, but I prefer it this way because

a)  That pattern seems to be a bit questionable w.r.t. C++ style guide function
overloading rules on being able to immediately tell which variant is being
called

b) It's a small thing, but we're going to be passing PpdReferences around in the
code soon, almost certainly as const references, so I'd rather be able to write
 
DoStuff(printer.ppd_reference()) 

instead of

DoStuff(*printer.ppd_reference()); 

Will change it if you feel strongly.

[Lei Zhang, 2016/10/11 23:36:24]

Alrighty.
I don't think its a style guide problem, at least not in Chromium land. It's
used in many places.

+  PpdReference* mutable_ppd_reference() { return &ppd_reference_; }
+  void set_ppd_reference(const PpdReference& ppd_reference) {

[Lei Zhang, 2016/10/11 23:13:28]

If you already have mutable_ppd_reference(), can't you just get rid of this and
write the following instead?

PpdReference* p_pointer = foo.mutable_ppd_reference();
*p_pointer = new_p;

[Carlson, 2016/10/11 23:28:02]

Done.

+    ppd_reference_ = ppd_reference;
+  }
 
   const std::string& uuid() const { return uuid_; }
   void set_uuid(const std::string& uuid) { uuid_ = uuid; }
 
  private:
   // Globally unique identifier. Empty indicates a new printer.
   std::string id_;
 
   // User defined string for printer identification.
   std::string display_name_;
 
   // User defined string for additional printer information.
   std::string description_;
 
   // The manufacturer of the printer, e.g. HP
   std::string manufacturer_;
 
   // The model of the printer, e.g. OfficeJet 415
   std::string model_;
 
   // The full path for the printer. Suitable for configuration in CUPS.
   // Contains protocol, hostname, port, and queue.
   std::string uri_;
 
-  // The associated postscript printer description.
-  PPDFile ppd_;
+  // How to find the associated postscript printer description.
+  PpdReference ppd_reference_;
 
   // The UUID from an autoconf protocol for deduplication. Could be empty.
   std::string uuid_;
 };
 
 }  // namespace chromeos
 
 #endif  // CHROMEOS_PRINTING_PRINTER_CONFIGURATION_H_