Track printer installations for each configuration.

chrome/browser/chromeos/printing/printers_manager.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 CHROME_BROWSER_CHROMEOS_PRINTING_PRINTERS_MANAGER_H_
 #define CHROME_BROWSER_CHROMEOS_PRINTING_PRINTERS_MANAGER_H_
 
 #include <map>
 #include <memory>
 #include <string>
@@ skipping 37 matching lines @@
   // Returns the printers that are saved in preferences.
   std::vector<std::unique_ptr<Printer>> GetPrinters() const;
 
   // Returns printers from enterprise policy.
   std::vector<std::unique_ptr<Printer>> GetRecommendedPrinters() const;
 
   // Returns the printer with id |printer_id|, or nullptr if no such
   // printer exists.
   std::unique_ptr<Printer> GetPrinter(const std::string& printer_id) const;
 
   // Adds or updates a printer. Printers are identified by the id field.  Use an

[Lei Zhang, 2017/05/08 21:42:48]

BTW, this comment looks out of date.

[skau, 2017/05/09 00:52:01]

Done.

[Lei Zhang, 2017/05/09 01:14:39]

Err, what I mean was this felt out of date because there was no "id" parameter
to RegisterPrinter(). Maybe this can be:

The |printer| is identified by its id field. Those with an empty id are treated
as new printers.

[skau, 2017/05/09 19:39:26]

Thanks.  That phrasing is clearer.

   // empty id to add a new printer.
   void RegisterPrinter(std::unique_ptr<Printer> printer);
 
   // Remove printer from preferences with the id |printer_id|.  Returns true if
   // the printer was successfully removed.
   bool RemovePrinter(const std::string& printer_id);
 
   // Attach |observer| for notification of events.  |observer| is expected to
   // live on the same thread (UI) as this object.  OnPrinter* methods are
   // invoked inline so calling RegisterPrinter in response to OnPrinterAdded is
   // forbidden.
   void AddObserver(PrintersManager::Observer* observer);
 
   // Remove |observer| so that it no longer receives notifications.  After the
   // completion of this method, the |observer| can be safely destroyed.
   void RemoveObserver(PrintersManager::Observer* observer);
 
   // Returns a ModelTypeSyncBridge for the sync client.
   PrintersSyncBridge* GetSyncBridge();
 
+  // Registers that the printer was installed in CUPS.

[Lei Zhang, 2017/05/08 21:42:48]

One of the PrinterInstalled() callers is as follows:

manager->PrinterInstalled(*printer);
manager->RegisterPrinter(std::move(printer));

Can you better explain the difference between PrinterInstalled(), which
"registers the printer" in some way, vs RegisterPrinter()?

[skau, 2017/05/09 00:52:01]

I've tried to clarify the comment on RegisterPrinter to distinguish between the
two methods.  Let me know if you think it can be clearer.

[Lei Zhang, 2017/05/09 01:14:39]

Yes, definitely clearer.

+  void PrinterInstalled(const Printer& printer);
+
+  // Returns true if the printer is installed in CUPS.
+  bool IsConfigurationCurrent(const Printer& printer);

[Lei Zhang, 2017/05/08 21:42:48]

const method?

[skau, 2017/05/09 00:52:01]

Done.

+
  private:
   // Updates the in-memory recommended printer list.
   void UpdateRecommendedPrinters();
 
   Profile* profile_;
   PrefChangeRegistrar pref_change_registrar_;
 
   // The backend for profile printers.
   std::unique_ptr<PrintersSyncBridge> sync_bridge_;
 
   // Contains the keys for all recommended printers in order so we can return
   // the list of recommended printers in the order they were received.
   std::vector<std::string> recommended_printer_ids_;
-  std::map<std::string, std::unique_ptr<base::DictionaryValue>>
-      recommended_printers_;
+  std::map<std::string, std::unique_ptr<Printer>> recommended_printers_;
+
+  // Map of printer ids to installation timestamps.
+  std::map<std::string, base::Time> installed_printer_timestamps_;
 
   base::ObserverList<Observer> observers_;
 
   DISALLOW_COPY_AND_ASSIGN(PrintersManager);
 };
 
 }  // namespace chromeos
 
 #endif  // CHROME_BROWSER_CHROMEOS_PRINTING_PRINTERS_MANAGER_H_