Make chrome.alarms API accept non-integers and too-short delays.

chrome/browser/extensions/api/alarms/alarms_api.cc

Index: chrome/browser/extensions/api/alarms/alarms_api.cc
diff --git a/chrome/browser/extensions/api/alarms/alarms_api.cc b/chrome/browser/extensions/api/alarms/alarms_api.cc
index 938ded45dc2885f1eec8d8bff69aa30454e0f729..091a20b25f3f2e95f597820898efad8a0d45e9f5 100644
--- a/chrome/browser/extensions/api/alarms/alarms_api.cc
+++ b/chrome/browser/extensions/api/alarms/alarms_api.cc
@@ -19,8 +19,6 @@ namespace {
 
 const char kDefaultAlarmName[] = "";
 const char kAlarmNotFound[] = "No alarm named '*' exists.";
-const char kDelayLessThanMinimum[] = "* is less than minimum of * minutes.";
-const char kDelayIsNonInteger[] = "* is not an integer value.";
 const char kBothRelativeAndAbsoluteTime[] =
     "Cannot set both when and delayInMinutes.";
 const char kNoScheduledTime[] =
@@ -28,38 +26,10 @@ const char kNoScheduledTime[] =
 const int kReleaseDelayMinimum = 5;

[Yoyo Zhou, 2013/01/09 00:38:00]

Per crbug.com/153838 the minimum was changed from 5 minutes to 1 minute (that's
why you see the 1 minute in alarm_manager). I don't know if that's still
necessary though (there were other bugs in Gmail checker that had to be fixed),
and since it didn't touch this, it may not have actually done anything =/.

Matt, any thoughts?

[Matt Perry, 2013/01/09 00:58:00]

Ah, I forgot we had that bug. It looks like it wasn't fixed. Let's update the
minimum to 1.

[Jeffrey Yasskin, 2013/01/09 01:04:45]

It probably changed the first poll time but not later polls. Since the gmail
checker reschedules its alarm each time it fires, the granularity wasn't getting
enforced.

... Hm, actually the gmail checker uses a poll interval of 5 minutes, at least,
so it won't be affected by this change.

Other extensions that were using 'when' to get around the limit might be
affected, but they really shouldn't be waking up the event page that often
anyway.

 const int kDevDelayMinimum = 0;
 
-bool ValidateDelay(double delay_in_minutes,
-                   const Extension* extension,
-                   const std::string& delay_or_period,
-                   std::string* error) {
-  double delay_minimum = kDevDelayMinimum;
-  if (extension->location() != Extension::LOAD) {
-    // In release mode we check for integer delay values and a stricter delay
-    // minimum.
-    if (delay_in_minutes != static_cast<int>(delay_in_minutes)) {
-      *error = ErrorUtils::FormatErrorMessage(
-          kDelayIsNonInteger,
-          delay_or_period);
-      return false;
-    }
-    delay_minimum = kReleaseDelayMinimum;
-  }
-
-  // Validate against our found delay minimum.
-  if (delay_in_minutes < delay_minimum) {
-    *error = ErrorUtils::FormatErrorMessage(
-        kDelayLessThanMinimum,
-        delay_or_period,
-        base::DoubleToString(delay_minimum));
-    return false;
-  }
-
-  return true;
-}
-
 bool ValidateAlarmCreateInfo(const alarms::AlarmCreateInfo& create_info,
                              const Extension* extension,
-                             std::string* error) {
+                             std::string* error,
+                             std::vector<std::string>* warnings) {
   if (create_info.delay_in_minutes.get() &&
       create_info.when.get()) {
     *error = kBothRelativeAndAbsoluteTime;
@@ -79,14 +49,32 @@ bool ValidateAlarmCreateInfo(const alarms::AlarmCreateInfo& create_info,
   // gets delayed past the boundary).  However, it's still worth warning about
   // relative delays that are shorter than we'll honor.
   if (create_info.delay_in_minutes.get()) {
-    if (!ValidateDelay(*create_info.delay_in_minutes,
-                       extension, "Delay", error))
-      return false;
+    if (*create_info.delay_in_minutes < kReleaseDelayMinimum) {
+      COMPILE_ASSERT(kReleaseDelayMinimum == 5, update_warning_message_below);
+      if (extension->location() == Extension::LOAD)
+        warnings->push_back(
+            "Alarm delay is less than minimum of 5 minutes."
+            " In released .crx the alarm will fire in approximately"
+            " 5 minutes.");
+      else
+        warnings->push_back(
+            "Alarm delay is less than minimum of 5 minutes."
+            " The alarm will fire in approximately 5 minutes.");

[Yoyo Zhou, 2013/01/09 00:38:00]

Alarms have names - can we include those in the warning message?

[Jeffrey Yasskin, 2013/01/09 01:04:45]

Done.

+    }
   }
   if (create_info.period_in_minutes.get()) {
-    if (!ValidateDelay(*create_info.period_in_minutes,
-                       extension, "Period", error))
-      return false;
+    if (*create_info.period_in_minutes < kReleaseDelayMinimum) {
+      COMPILE_ASSERT(kReleaseDelayMinimum == 5, update_warning_message_below);
+      if (extension->location() == Extension::LOAD)
+        warnings->push_back(
+            "Alarm period is less than minimum of 5 minutes."
+            " In released .crx the alarm will fire approximately"
+            " every 5 minutes.");
+      else
+        warnings->push_back(
+            "Alarm period is less than minimum of 5 minutes."
+            " The alarm will fire approximately every 5 minutes.");
+    }
   }
 
   return true;
@@ -102,9 +90,13 @@ bool AlarmsCreateFunction::RunImpl() {
   scoped_ptr<alarms::Create::Params> params(
       alarms::Create::Params::Create(*args_));
   EXTENSION_FUNCTION_VALIDATE(params.get());
+  std::vector<std::string> warnings;
   if (!ValidateAlarmCreateInfo(
-          params->alarm_info, GetExtension(), &error_))
+          params->alarm_info, GetExtension(), &error_, &warnings))
     return false;
+  for (std::vector<std::string>::const_iterator it = warnings.begin();
+       it != warnings.end(); ++it)
+    WriteToConsole(content::CONSOLE_MESSAGE_LEVEL_WARNING, *it);
 
   Alarm alarm(params->name.get() ? *params->name : kDefaultAlarmName,
               params->alarm_info,