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
parent 36ab2175
......@@ -46,17 +46,17 @@ bool CloudPrintPrivateSetupConnectorFunction::RunImpl() {
params->user_email,
params->robot_email,
params->credentials,
params->connect_new_printers,
params->printer_blacklist);
params->user_settings);
} else {
if (!CloudPrintProxyServiceFactory::GetForProfile(profile_))
return false;
scoped_ptr<base::DictionaryValue> user_setings(
params->user_settings.ToValue());
CloudPrintProxyServiceFactory::GetForProfile(profile_)->
EnableForUserWithRobot(params->credentials,
params->robot_email,
params->user_email,
params->connect_new_printers,
params->printer_blacklist);
*user_setings);
}
SendResponse(true);
return true;
......
......@@ -12,6 +12,15 @@
namespace extensions {
namespace api {
namespace cloud_print_private {
struct UserSettings;
} // namespace cloud_print_private
} // namespace api
// For use only in tests.
class CloudPrintTestsDelegate {
public:
......@@ -22,8 +31,7 @@ class CloudPrintTestsDelegate {
const std::string& user_email,
const std::string& robot_email,
const std::string& credentials,
bool connect_new_printers,
const std::vector<std::string>& printer_blacklist) = 0;
const api::cloud_print_private::UserSettings& user_settings) = 0;
virtual std::string GetHostName() = 0;
......
......@@ -9,6 +9,7 @@
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/common/chrome_switches.h"
#include "chrome/common/extensions/api/cloud_print_private.h"
#include "chrome/test/base/ui_test_utils.h"
#include "net/dns/mock_host_resolver.h"
#include "testing/gmock/include/gmock/gmock.h"
......@@ -55,37 +56,49 @@ class ExtensionCloudPrintPrivateApiTest : public ExtensionApiTest {
#if !defined(OS_CHROMEOS)
using extensions::api::cloud_print_private::UserSettings;
class CloudPrintTestsDelegateMock : public extensions::CloudPrintTestsDelegate {
public:
CloudPrintTestsDelegateMock() {}
MOCK_METHOD5(SetupConnector,
MOCK_METHOD4(SetupConnector,
void(const std::string& user_email,
const std::string& robot_email,
const std::string& credentials,
bool connect_new_printers,
const std::vector<std::string>& printer_blacklist));
const UserSettings& user_settings));
MOCK_METHOD0(GetHostName, std::string());
MOCK_METHOD0(GetPrinters, std::vector<std::string>());
MOCK_METHOD0(GetClientId, std::string());
private:
DISALLOW_COPY_AND_ASSIGN(CloudPrintTestsDelegateMock);
};
MATCHER(IsExpectedUserSettings, "") {
const UserSettings& settings = arg;
return settings.connect_new_printers &&
settings.printers.size() == 2 &&
settings.printers[0]->name == "printer1" &&
!settings.printers[0]->connect &&
settings.printers[1]->name == "printer2" &&
settings.printers[1]->connect;
}
IN_PROC_BROWSER_TEST_F(ExtensionCloudPrintPrivateApiTest, CloudPrintHosted) {
CloudPrintTestsDelegateMock cloud_print_mock;
std::vector<std::string> printers;
printers.push_back("printer1");
printers.push_back("printer2");
EXPECT_CALL(cloud_print_mock,
SetupConnector("foo@gmail.com",
"foorobot@googleusercontent.com",
"1/23546efa54",
true,
printers));
IsExpectedUserSettings()));
EXPECT_CALL(cloud_print_mock, GetHostName())
.WillRepeatedly(Return("TestHostName"));
std::vector<std::string> printers;
printers.push_back("printer1");
printers.push_back("printer2");
EXPECT_CALL(cloud_print_mock, GetPrinters())
.WillRepeatedly(Return(printers));
......
......@@ -91,13 +91,12 @@ void CloudPrintProxyService::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_preferences) {
if (profile_->GetPrefs()->GetBoolean(prefs::kCloudPrintProxyEnabled)) {
InvokeServiceTask(
base::Bind(&CloudPrintProxyService::EnableCloudPrintProxyWithRobot,
weak_factory_.GetWeakPtr(), robot_auth_code, robot_email,
user_email, connect_new_printers, printer_blacklist));
user_email, base::Owned(user_preferences.DeepCopy())));
}
}
......@@ -185,14 +184,12 @@ void CloudPrintProxyService::EnableCloudPrintProxyWithRobot(
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_preferences) {
ServiceProcessControl* process_control = GetServiceProcessControl();
DCHECK(process_control->IsConnected());
process_control->Send(
new ServiceMsg_EnableCloudPrintProxyWithRobot(
robot_auth_code, robot_email, user_email, connect_new_printers,
printer_blacklist));
robot_auth_code, robot_email, user_email, *user_preferences));
// Assume the IPC worked.
profile_->GetPrefs()->SetString(prefs::kCloudPrintEmail, user_email);
}
......
......@@ -42,8 +42,7 @@ class CloudPrintProxyService
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);
virtual void DisableForUser();
// Query the service process for the status of the cloud print proxy and
......@@ -69,9 +68,6 @@ class CloudPrintProxyService
class TokenExpiredNotificationDelegate;
friend class TokenExpiredNotificationDelegate;
Profile* profile_;
std::string proxy_id_;
// Methods that send an IPC to the service.
void RefreshCloudPrintProxyStatus();
void EnableCloudPrintProxy(const std::string& lsid, const std::string& email);
......@@ -79,8 +75,7 @@ class CloudPrintProxyService
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_preferences);
void DisableCloudPrintProxy();
// Callback that gets the cloud print proxy info.
......@@ -96,12 +91,15 @@ class CloudPrintProxyService
// not set or the connector is not enabled).
bool ApplyCloudPrintConnectorPolicy();
Profile* profile_;
std::string proxy_id_;
// Virtual for testing.
virtual ServiceProcessControl* GetServiceProcessControl();
base::WeakPtrFactory<CloudPrintProxyService> weak_factory_;
// For watching for connector enablement policy changes.
// For watching for connector policy changes.
PrefChangeRegistrar pref_change_registrar_;
// If set, continue trying to disable the connector, and quit the process
......
......@@ -16,6 +16,7 @@ const char kProxyIdValue[] = "proxy";
const char kPrinterNameValue[] = "printer";
const char kPrinterDescValue[] = "description";
const char kPrinterCapsValue[] = "capabilities";
const char kPrinterDisplayNameValue[] = "default_display_name";
const char kPrinterDefaultsValue[] = "defaults";
const char kPrinterStatusValue[] = "status";
const char kPrinterTagValue[] = "tag";
......
......@@ -25,6 +25,7 @@ extern const char kProxyIdValue[];
extern const char kPrinterNameValue[];
extern const char kPrinterDescValue[];
extern const char kPrinterCapsValue[];
extern const char kPrinterDisplayNameValue[];
extern const char kPrinterDefaultsValue[];
extern const char kPrinterStatusValue[];
extern const char kPrinterTagValue[];
......
......@@ -6,6 +6,41 @@
{
"namespace": "cloudPrintPrivate",
"nodoc": "true",
"types": [
{
"id": "PrinterSettings",
"type": "object",
"description": "Settings per printer.",
"properties": {
"name": {
"type": "string",
"description": "Unique printer id."
},
"connect": {
"type": "boolean",
"description": "Whether printer is selected."
}
}
},
{
"id": "UserSettings",
"type": "object",
"description": "Settings set by user.",
"properties": {
"printers": {
"description": "Printer settings.",
"type": "array",
"items": {
"$ref": "PrinterSettings"
}
},
"connectNewPrinters": {
"type": "boolean",
"description": "Whether should printer be connected."
}
}
}
],
"functions": [
{
"name": "setupConnector",
......@@ -28,15 +63,9 @@
"description": "The login credentials(OAuth2 Auth code)."
},
{
"name": "connectNewPrinters",
"type": "boolean",
"description": "True if new printers should be connected."
},
{
"name": "printerBlacklist",
"description": "Printers that should not be connected.",
"type": "array",
"items": {"type": "string"}
"name": "userSettings",
"$ref": "UserSettings",
"description": "Options configured by user."
}
]
},
......
......@@ -2104,13 +2104,16 @@ const char kCloudPrintEnableJobPoll[] = "cloud_print.enable_job_poll";
const char kCloudPrintRobotRefreshToken[] = "cloud_print.robot_refresh_token";
const char kCloudPrintRobotEmail[] = "cloud_print.robot_email";
// A boolean indicating whether we should connect to cloud print new printers.
const char kCloudPrintConnectNewPrinters[] = "cloud_print.connect_new_printers";
const char kCloudPrintConnectNewPrinters[] =
"cloud_print.user_settings.connect_new_printers";
// A boolean indicating whether we should ping XMPP connection.
const char kCloudPrintXmppPingEnabled[] = "cloud_print.xmpp_ping_enabled";
// An int value indicating the average timeout between xmpp pings.
const char kCloudPrintXmppPingTimeout[] = "cloud_print.xmpp_ping_timeout_sec";
// List of printers which should not be connected.
const char kCloudPrintPrinterBlacklist[] = "cloud_print.printer_blacklist";
// Dictionary with settings stored by connector setup page.
const char kCloudPrintUserSettings[] = "cloud_print.user_settings";
// List of printers settings.
extern const char kCloudPrintPrinters[] = "cloud_print.user_settings.printers";
// A boolean indicating whether submitting jobs to Google Cloud Print is
// blocked by policy.
const char kCloudPrintSubmitEnabled[] = "cloud_print.submit_enabled";
......
......@@ -696,8 +696,9 @@ extern const char kCloudPrintRobotEmail[];
extern const char kCloudPrintConnectNewPrinters[];
extern const char kCloudPrintXmppPingEnabled[];
extern const char kCloudPrintXmppPingTimeout[];
extern const char kCloudPrintPrinterBlacklist[];
extern const char kCloudPrintPrinters[];
extern const char kCloudPrintSubmitEnabled[];
extern const char kCloudPrintUserSettings[];
#if !defined(OS_ANDROID)
extern const char kChromeToMobileDeviceList[];
......
......@@ -6,6 +6,7 @@
#include <string>
#include <vector>
#include "base/values.h"
#include "chrome/common/cloud_print/cloud_print_proxy_info.h"
#include "ipc/ipc_channel_handle.h"
#include "ipc/ipc_message_macros.h"
......@@ -28,12 +29,11 @@ IPC_MESSAGE_CONTROL1(ServiceMsg_EnableCloudPrintProxy,
// Tell the service process to enable the cloud proxy passing in the OAuth2
// auth code of a robot account.
IPC_MESSAGE_CONTROL5(ServiceMsg_EnableCloudPrintProxyWithRobot,
IPC_MESSAGE_CONTROL4(ServiceMsg_EnableCloudPrintProxyWithRobot,
std::string /* robot_auth_code */,
std::string /* robot_email */,
std::string /* user_email */,
bool /* connect_new_printers */,
std::vector<std::string> /* printer_blacklist */)
DictionaryValue /* user_settings */)
// Tell the service process to disable the cloud proxy.
IPC_MESSAGE_CONTROL0(ServiceMsg_DisableCloudPrintProxy)
......
......@@ -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);
......
......@@ -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.
......
......@@ -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())
......
......@@ -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.
......
......@@ -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());
}
......
......@@ -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_;
......
......@@ -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) {
......
......@@ -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() {
......
......@@ -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();
......
......@@ -8,7 +8,18 @@ var tests = [
var robotEmail = 'foorobot@googleusercontent.com';
var credentials = '1/23546efa54';
chrome.cloudPrintPrivate.setupConnector(
userEmail, robotEmail, credentials, true, ['printer1', 'printer2']);
userEmail, robotEmail, credentials, {
"connectNewPrinters": true,
"printers": [
{
"name" : "printer1",
"connect" : false
}, {
"name" : "printer2",
"connect" : true
}
]
});
chrome.test.succeed();
},
function getHostName() {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment