Define printer configuration objects for Chrome OS.

chrome/browser/chromeos/printing/printer_pref_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/printer_pref_manager.h"
+
+#include <memory>
+#include <string>
+#include <utility>
+#include <vector>
+
+#include "base/guid.h"
+#include "base/memory/ptr_util.h"
+#include "base/values.h"
+#include "chrome/browser/chromeos/printing/printer_configuration.h"
+#include "chrome/browser/chromeos/printing/printer_translator.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/common/pref_names.h"

[stevenjb, 2016/07/29 16:59:26]

Note: This file will need to remain in chrome/browser/chromeos/printing/ since
it depends on Profile.

[skau, 2016/08/01 19:37:20]

Acknowledged.

+#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 {
+
+using base::DictionaryValue;

[stevenjb, 2016/07/29 16:56:50]

nit: This is fine, but generally not necessary. base::ListValue is used below
without a typedef, so we should be consistent one way or the other.

[skau, 2016/08/01 19:37:19]

Done.

+
+namespace {
+
+const base::ListValue* GetPrinterList(Profile* profile) {
+  PrefService* user_state = profile->GetPrefs();

[stevenjb, 2016/07/29 16:56:51]

nit: local not necessary: 'return 
profile->GetPrefs()>GetList(prefs::kPrintingDevices)'.
clang-format makes formatting long lines easy, so even if that were to cause
wrapping, we generally avoid declaring locals that are used only once unless it
clarifies the code. Here I would suggest that 'user_state' is less clear than
'profile->GetPrefs()'.

[skau, 2016/08/01 19:37:20]

Done.

+  return user_state->GetList(prefs::kPrintingDevices);
+}
+
+DictionaryValue* FindMatchingPrinter(base::ListValue* values,

[stevenjb, 2016/07/29 16:56:51]

const base::ListValue&

[skau, 2016/08/01 19:37:19]

Thanks...  Why can I get a mutable DictionaryValue from a const ListValue?  The
const version of GetAsDictionary is supposed to point to a const Dictionary
value.

[stevenjb, 2016/08/01 20:48:16]

Good question, I don't know. Anyway, my apologies, I didn't notice this was
returning (and modifying) a non-const *. The way you had this before was
correct. Maybe add a comment, or rename the method (e.g. FindPrinterPref) to
make that more clear.

[skau, 2016/08/01 21:10:10]

It looks like values behaves like a const collection where I can retrieve
references to elements and modify them.  I can remove the const if it's
confusing.  I've renamed the function and added a comment, I hope that's
clearer.

+                                     const std::string& id) {
+  DictionaryValue* match = nullptr;

[stevenjb, 2016/07/29 16:56:51]

Not needed, see below

[skau, 2016/08/01 19:37:20]

Done.

+  size_t size = values->GetSize();
+  for (size_t i = 0; i < size; ++i) {

[stevenjb, 2016/07/29 16:56:50]

for (const auto& value : values) {
  DictionaryValue* printer_dictionary;
  if (!value->GetAsDictionary(&printer_dictionary))
    continue;
  ...

[skau, 2016/08/01 19:37:20]

Thanks.  I didn't notice that ListValue implemented begin and end.

+    DictionaryValue* printer_dictionary;
+    if (!values->GetDictionary(i, &printer_dictionary)) {
+      continue;
+    }
+
+    std::string printer_id;
+    if (printer_dictionary->GetString(kPrinterId, &printer_id) &&
+        id == printer_id) {
+      match = printer_dictionary;

[stevenjb, 2016/07/29 16:56:51]

return printer_dictionary;

[skau, 2016/08/01 19:37:20]

Done.

+      break;
+    }
+  }
+
+  return match;

[stevenjb, 2016/07/29 16:56:50]

return nullptr;

[skau, 2016/08/01 19:37:19]

Done.

+}
+
+void UpdatePrinterPref(Profile* profile,
+                       const std::string& id,
+                       std::unique_ptr<DictionaryValue> printer_dictionary) {
+  ListPrefUpdate update(profile->GetPrefs(), prefs::kPrintingDevices);
+  base::ListValue* printer_list = update.Get();

[stevenjb, 2016/07/29 16:56:50]

DCHECK(printer_list)

This documents that update.Get() should logically never return nullptr. (If that
is not the case we need to handle a null result).

[skau, 2016/08/01 19:37:20]

Done.

+  DictionaryValue* printer = FindMatchingPrinter(printer_list, id);
+  if (!printer) {
+    printer_list->Append(
+        std::unique_ptr<base::Value>(std::move(printer_dictionary)));

[stevenjb, 2016/07/29 16:56:51]

return;
no else

[skau, 2016/08/01 19:37:20]

Done.

+  } else {
+    printer->MergeDictionary(printer_dictionary.get());
+  }
+}
+
+}  // anonymous namespace
+
+// 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);
+}
+
+PrinterPrefManager::PrinterPrefManager(Profile* profile) : profile_(profile) {}
+
+std::vector<std::unique_ptr<Printer>> PrinterPrefManager::GetPrinters() const {
+  std::vector<std::unique_ptr<Printer>> printers;
+
+  const base::ListValue* values = GetPrinterList(profile_);
+  size_t size = values->GetSize();
+  for (size_t i = 0; i < size; ++i) {

[stevenjb, 2016/07/29 16:56:51]

for (const auto& value : *values), etc.

[skau, 2016/08/01 19:37:20]

Done.

+    const DictionaryValue* printer_dictionary;

[stevenjb, 2016/07/29 16:56:50]

= nullptr;

[skau, 2016/08/01 19:37:20]

Done.

+    if (values->GetDictionary(i, &printer_dictionary)) {

[stevenjb, 2016/07/29 16:56:51]

value->GetAsDictionary(&printer_dictionary);
DCHECK(printer_dictionary);

[skau, 2016/08/01 19:37:20]

Done.

+      std::unique_ptr<Printer> printer = base::MakeUnique<Printer>();
+      if (MergePrinterPreference(*printer_dictionary, printer.get()))
+        printers.push_back(std::move(printer));
+    } else {
+      NOTREACHED();

[stevenjb, 2016/07/29 16:56:51]

elim (use DCHECK mentioned above instead)

[skau, 2016/08/01 19:37:20]

Done.

+    }
+  }
+
+  return printers;
+}
+
+void PrinterPrefManager::RegisterPrinter(std::unique_ptr<Printer> printer) {
+  if (printer->id().empty()) {
+    printer->set_id(base::GenerateGUID());
+  }

[stevenjb, 2016/07/29 16:56:50]

nit: no {} around single line if statement (optional, but more common pattern in
chrome)

[skau, 2016/08/01 19:37:20]

Righto. I've been writing too much Java.

+
+  std::unique_ptr<base::DictionaryValue> updated_printer =
+      base::MakeUnique<DictionaryValue>();
+  PrinterToPref(*(printer.get()), updated_printer.get());
+  UpdatePrinterPref(profile_, printer->id(), std::move(updated_printer));
+}
+
+}  // namespace chromeos