Adding XMPP ping functionality to CloudPrint. XMPP ping and timeout i…

…s controlled thorugh Service State file.

BUG=157735

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

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

chrome/common/pref_names.cc

@@ -1934,6 +1934,10 @@ 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";
+// 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";
 // A boolean indicating whether submitting jobs to Google Cloud Print is

chrome/common/pref_names.h

@@ -640,6 +640,8 @@ extern const char kCloudPrintEnableJobPoll[];
 extern const char kCloudPrintRobotRefreshToken[];
 extern const char kCloudPrintRobotEmail[];
 extern const char kCloudPrintConnectNewPrinters[];
+extern const char kCloudPrintXmppPingEnabled[];
+extern const char kCloudPrintXmppPingTimeout[];
 extern const char kCloudPrintPrinterBlacklist[];
 extern const char kCloudPrintSubmitEnabled[];
 

chrome/service/cloud_print/cloud_print_consts.h

@@ -64,6 +64,15 @@ const int kCloudPrintAuthMaxRetryCount = -1;
 const int kMinJobPollIntervalSecs = 5*60;  // 5 minutes in seconds
 const int kMaxJobPollIntervalSecs = 8*60;  // 8 minutes in seconds
 
+// When we have XMPP notifications available, we ping server to keep connection
+// alive or check connection status.
+const int kDefaultXmppPingTimeoutSecs = 5*60;  // 5 minutes in seconds
+const int kMinimumXmppPingTimeoutSecs = 2*60;  // 2 minutes in seconds
+const int kXmppPingCheckIntervalSecs = 60;
+
+// Number of failed pings before we try to reinstablish XMPP connection.
+const int kMaxFailedXmppPings = 2;
+
 // The number of seconds before the OAuth2 access token is due to expire that
 // we try and refresh it.
 const int kTokenRefreshGracePeriodSecs = 5*60;  // 5 minutes in seconds

chrome/service/cloud_print/cloud_print_proxy_backend.cc

@@ -8,10 +8,12 @@
 #include <vector>
 
 #include "base/bind.h"
+#include "base/command_line.h"
 #include "base/compiler_specific.h"
 #include "base/file_util.h"
 #include "base/rand_util.h"
 #include "base/values.h"
+#include "chrome/common/chrome_switches.h"
 #include "chrome/service/cloud_print/cloud_print_auth.h"
 #include "chrome/service/cloud_print/cloud_print_connector.h"
 #include "chrome/service/cloud_print/cloud_print_consts.h"
@@ -88,6 +90,7 @@ class CloudPrintProxyBackend::Core
       notifier::NotificationsDisabledReason reason) OVERRIDE;
   virtual void OnIncomingNotification(
       const notifier::Notification& notification) OVERRIDE;
+  virtual void OnPingResponse() OVERRIDE;
 
  private:
   friend class base::RefCountedThreadSafe<Core>;
@@ -119,6 +122,10 @@ class CloudPrintProxyBackend::Core
   // Schedules a task to poll for jobs. Does nothing if a task is already
   // scheduled.
   void ScheduleJobPoll();
+  void PingXmppServer();
+  void ScheduleXmppPing();
+  void CheckXmppPingStatus();
+
   CloudPrintTokenStore* GetTokenStore();
 
   // Our parent CloudPrintProxyBackend
@@ -143,8 +150,13 @@ class CloudPrintProxyBackend::Core
   bool job_poll_scheduled_;
   // Indicates whether we should poll for jobs when we lose XMPP connection.
   bool enable_job_poll_;
+  // Indicates whether a task to ping xmpp server has been scheduled.
+  bool xmpp_ping_scheduled_;
+  // Number of XMPP pings pending reply from the server.
+  int pending_xmpp_pings_;
   // Connector settings.
   ConnectorSettings settings_;
+  std::string robot_email_;
   scoped_ptr<CloudPrintTokenStore> token_store_;
 
   DISALLOW_COPY_AND_ASSIGN(Core);
@@ -240,7 +252,9 @@ CloudPrintProxyBackend::Core::Core(
         oauth_client_info_(oauth_client_info),
         notifications_enabled_(false),
         job_poll_scheduled_(false),
-        enable_job_poll_(enable_job_poll) {
+        enable_job_poll_(enable_job_poll),
+        xmpp_ping_scheduled_(false),
+        pending_xmpp_pings_(0) {
   settings_.CopyFrom(settings);
 }
 
@@ -305,6 +319,7 @@ void CloudPrintProxyBackend::Core::OnAuthenticationComplete(
   CloudPrintTokenStore* token_store  = GetTokenStore();
   bool first_time = token_store->token().empty();
   token_store->SetToken(access_token);
+  robot_email_ = robot_email;
   // Let the frontend know that we have authenticated.
   backend_->frontend_loop_->PostTask(
       FROM_HERE,
@@ -350,6 +365,7 @@ void CloudPrintProxyBackend::Core::InitNotifications(
     const std::string& access_token) {
   DCHECK(MessageLoop::current() == backend_->core_thread_.message_loop());
 
+  pending_xmpp_pings_ = 0;
   notifier::NotifierOptions notifier_options;
   notifier_options.request_context_getter =
       g_service_process->GetServiceURLRequestContextGetter();
@@ -430,6 +446,53 @@ void CloudPrintProxyBackend::Core::ScheduleJobPoll() {
   }
 }
 
+void CloudPrintProxyBackend::Core::PingXmppServer() {
+  xmpp_ping_scheduled_ = false;
+
+  if (!push_client_.get())
+    return;
+
+  push_client_->SendPing();
+
+  pending_xmpp_pings_++;
+  if (pending_xmpp_pings_ >= kMaxFailedXmppPings) {
+    // Check ping status when we close to the limit.
+    MessageLoop::current()->PostDelayedTask(
+        FROM_HERE,
+        base::Bind(&CloudPrintProxyBackend::Core::CheckXmppPingStatus, this),
+        base::TimeDelta::FromSeconds(kXmppPingCheckIntervalSecs));
+  }
+
+  // Schedule next ping if needed.
+  if (notifications_enabled_)
+    ScheduleXmppPing();
+}
+
+void CloudPrintProxyBackend::Core::ScheduleXmppPing() {
+  if (!settings_.xmpp_ping_enabled())
+    return;
+
+  if (!xmpp_ping_scheduled_) {
+    base::TimeDelta interval = base::TimeDelta::FromSeconds(
+      base::RandInt(settings_.xmpp_ping_timeout_sec() * 0.9,
+                    settings_.xmpp_ping_timeout_sec() * 1.1));
+    MessageLoop::current()->PostDelayedTask(
+        FROM_HERE,
+        base::Bind(&CloudPrintProxyBackend::Core::PingXmppServer, this),
+        interval);
+    xmpp_ping_scheduled_ = true;
+  }
+}
+
+void CloudPrintProxyBackend::Core::CheckXmppPingStatus() {
+  if (pending_xmpp_pings_ >= kMaxFailedXmppPings) {
+    // Reconnect to XMPP.
+    pending_xmpp_pings_ = 0;
+    push_client_.reset();
+    InitNotifications(robot_email_, GetTokenStore()->token());
+  }
+}
+
 CloudPrintTokenStore* CloudPrintProxyBackend::Core::GetTokenStore() {
   DCHECK(MessageLoop::current() == backend_->core_thread_.message_loop());
   if (!token_store_.get())
@@ -476,6 +539,9 @@ void CloudPrintProxyBackend::Core::OnNotificationsEnabled() {
   // Note that ScheduleJobPoll will not schedule again if a job poll task is
   // already scheduled.
   ScheduleJobPoll();
+
+  // Schedule periodic ping for XMPP notification channel.
+  ScheduleXmppPing();
 }
 
 void CloudPrintProxyBackend::Core::OnNotificationsDisabled(
@@ -494,10 +560,19 @@ void CloudPrintProxyBackend::Core::OnNotificationsDisabled(
 
 void CloudPrintProxyBackend::Core::OnIncomingNotification(
     const notifier::Notification& notification) {
+  // Since we got some notification from the server,
+  // reset pending ping counter to 0.
+  pending_xmpp_pings_ = 0;
+
   DCHECK(MessageLoop::current() == backend_->core_thread_.message_loop());
   VLOG(1) << "CP_CONNECTOR: Incoming notification.";
   if (0 == base::strcasecmp(kCloudPrintPushNotificationsSource,
                             notification.channel.c_str()))
     HandlePrinterNotification(notification.data);
 }
 
+void CloudPrintProxyBackend::Core::OnPingResponse() {
+  pending_xmpp_pings_ = 0;
+  VLOG(1) << "CP_CONNECTOR: Ping response received.";
+}
+

chrome/service/cloud_print/connector_settings.cc

@@ -19,7 +19,9 @@ const char kDeleteOnEnumFail[] = "delete_on_enum_fail";
 
 ConnectorSettings::ConnectorSettings()
     : delete_on_enum_fail_(false),
-      connect_new_printers_(true) {
+      connect_new_printers_(true),
+      xmpp_ping_enabled_(false),
+      xmpp_ping_timeout_sec_(kDefaultXmppPingTimeoutSecs) {
 }
 
 ConnectorSettings::~ConnectorSettings() {
@@ -56,6 +58,13 @@ void ConnectorSettings::InitFrom(ServiceProcessPrefs* prefs) {
 
   connect_new_printers_ = prefs->GetBoolean(
       prefs::kCloudPrintConnectNewPrinters, true);
+
+  xmpp_ping_enabled_ = prefs->GetBoolean(
+      prefs::kCloudPrintXmppPingEnabled, false);
+  int timeout = prefs->GetInt(
+      prefs::kCloudPrintXmppPingTimeout, kDefaultXmppPingTimeoutSecs);
+  SetXmppPingTimeoutSec(timeout);
+
   const base::ListValue* printers = prefs->GetList(
       prefs::kCloudPrintPrinterBlacklist);
   if (printers) {
@@ -76,8 +85,18 @@ void ConnectorSettings::CopyFrom(const ConnectorSettings& source) {
   proxy_id_ = source.proxy_id();
   delete_on_enum_fail_ = source.delete_on_enum_fail();
   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_;
   if (source.print_system_settings())
     print_system_settings_.reset(source.print_system_settings()->DeepCopy());
 }
 
+void ConnectorSettings::SetXmppPingTimeoutSec(int timeout) {
+  xmpp_ping_timeout_sec_ = timeout;
+  if (xmpp_ping_timeout_sec_ < kMinimumXmppPingTimeoutSecs) {
+    LOG(WARNING) <<
+        "CP_CONNECTOR: XMPP ping timeout is less then minimal value";
+    xmpp_ping_timeout_sec_ = kMinimumXmppPingTimeoutSecs;
+  }
+}

chrome/service/cloud_print/connector_settings.h

@@ -42,12 +42,26 @@ class ConnectorSettings {
     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_;
+  }
+
   const base::DictionaryValue* print_system_settings() const {
     return print_system_settings_.get();
   };
 
   bool IsPrinterBlacklisted(const std::string& name) const;
 
+  void SetXmppPingTimeoutSec(int timeout);
+
  private:
   // Cloud Print server url.
   GURL server_url_;
@@ -63,6 +77,12 @@ class ConnectorSettings {
   // If true register all new printers in cloud print.
   bool connect_new_printers_;
 
+  // Indicate if XMPP pings are enabled.
+  bool xmpp_ping_enabled_;
+
+  // Indicate timeout between XMPP pings.
+  int xmpp_ping_timeout_sec_;
+
   // List of printers which should not be connected.
   std::set<std::string> printer_blacklist_;
 

chrome/service/cloud_print/connector_settings_unittest.cc

@@ -11,6 +11,7 @@
 #include "base/message_loop_proxy.h"
 #include "base/scoped_temp_dir.h"
 #include "base/values.h"
+#include "chrome/service/cloud_print/cloud_print_consts.h"
 #include "chrome/service/service_process_prefs.h"
 
 #include "testing/gmock/include/gmock/gmock.h"
@@ -28,6 +29,8 @@ const char kServiceStateContent[] =
     "      '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'"
@@ -83,6 +86,7 @@ TEST_F(ConnectorSettingsTest, InitFromEmpty) {
     EXPECT_FALSE(settings.delete_on_enum_fail());
     EXPECT_EQ(NULL, settings.print_system_settings());
     EXPECT_TRUE(settings.connect_new_printers());
+    EXPECT_FALSE(settings.xmpp_ping_enabled());
     EXPECT_FALSE(settings.IsPrinterBlacklisted("prn1"));
   }
 }
@@ -97,6 +101,8 @@ TEST_F(ConnectorSettingsTest, InitFromFile) {
   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"));
 }
@@ -115,6 +121,25 @@ TEST_F(ConnectorSettingsTest, CopyFrom) {
   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"));
 }
 
+TEST_F(ConnectorSettingsTest, SettersTest) {
+  scoped_ptr<ServiceProcessPrefs> prefs(CreateTestFile("{}"));
+  ConnectorSettings settings;
+  settings.InitFrom(prefs.get());
+  EXPECT_FALSE(settings.xmpp_ping_enabled());
+
+  // Set and check valid settings.
+  settings.set_xmpp_ping_enabled(true);
+  settings.SetXmppPingTimeoutSec(256);
+  EXPECT_TRUE(settings.xmpp_ping_enabled());
+  EXPECT_EQ(settings.xmpp_ping_timeout_sec(), 256);
+
+  // Set invalid settings, and check correct defaults.
+  settings.SetXmppPingTimeoutSec(1);
+  EXPECT_EQ(settings.xmpp_ping_timeout_sec(), kMinimumXmppPingTimeoutSecs);
+}

chrome/service/service_process_prefs.cc

@@ -54,6 +54,21 @@ void ServiceProcessPrefs::SetBoolean(const std::string& key, bool value) {
   prefs_->SetValue(key, Value::CreateBooleanValue(value));
 }
 
+int ServiceProcessPrefs::GetInt(const std::string& key,
+                                int default_value) const {
+  const Value* value;
+  int result = default_value;
+  if (prefs_->GetValue(key, &value) != PersistentPrefStore::READ_OK ||
+      !value->GetAsInteger(&result)) {
+    return default_value;
+  }
+  return result;
+}
+
+void ServiceProcessPrefs::SetInt(const std::string& key, int value) {
+  prefs_->SetValue(key, Value::CreateIntegerValue(value));
+}
+
 const DictionaryValue* ServiceProcessPrefs::GetDictionary(
     const std::string& key) const {
   const Value* value;

chrome/service/service_process_prefs.h

@@ -43,6 +43,12 @@ class ServiceProcessPrefs {
   // Set a boolean |value| for |key|.
   void SetBoolean(const std::string& key, bool value);
 
+  // Returns an int preference for |key|.
+  int GetInt(const std::string& key, int default_value) const;
+
+  // Set an int |value| for |key|.
+  void SetInt(const std::string& key, int value);
+
   // Returns a dictionary preference for |key|.
   const base::DictionaryValue* GetDictionary(const std::string& key) const;
 

jingle/jingle.gyp

@@ -95,6 +95,8 @@
             'notifier/listener/push_notifications_send_update_task.h',
             'notifier/listener/push_notifications_subscribe_task.cc',
             'notifier/listener/push_notifications_subscribe_task.h',
+            'notifier/listener/send_ping_task.cc',
+            'notifier/listener/send_ping_task.h',
             'notifier/listener/xml_element_util.cc',
             'notifier/listener/xml_element_util.h',
             'notifier/listener/xmpp_push_client.cc',
@@ -179,6 +181,7 @@
             'notifier/listener/push_client_unittest.cc',
             'notifier/listener/push_notifications_send_update_task_unittest.cc',
             'notifier/listener/push_notifications_subscribe_task_unittest.cc',
+            'notifier/listener/send_ping_task_unittest.cc',
             'notifier/listener/xml_element_util_unittest.cc',
             'notifier/listener/xmpp_push_client_unittest.cc',
             'run_all_unittests.cc',