Changed cloud print private API to pass all page settings as single object.

Pass settings from connector setup page using "UserSettings" object that includes "connectNewPrinters" flag and list of "PrinterSettings".

BUG=230187
R=gene

Review URL: https://chromiumcodereview.appspot.com/14215009

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@194754 0039d316-1c4b-4281-b951-d872f2087c98

9 files changed, 69 insertions, 57 deletions

diff --git a/chrome/service/cloud_print/cloud_print_connector.cc b/chrome/service/cloud_print/cloud_print_connector.cc
index 8792017..d374cc2 100644
--- a/chrome/service/cloud_print/cloud_print_connector.cc
+++ b/chrome/service/cloud_print/cloud_print_connector.cc
@@ -90,11 +90,11 @@ void CloudPrintConnector::GetPrinterIds(std::list<std::string>* printer_ids) {
 
 void CloudPrintConnector::RegisterPrinters(
     const printing::PrinterList& printers) {
-  if (!settings_.connect_new_printers() || !IsRunning())
+  if (!IsRunning())
     return;
   printing::PrinterList::const_iterator it;
   for (it = printers.begin(); it != printers.end(); ++it) {
-    if (!settings_.IsPrinterBlacklisted(it->printer_name))
+    if (settings_.ShouldConnect(it->printer_name))
       AddPendingRegisterTask(*it);
   }
 }
@@ -212,7 +212,8 @@ CloudPrintConnector::HandlePrinterListResponse(
         printer_data->GetString(kNameValue, &printer_name);
         std::string printer_id;
         printer_data->GetString(kIdValue, &printer_id);
-        if (settings_.IsPrinterBlacklisted(printer_name)) {
+
+        if (!settings_.ShouldConnect(printer_name)) {
           VLOG(1) << "CP_CONNECTOR: Deleting " << printer_name <<
               " id: " << printer_id << " as blacklisted";
           AddPendingDeleteTask(printer_id);
diff --git a/chrome/service/cloud_print/cloud_print_connector.h b/chrome/service/cloud_print/cloud_print_connector.h
index ee80650..9e6c690 100644
--- a/chrome/service/cloud_print/cloud_print_connector.h
+++ b/chrome/service/cloud_print/cloud_print_connector.h
@@ -68,7 +68,7 @@ class CloudPrintConnector
     PENDING_PRINTER_DELETE
   };
 
-  // TODO(jhawkins): This name conflicts with base::PendingTask.
+  // TODO(vitalybuka): Consider delete pending_tasks_ and just use MessageLoop.
   struct PendingTask {
     PendingTaskType type;
     // Optional members, depending on type.
diff --git a/chrome/service/cloud_print/cloud_print_proxy.cc b/chrome/service/cloud_print/cloud_print_proxy.cc
index 12eee70..c3df3c1 100644
--- a/chrome/service/cloud_print/cloud_print_proxy.cc
+++ b/chrome/service/cloud_print/cloud_print_proxy.cc
@@ -118,8 +118,7 @@ void CloudPrintProxy::EnableForUserWithRobot(
     const std::string& robot_auth_code,
     const std::string& robot_email,
     const std::string& user_email,
-    bool connect_new_printers,
-    const std::vector<std::string>& printer_blacklist) {
+    const base::DictionaryValue& user_settings) {
   DCHECK(CalledOnValidThread());
 
   ShutdownBackend();
@@ -130,14 +129,8 @@ void CloudPrintProxy::EnableForUserWithRobot(
     // Keep only proxy id;
     service_prefs_->SetString(prefs::kCloudPrintProxyId, proxy_id);
   }
-  service_prefs_->SetBoolean(prefs::kCloudPrintConnectNewPrinters,
-                             connect_new_printers);
-  if (!printer_blacklist.empty()) {
-    scoped_ptr<base::ListValue> printers(new base::ListValue());
-    printers->AppendStrings(printer_blacklist);
-    service_prefs_->SetValue(prefs::kCloudPrintPrinterBlacklist,
-                             printers.release());
-  }
+  service_prefs_->SetValue(prefs::kCloudPrintUserSettings,
+                           user_settings.DeepCopy());
   service_prefs_->WritePrefs();
 
   if (!CreateBackend())
diff --git a/chrome/service/cloud_print/cloud_print_proxy.h b/chrome/service/cloud_print/cloud_print_proxy.h
index 8c8fafe..9765add 100644
--- a/chrome/service/cloud_print/cloud_print_proxy.h
+++ b/chrome/service/cloud_print/cloud_print_proxy.h
@@ -46,8 +46,7 @@ class CloudPrintProxy : public CloudPrintProxyFrontend,
       const std::string& robot_auth_code,
       const std::string& robot_email,
       const std::string& user_email,
-      bool connect_new_printers,
-      const std::vector<std::string>& printer_blacklist);
+      const base::DictionaryValue& user_settings);
   void UnregisterPrintersAndDisableForUser();
   void DisableForUser();
   // Returns the proxy info.
diff --git a/chrome/service/cloud_print/connector_settings.cc b/chrome/service/cloud_print/connector_settings.cc
index 1e9b761..ac7588d 100644
--- a/chrome/service/cloud_print/connector_settings.cc
+++ b/chrome/service/cloud_print/connector_settings.cc
@@ -14,6 +14,8 @@ namespace {
 
 const char kDefaultCloudPrintServerUrl[] = "https://www.google.com/cloudprint";
 const char kDeleteOnEnumFail[] = "delete_on_enum_fail";
+const char kName[] = "name";
+const char kConnect[] = "connect";
 
 }  // namespace
 
@@ -68,29 +70,39 @@ void ConnectorSettings::InitFrom(ServiceProcessPrefs* prefs) {
       prefs::kCloudPrintXmppPingTimeout, kDefaultXmppPingTimeoutSecs);
   SetXmppPingTimeoutSec(timeout);
 
-  const base::ListValue* printers = prefs->GetList(
-      prefs::kCloudPrintPrinterBlacklist);
+  const base::ListValue* printers = prefs->GetList(prefs::kCloudPrintPrinters);
   if (printers) {
     for (size_t i = 0; i < printers->GetSize(); ++i) {
-      std::string printer;
-      if (printers->GetString(i, &printer))
-        printer_blacklist_.insert(printer);
+      const base::DictionaryValue* dictionary = NULL;
+      if (printers->GetDictionary(i, &dictionary) && dictionary) {
+        std::string name;
+        dictionary->GetString(kName, &name);
+        if (!name.empty()) {
+          bool connect = connect_new_printers_;
+          dictionary->GetBoolean(kConnect, &connect);
+          if (connect != connect_new_printers_)
+            printers_.insert(name);
+        }
+      }
     }
   }
 }
 
-bool ConnectorSettings::IsPrinterBlacklisted(const std::string& name) const {
-  return printer_blacklist_.find(name) != printer_blacklist_.end();
-};
+bool ConnectorSettings::ShouldConnect(const std::string& printer_name) const {
+  Printers::const_iterator printer = printers_.find(printer_name);
+  if (printer != printers_.end())
+    return !connect_new_printers_;
+  return connect_new_printers_;
+}
 
 void ConnectorSettings::CopyFrom(const ConnectorSettings& source) {
   server_url_ = source.server_url();
   proxy_id_ = source.proxy_id();
   delete_on_enum_fail_ = source.delete_on_enum_fail();
-  connect_new_printers_ = source.connect_new_printers();
+  connect_new_printers_ = source.connect_new_printers_;
   xmpp_ping_enabled_ = source.xmpp_ping_enabled();
   xmpp_ping_timeout_sec_ = source.xmpp_ping_timeout_sec();
-  printer_blacklist_ = source.printer_blacklist_;
+  printers_ = source.printers_;
   if (source.print_system_settings())
     print_system_settings_.reset(source.print_system_settings()->DeepCopy());
 }
diff --git a/chrome/service/cloud_print/connector_settings.h b/chrome/service/cloud_print/connector_settings.h
index 94aa841..758e101 100644
--- a/chrome/service/cloud_print/connector_settings.h
+++ b/chrome/service/cloud_print/connector_settings.h
@@ -8,6 +8,7 @@
 #include <set>
 #include <string>
 
+#include "base/gtest_prod_util.h"
 #include "base/memory/scoped_ptr.h"
 #include "googleurl/src/gurl.h"
 
@@ -40,18 +41,10 @@ class ConnectorSettings {
     return delete_on_enum_fail_;
   }
 
-  bool connect_new_printers() const {
-    return connect_new_printers_;
-  };
-
   bool xmpp_ping_enabled() const {
     return xmpp_ping_enabled_;
   }
 
-  void set_xmpp_ping_enabled(bool enabled) {
-    xmpp_ping_enabled_ = enabled;
-  }
-
   int xmpp_ping_timeout_sec() const {
     return xmpp_ping_timeout_sec_;
   }
@@ -60,11 +53,18 @@ class ConnectorSettings {
     return print_system_settings_.get();
   };
 
-  bool IsPrinterBlacklisted(const std::string& name) const;
+  bool ShouldConnect(const std::string& printer_name) const;
+
+ private:
+  friend class ConnectorSettingsTest;
+  FRIEND_TEST_ALL_PREFIXES(ConnectorSettingsTest, SettersTest);
+
+  void set_xmpp_ping_enabled(bool enabled) {
+    xmpp_ping_enabled_ = enabled;
+  }
 
   void SetXmppPingTimeoutSec(int timeout);
 
- private:
   // Cloud Print server url.
   GURL server_url_;
 
@@ -85,8 +85,9 @@ class ConnectorSettings {
   // Indicate timeout between XMPP pings.
   int xmpp_ping_timeout_sec_;
 
-  // List of printers which should not be connected.
-  std::set<std::string> printer_blacklist_;
+  // Black list if connect_new_printers_ is true, or whitelist if false.
+  typedef std::set<std::string> Printers;
+  Printers printers_;
 
   // Print system settings.
   scoped_ptr<base::DictionaryValue> print_system_settings_;
diff --git a/chrome/service/cloud_print/connector_settings_unittest.cc b/chrome/service/cloud_print/connector_settings_unittest.cc
index 9f812ea..f1126e1 100644
--- a/chrome/service/cloud_print/connector_settings_unittest.cc
+++ b/chrome/service/cloud_print/connector_settings_unittest.cc
@@ -30,16 +30,19 @@ const char kServiceStateContent[] =
     "      'robot_refresh_token': '123',"
     "      'service_url': 'http://cp.google.com',"
     "      'xmpp_auth_token': 'xmp token',"
-    "      'connect_new_printers': false,"
     "      'xmpp_ping_enabled': true,"
     "      'xmpp_ping_timeout_sec': 256,"
-    "      'printer_blacklist': ["
-    "         'prn1',"
-    "         'prn2'"
-    "       ],"
+    "      'user_settings': {"
+    "        'printers': ["
+    "          { 'name': 'prn1', 'connect': false },"
+    "          { 'name': 'prn2', 'connect': false },"
+    "          { 'name': 'prn3', 'connect': true }"
+    "        ],"
+    "        'connect_new_printers': false"
+    "      },"
     "      'print_system_settings': {"
     "         'delete_on_enum_fail' : true"
-    "       }"
+    "      }"
     "   }"
     "}";
 
@@ -87,9 +90,8 @@ TEST_F(ConnectorSettingsTest, InitFromEmpty) {
     EXPECT_FALSE(settings.proxy_id().empty());
     EXPECT_FALSE(settings.delete_on_enum_fail());
     EXPECT_EQ(NULL, settings.print_system_settings());
-    EXPECT_TRUE(settings.connect_new_printers());
+    EXPECT_TRUE(settings.ShouldConnect("prn1"));
     EXPECT_FALSE(settings.xmpp_ping_enabled());
-    EXPECT_FALSE(settings.IsPrinterBlacklisted("prn1"));
   }
 }
 
@@ -102,11 +104,11 @@ TEST_F(ConnectorSettingsTest, InitFromFile) {
   EXPECT_FALSE(settings.proxy_id().empty());
   EXPECT_TRUE(settings.delete_on_enum_fail());
   EXPECT_TRUE(settings.print_system_settings());
-  EXPECT_FALSE(settings.connect_new_printers());
   EXPECT_TRUE(settings.xmpp_ping_enabled());
   EXPECT_EQ(settings.xmpp_ping_timeout_sec(), 256);
-  EXPECT_FALSE(settings.IsPrinterBlacklisted("prn0"));
-  EXPECT_TRUE(settings.IsPrinterBlacklisted("prn1"));
+  EXPECT_FALSE(settings.ShouldConnect("prn0"));
+  EXPECT_FALSE(settings.ShouldConnect("prn1"));
+  EXPECT_TRUE(settings.ShouldConnect("prn3"));
 }
 
 TEST_F(ConnectorSettingsTest, CopyFrom) {
@@ -122,11 +124,12 @@ TEST_F(ConnectorSettingsTest, CopyFrom) {
   EXPECT_EQ(settings1.delete_on_enum_fail(), settings2.delete_on_enum_fail());
   EXPECT_EQ(settings1.print_system_settings()->size(),
             settings2.print_system_settings()->size());
-  EXPECT_EQ(settings1.connect_new_printers(), settings2.connect_new_printers());
   EXPECT_EQ(settings1.xmpp_ping_enabled(), settings2.xmpp_ping_enabled());
   EXPECT_EQ(settings1.xmpp_ping_timeout_sec(),
             settings2.xmpp_ping_timeout_sec());
-  EXPECT_TRUE(settings2.IsPrinterBlacklisted("prn1"));
+  EXPECT_FALSE(settings2.ShouldConnect("prn0"));
+  EXPECT_FALSE(settings2.ShouldConnect("prn1"));
+  EXPECT_TRUE(settings2.ShouldConnect("prn3"));
 }
 
 TEST_F(ConnectorSettingsTest, SettersTest) {
diff --git a/chrome/service/service_ipc_server.cc b/chrome/service/service_ipc_server.cc
index 4cca7ec..9935f34 100644
--- a/chrome/service/service_ipc_server.cc
+++ b/chrome/service/service_ipc_server.cc
@@ -112,11 +112,9 @@ void ServiceIPCServer::OnEnableCloudPrintProxyWithRobot(
     const std::string& robot_auth_code,
     const std::string& robot_email,
     const std::string& user_email,
-    bool connect_new_printers,
-    const std::vector<std::string>& printer_blacklist) {
+    const base::DictionaryValue& user_settings) {
   g_service_process->GetCloudPrintProxy()->EnableForUserWithRobot(
-      robot_auth_code, robot_email, user_email, connect_new_printers,
-      printer_blacklist);
+      robot_auth_code, robot_email, user_email, user_settings);
 }
 
 void ServiceIPCServer::OnGetCloudPrintProxyInfo() {
diff --git a/chrome/service/service_ipc_server.h b/chrome/service/service_ipc_server.h
index 4f7f5f7..7fd5d61 100644
--- a/chrome/service/service_ipc_server.h
+++ b/chrome/service/service_ipc_server.h
@@ -15,6 +15,12 @@
 #include "ipc/ipc_sync_message_filter.h"
 #include "ipc/ipc_sender.h"
 
+namespace base {
+
+class DictionaryValue;
+
+}  // namespace base
+
 // This class handles IPC commands for the service process.
 class ServiceIPCServer : public IPC::Listener, public IPC::Sender {
  public:
@@ -49,8 +55,7 @@ class ServiceIPCServer : public IPC::Listener, public IPC::Sender {
       const std::string& robot_auth_code,
       const std::string& robot_email,
       const std::string& user_email,
-      bool connect_new_printers,
-      const std::vector<std::string>& printer_blacklist);
+      const base::DictionaryValue& user_settings);
   void OnGetCloudPrintProxyInfo();
   void OnDisableCloudPrintProxy();