[GCM] fixing G-settings initialization from an empty store

google_apis/gcm/engine/gservices_settings.cc

Index: google_apis/gcm/engine/gservices_settings.cc
diff --git a/google_apis/gcm/engine/gservices_settings.cc b/google_apis/gcm/engine/gservices_settings.cc
index fc38e9d315b3475e8f1ad56661e0a7990270e1a4..c814302b63765f3a7fc28046cd7a56dcba092229 100644
--- a/google_apis/gcm/engine/gservices_settings.cc
+++ b/google_apis/gcm/engine/gservices_settings.cc
@@ -20,6 +20,7 @@ const char kMCSSecurePortKey[] = "gcm_secure_port";
 const char kRegistrationURLKey[] = "gcm_registration_url";
 
 const int64 kDefaultCheckinInterval = 2 * 24 * 60 * 60;  // seconds = 2 days.
+const int64 kMinimumCheckinInterval = 12 * 60 * 60;      // seconds = 12 hours.
 const char kDefaultCheckinURL[] = "https://android.clients.google.com/checkin";
 const char kDefaultMCSHostname[] = "https://mtalk.google.com";
 const int kDefaultMCSSecurePort = 5228;
@@ -77,61 +78,72 @@ void GServicesSettings::UpdateFromLoadResult(
 
 bool GServicesSettings::UpdateSettings(
     const std::map<std::string, std::string>& settings) {
-  int64 new_checkin_interval = 0LL;
+  int64 new_checkin_interval = kMinimumCheckinInterval;
   std::map<std::string, std::string>::const_iterator iter =
       settings.find(kCheckinIntervalKey);
-  if (iter != settings.end()) {
-    if (!base::StringToInt64(iter->second, &new_checkin_interval)) {
-      LOG(ERROR) << "Failed to parse checkin interval: " << iter->second;
-      return false;
-    }
-    if (new_checkin_interval <= 0LL) {
-      LOG(ERROR) << "Checkin interval not positive: " << new_checkin_interval;
-      return false;
-    }
+  if (iter == settings.end()) {
+    LOG(ERROR) << "Setting not found: " << kCheckinIntervalKey;
+    return false;
+  }
+  if (!base::StringToInt64(iter->second, &new_checkin_interval)) {
+    LOG(ERROR) << "Failed to parse checkin interval: " << iter->second;
+    return false;
+  }
+  if (new_checkin_interval < kMinimumCheckinInterval) {

[jianli, 2014/04/28 17:33:50]

Probably we should use kMinimumCheckinInterval if the new value is smaller than
it.

[fgorski, 2014/04/28 17:50:53]

Done. I was thinking about that, but it might no longer work if we are to
calculate a correct digest for v3. I think we can go around that though.

+    LOG(ERROR) << "Checkin interval: " << new_checkin_interval
+               << " is less than allowed minimum: " << kMinimumCheckinInterval;
+    return false;
   }
 
   std::string new_mcs_hostname;
-  int new_mcs_secure_port = -1;
   iter = settings.find(kMCSHostnameKey);
-  if (iter != settings.end()) {
-    new_mcs_hostname = iter->second;
-    if (new_mcs_hostname.empty()) {
-      LOG(ERROR) << "Empty MCS hostname provided.";
-      return false;
-    }
-
-    iter = settings.find(kMCSSecurePortKey);
-    if (iter != settings.end()) {
-      if (!base::StringToInt(iter->second, &new_mcs_secure_port)) {
-        LOG(ERROR) << "Failed to parse MCS secure port: " << iter->second;
-        return false;
-      }
-      if (new_mcs_secure_port < 0 || 65535 < new_mcs_secure_port) {
-        LOG(ERROR) << "Incorrect port value: " << new_mcs_secure_port;
-        return false;
-      }
-    }
+  if (iter == settings.end()) {
+    LOG(ERROR) << "Setting not found: " << kMCSHostnameKey;
+    return false;
+  }
+  new_mcs_hostname = iter->second;
+  if (new_mcs_hostname.empty()) {
+    LOG(ERROR) << "Empty MCS hostname provided.";
+    return false;
+  }
+
+  int new_mcs_secure_port = -1;
+  iter = settings.find(kMCSSecurePortKey);
+  if (iter == settings.end()) {
+    LOG(ERROR) << "Setting not found: " << kMCSSecurePortKey;
+    return false;
+  }
+  if (!base::StringToInt(iter->second, &new_mcs_secure_port)) {
+    LOG(ERROR) << "Failed to parse MCS secure port: " << iter->second;
+    return false;
+  }
+  if (new_mcs_secure_port < 0 || 65535 < new_mcs_secure_port) {
+    LOG(ERROR) << "Incorrect port value: " << new_mcs_secure_port;
+    return false;
   }
 
   std::string new_checkin_url;
   iter = settings.find(kCheckinURLKey);
-  if (iter != settings.end()) {
-    new_checkin_url = iter->second;
-    if (new_checkin_url.empty()) {
-      LOG(ERROR) << "Empty checkin URL provided.";
-      return false;
-    }
+  if (iter == settings.end()) {
+    LOG(ERROR) << "Setting not found: " << kCheckinURLKey;
+    return false;
+  }
+  new_checkin_url = iter->second;
+  if (new_checkin_url.empty()) {
+    LOG(ERROR) << "Empty checkin URL provided.";
+    return false;
   }
 
   std::string new_registration_url;
   iter = settings.find(kRegistrationURLKey);
-  if (iter != settings.end()) {
-    new_registration_url = iter->second;
-    if (new_registration_url.empty()) {
-      LOG(ERROR) << "Empty registration URL provided.";
-      return false;
-    }
+  if (iter == settings.end()) {
+    LOG(ERROR) << "Setting not found: " << kRegistrationURLKey;
+    return false;
+  }
+  new_registration_url = iter->second;
+  if (new_registration_url.empty()) {
+    LOG(ERROR) << "Empty registration URL provided.";
+    return false;
   }
 
   // We only update the settings once all of them are correct.