Sync Printer information for Chrome OS.

Move printer preference storage from user preference JSON to LevelDB via ModelTypeStore. Enable User Sync Service for the syncer::PRINTERS type to synchronize printer configuration information between devices. BUG=616862 Review-Url: https://codereview.chromium.org/2656813004 Cr-Commit-Position: refs/heads/master@{#449216}
marcosholgado · Feb 9, 2017 · 5be8b58 · 5be8b58
1 parent 4be275a
commit 5be8b58
Show file tree

Hide file tree

Showing 19 changed files with 1,207 additions and 133 deletions.
diff --git a/chrome/browser/chromeos/BUILD.gn b/chrome/browser/chromeos/BUILD.gn
@@ -1196,6 +1196,8 @@ source_set("chromeos") {
     "printing/printer_pref_manager.h",
     "printing/printer_pref_manager_factory.cc",
     "printing/printer_pref_manager_factory.h",
+    "printing/printers_sync_bridge.cc",
+    "printing/printers_sync_bridge.h",
     "printing/specifics_translation.cc",
     "printing/specifics_translation.h",
     "profiles/avatar_menu_actions_chromeos.cc",

diff --git a/chrome/browser/chromeos/printing/DEPS b/chrome/browser/chromeos/printing/DEPS
@@ -0,0 +1,5 @@
+include_rules = [
+  "+components/sync/base",
+  "+components/sync/model",
+  "+components/sync/protocol",
+]
diff --git a/chrome/browser/chromeos/printing/printer_pref_manager.cc b/chrome/browser/chromeos/printing/printer_pref_manager.cc
@@ -13,61 +13,48 @@
 #include "base/json/json_reader.h"
 #include "base/md5.h"
 #include "base/memory/ptr_util.h"
+#include "base/optional.h"
 #include "base/values.h"
+#include "chrome/browser/chromeos/printing/printers_sync_bridge.h"
+#include "chrome/browser/chromeos/printing/specifics_translation.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/pref_names.h"
 #include "chromeos/printing/printer_configuration.h"
 #include "chromeos/printing/printer_translator.h"
 #include "components/pref_registry/pref_registry_syncable.h"
-#include "components/prefs/pref_registry_simple.h"
 #include "components/prefs/pref_service.h"
-#include "components/prefs/scoped_user_pref_update.h"
 
 namespace chromeos {
 
 namespace {
 
-const base::ListValue* GetPrinterList(Profile* profile) {
-  return profile->GetPrefs()->GetList(prefs::kPrintingDevices);
-}
-
-// Returns the printer with the matching |id| from the list |values|.  The
-// returned value is mutable and |values| could be modified.
-base::DictionaryValue* FindPrinterPref(const base::ListValue* values,
-                                       const std::string& id) {
-  for (const auto& value : *values) {
-    base::DictionaryValue* printer_dictionary;
-    if (!value->GetAsDictionary(&printer_dictionary))
-      continue;
-
-    std::string printer_id;
-    if (printer_dictionary->GetString(printing::kPrinterId, &printer_id) &&
-        id == printer_id)
-      return printer_dictionary;
+// Adds |printer| with |id| to prefs.  Returns true if the printer is new,
+// false for an update.
+bool UpdatePrinterPref(PrintersSyncBridge* sync_bridge,
+                       const std::string& id,
+                       const Printer& printer) {
+  base::Optional<sync_pb::PrinterSpecifics> specifics =
+      sync_bridge->GetPrinter(id);
+  if (!specifics.has_value()) {
+    sync_bridge->AddPrinter(printing::PrinterToSpecifics(printer));
+    return true;
   }
 
-  return nullptr;
-}
+  // Preserve fields in the proto which we don't understand.
+  std::unique_ptr<sync_pb::PrinterSpecifics> updated_printer =
+      base::MakeUnique<sync_pb::PrinterSpecifics>(*specifics);
+  printing::MergePrinterToSpecifics(printer, updated_printer.get());
+  sync_bridge->AddPrinter(std::move(updated_printer));
 
-void UpdatePrinterPref(
-    Profile* profile,
-    const std::string& id,
-    std::unique_ptr<base::DictionaryValue> printer_dictionary) {
-  ListPrefUpdate update(profile->GetPrefs(), prefs::kPrintingDevices);
-  base::ListValue* printer_list = update.Get();
-  DCHECK(printer_list) << "Register the printer preference";
-  base::DictionaryValue* printer = FindPrinterPref(printer_list, id);
-  if (!printer) {
-    printer_list->Append(std::move(printer_dictionary));
-    return;
-  }
-
-  printer->MergeDictionary(printer_dictionary.get());
+  return false;
 }
 
 }  // anonymous namespace
 
-PrinterPrefManager::PrinterPrefManager(Profile* profile) : profile_(profile) {
+PrinterPrefManager::PrinterPrefManager(
+    Profile* profile,
+    std::unique_ptr<PrintersSyncBridge> sync_bridge)
+    : profile_(profile), sync_bridge_(std::move(sync_bridge)) {
   pref_change_registrar_.Init(profile->GetPrefs());
   pref_change_registrar_.Add(
       prefs::kRecommendedNativePrinters,
@@ -81,27 +68,18 @@ PrinterPrefManager::~PrinterPrefManager() {}
 // static
 void PrinterPrefManager::RegisterProfilePrefs(
     user_prefs::PrefRegistrySyncable* registry) {
-  // TODO(skau): Change to user_prefs::PrefRegistrySyncable::SYNCABLE_PREF) when
-  // sync is implemented.
   registry->RegisterListPref(prefs::kPrintingDevices,
-                             PrefRegistry::NO_REGISTRATION_FLAGS);
+                             user_prefs::PrefRegistrySyncable::SYNCABLE_PREF);
   registry->RegisterListPref(prefs::kRecommendedNativePrinters);
 }
 
 std::vector<std::unique_ptr<Printer>> PrinterPrefManager::GetPrinters() const {
   std::vector<std::unique_ptr<Printer>> printers;
 
-  const base::ListValue* values = GetPrinterList(profile_);
-  for (const auto& value : *values) {
-    const base::DictionaryValue* printer_dictionary = nullptr;
-    value->GetAsDictionary(&printer_dictionary);
-
-    DCHECK(printer_dictionary);
-
-    std::unique_ptr<Printer> printer =
-        printing::PrefToPrinter(*printer_dictionary);
-    if (printer)
-      printers.push_back(std::move(printer));
+  std::vector<sync_pb::PrinterSpecifics> values =
+      sync_bridge_->GetAllPrinters();
+  for (const auto& value : values) {
+    printers.push_back(printing::SpecificsToPrinter(value));
   }
 
   return printers;
@@ -133,30 +111,62 @@ std::unique_ptr<Printer> PrinterPrefManager::GetPrinter(
   if (found != policy_printers.end())
     return printing::RecommendedPrinterToPrinter(*(found->second));
 
-  const base::ListValue* values = GetPrinterList(profile_);
-  const base::DictionaryValue* printer = FindPrinterPref(values, printer_id);
-
-  return printer ? printing::PrefToPrinter(*printer) : nullptr;
+  base::Optional<sync_pb::PrinterSpecifics> printer =
+      sync_bridge_->GetPrinter(printer_id);
+  return printer.has_value() ? printing::SpecificsToPrinter(*printer) : nullptr;
 }
 
 void PrinterPrefManager::RegisterPrinter(std::unique_ptr<Printer> printer) {
-  if (printer->id().empty())
+  if (printer->id().empty()) {
     printer->set_id(base::GenerateGUID());
+  }
 
   DCHECK_EQ(Printer::SRC_USER_PREFS, printer->source());
-  std::unique_ptr<base::DictionaryValue> updated_printer =
-      printing::PrinterToPref(*printer);
-  UpdatePrinterPref(profile_, printer->id(), std::move(updated_printer));
+  bool new_printer =
+      UpdatePrinterPref(sync_bridge_.get(), printer->id(), *printer);
+
+  if (new_printer) {
+    for (Observer& obs : observers_) {
+      obs.OnPrinterAdded(*printer);
+    }
+  } else {
+    for (Observer& obs : observers_) {
+      obs.OnPrinterUpdated(*printer);
+    }
+  }
 }
 
 bool PrinterPrefManager::RemovePrinter(const std::string& printer_id) {
   DCHECK(!printer_id.empty());
-  ListPrefUpdate update(profile_->GetPrefs(), prefs::kPrintingDevices);
-  base::ListValue* printer_list = update.Get();
-  DCHECK(printer_list) << "Printer preference not registered";
-  base::DictionaryValue* printer = FindPrinterPref(printer_list, printer_id);
 
-  return printer && printer_list->Remove(*printer, nullptr);
+  base::Optional<sync_pb::PrinterSpecifics> printer =
+      sync_bridge_->GetPrinter(printer_id);
+  bool success = false;
+  if (printer.has_value()) {
+    std::unique_ptr<Printer> p = printing::SpecificsToPrinter(*printer);
+    success = sync_bridge_->RemovePrinter(p->id());
+    if (success) {
+      for (Observer& obs : observers_) {
+        obs.OnPrinterRemoved(*p);
+      }
+    }
+  } else {
+    LOG(WARNING) << "Could not find printer" << printer_id;
+  }
+
+  return success;
+}
+
+void PrinterPrefManager::AddObserver(Observer* observer) {
+  observers_.AddObserver(observer);
+}
+
+void PrinterPrefManager::RemoveObserver(Observer* observer) {
+  observers_.RemoveObserver(observer);
+}
+
+PrintersSyncBridge* PrinterPrefManager::GetSyncBridge() {
+  return sync_bridge_.get();
 }
 
 // This method is not thread safe and could interact poorly with readers of

diff --git a/chrome/browser/chromeos/printing/printer_pref_manager.h b/chrome/browser/chromeos/printing/printer_pref_manager.h
@@ -10,6 +10,9 @@
 #include <string>
 #include <vector>
 
+#include "base/memory/weak_ptr.h"
+#include "base/observer_list.h"
+#include "chrome/browser/chromeos/printing/printers_sync_bridge.h"
 #include "chromeos/printing/printer_configuration.h"
 #include "chromeos/printing/printer_translator.h"
 #include "components/keyed_service/core/keyed_service.h"
@@ -25,7 +28,15 @@ namespace chromeos {
 
 class PrinterPrefManager : public KeyedService {
  public:
-  explicit PrinterPrefManager(Profile* profile);
+  class Observer {
+   public:
+    virtual void OnPrinterAdded(const Printer& printer) = 0;
+    virtual void OnPrinterUpdated(const Printer& printer) = 0;
+    virtual void OnPrinterRemoved(const Printer& printer) = 0;
+  };
+
+  PrinterPrefManager(Profile* profile,
+                     std::unique_ptr<PrintersSyncBridge> sync_bridge);
   ~PrinterPrefManager() override;
 
   // Register the printing preferences with the |registry|.
@@ -48,18 +59,38 @@ class PrinterPrefManager : public KeyedService {
   // 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(PrinterPrefManager::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(PrinterPrefManager::Observer* observer);
+
+  // Returns a ModelTypeSyncBridge for the sync client.
+  PrintersSyncBridge* GetSyncBridge();
+
  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_;
+
+  base::ObserverList<Observer> observers_;
+
+  DISALLOW_COPY_AND_ASSIGN(PrinterPrefManager);
 };
 
 }  // namespace chromeos

diff --git a/chrome/browser/chromeos/printing/printer_pref_manager_factory.cc b/chrome/browser/chromeos/printing/printer_pref_manager_factory.cc
@@ -4,8 +4,16 @@
 
 #include "chrome/browser/chromeos/printing/printer_pref_manager_factory.h"
 
+#include <memory>
+#include <utility>
+
+#include "base/debug/dump_without_crashing.h"
+#include "base/memory/ptr_util.h"
+#include "chrome/browser/chromeos/printing/printers_sync_bridge.h"
 #include "chrome/browser/profiles/incognito_helpers.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/sync/profile_sync_service_factory.h"
+#include "components/browser_sync/profile_sync_service.h"
 #include "components/keyed_service/content/browser_context_dependency_manager.h"
 #include "content/public/browser/browser_context.h"
 
@@ -45,7 +53,21 @@ PrinterPrefManagerFactory::~PrinterPrefManagerFactory() {}
 PrinterPrefManager* PrinterPrefManagerFactory::BuildServiceInstanceFor(
     content::BrowserContext* browser_context) const {
   Profile* profile = Profile::FromBrowserContext(browser_context);
-  return new PrinterPrefManager(profile);
+
+  browser_sync::ProfileSyncService* sync_service =
+      ProfileSyncServiceFactory::GetForProfile(profile);
+
+  // TODO(skau): --disable-sync and --enable-native-cups are mutually exclusive
+  // until crbug.com/688533 is resolved.
+  DCHECK(sync_service);
+
+  std::unique_ptr<PrintersSyncBridge> sync_bridge =
+      base::MakeUnique<PrintersSyncBridge>(
+          sync_service->GetModelTypeStoreFactory(syncer::PRINTERS),
+          base::BindRepeating(
+              base::IgnoreResult(&base::debug::DumpWithoutCrashing)));
+
+  return new PrinterPrefManager(profile, std::move(sync_bridge));
 }
 
 }  // namespace chromeos