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

chromeos/printing/printer_configuration.h

Index: chromeos/printing/printer_configuration.h
diff --git a/chromeos/printing/printer_configuration.h b/chromeos/printing/printer_configuration.h
index fc82c293798c94a84e7f71e5216374fdf54c69d9..b629c121d2532710d306d9d3732ad0a140ee998c 100644
--- a/chromeos/printing/printer_configuration.h
+++ b/chromeos/printing/printer_configuration.h
@@ -15,21 +15,20 @@ 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;
-
-    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;
+  // 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;
+
+    // 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.
@@ -68,8 +67,11 @@ class CHROMEOS_EXPORT Printer {
   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; }
@@ -94,8 +96,8 @@ class CHROMEOS_EXPORT Printer {
   // 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_;