Track printer installations for each configuration.

chrome/browser/chromeos/printing/printers_manager.cc

 // 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.
 
 #include "chrome/browser/chromeos/printing/printers_manager.h"
 
 #include <memory>
 #include <string>
 #include <utility>
 #include <vector>
@@ skipping 72 matching lines @@
   }
 
   return printers;
 }
 
 std::vector<std::unique_ptr<Printer>> PrintersManager::GetRecommendedPrinters()
     const {
   std::vector<std::unique_ptr<Printer>> printers;
 
   for (const std::string& key : recommended_printer_ids_) {
-    const base::DictionaryValue& printer_dictionary =
-        *(recommended_printers_.at(key));
-    std::unique_ptr<Printer> printer =
-        printing::RecommendedPrinterToPrinter(printer_dictionary);
-    if (printer) {
-      printer->set_source(Printer::SRC_POLICY);
-      printers.push_back(std::move(printer));
+    auto printer = recommended_printers_.find(key);
+    if (printer != recommended_printers_.end()) {
+      printers.push_back(base::MakeUnique<Printer>(*printer->second));
     }
   }
 
   return printers;
 }
 
 std::unique_ptr<Printer> PrintersManager::GetPrinter(
     const std::string& printer_id) const {
   // check for a policy printer first
   const auto& policy_printers = recommended_printers_;
   auto found = policy_printers.find(printer_id);
-  if (found != policy_printers.end())
-    return printing::RecommendedPrinterToPrinter(*(found->second));
+  if (found != policy_printers.end()) {
+    // Copy a printer.
+    return base::MakeUnique<Printer>(*(found->second));
+  }
 
   base::Optional<sync_pb::PrinterSpecifics> printer =
       sync_bridge_->GetPrinter(printer_id);
   return printer.has_value() ? printing::SpecificsToPrinter(*printer) : nullptr;
 }
 
 void PrintersManager::RegisterPrinter(std::unique_ptr<Printer> printer) {
   if (printer->id().empty()) {
     printer->set_id(base::GenerateGUID());
   }
@@ skipping 46 matching lines @@
   return sync_bridge_.get();
 }
 
 // This method is not thread safe and could interact poorly with readers of
 // |recommended_printers_|.
 void PrintersManager::UpdateRecommendedPrinters() {
   const PrefService* prefs = profile_->GetPrefs();
 
   const base::ListValue* values =
       prefs->GetList(prefs::kRecommendedNativePrinters);
+  const base::Time timestamp = base::Time::Now();
 
-  recommended_printer_ids_.clear();
+  // Parse the policy JSON into new structures.
+  std::vector<std::string> new_ids;
+  std::map<std::string, std::unique_ptr<Printer>> new_printers;
   for (const auto& value : *values) {
     std::string printer_json;
     if (!value.GetAsString(&printer_json)) {
       NOTREACHED();
       continue;
     }
 
     std::unique_ptr<base::DictionaryValue> printer_dictionary =
         base::DictionaryValue::From(base::JSONReader::Read(
             printer_json, base::JSON_ALLOW_TRAILING_COMMAS));
 
     if (!printer_dictionary) {
       LOG(WARNING) << "Ignoring invalid printer.  Invalid JSON object: "
                    << printer_json;
       continue;
     }
 
     // Policy printers don't have id's but the ids only need to be locally
     // unique so we'll hash the record.  This will not collide with the UUIDs
     // generated for user entries.
     std::string id = base::MD5String(printer_json);
     printer_dictionary->SetString(printing::kPrinterId, id);
 
-    recommended_printer_ids_.push_back(id);
-    recommended_printers_[id] = std::move(printer_dictionary);
+    new_ids.push_back(id);
+    // Move existing printers, create othewise.
+    auto old = recommended_printers_.find(id);
+    if (old != recommended_printers_.end()) {

[Carlson, 2017/05/08 21:01:17]

Generally looks fine, but back to my original comment, I think this behaves
badly if we have multiple entries for the same printer.  We'll end up moving the
existing entry into the new map twice, which will cause the entry in
new_printers to be null.

You could just erase the entry from the existing map after you move it to avoid
this, or do something else, but I think it's worth taking care of this.

[skau, 2017/05/08 21:19:25]

We expect all printer entries to be unique within the collection.  But I'll skip
duplicates to avoid this scenario.

+      new_printers[id] = std::move(old->second);
+    } else {
+      auto printer =
+          printing::RecommendedPrinterToPrinter(*printer_dictionary, timestamp);
+      printer->set_source(Printer::SRC_POLICY);
+
+      new_printers[id] = std::move(printer);
+    }
   }
+
+  // Objects not in the most recent update get deallocated after method exit.
+  recommended_printer_ids_.swap(new_ids);
+  recommended_printers_.swap(new_printers);
+}
+
+void PrintersManager::PrinterInstalled(const Printer& printer) {
+  DCHECK(!printer.last_updated().is_null());
+  installed_printer_timestamps_[printer.id()] = printer.last_updated();
+}
+
+bool PrintersManager::IsConfigurationCurrent(const Printer& printer) {
+  auto found = installed_printer_timestamps_.find(printer.id());
+  if (found == installed_printer_timestamps_.end())
+    return false;
+
+  return found->second == printer.last_updated();
 }
 
 }  // namespace chromeos