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

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

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/extensions/api/alarms/alarms_api.h"
 
 #include "base/string_number_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/extensions/api/alarms/alarm_manager.h"
 #include "chrome/browser/extensions/extension_system.h"
 #include "chrome/common/extensions/api/alarms.h"
 #include "extensions/common/error_utils.h"
 
 namespace alarms = extensions::api::alarms;
 
 namespace extensions {
 
 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[] =
     "Must set at least one of when, delayInMinutes, or periodInMinutes.";
 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;
     return false;
   }
   if (create_info.delay_in_minutes == NULL &&
       create_info.when == NULL &&
       create_info.period_in_minutes == NULL) {
     *error = kNoScheduledTime;
     return false;
   }
 
   // Users can always use an absolute timeout to request an arbitrarily-short or
   // negative delay.  We won't honor the short timeout, but we can't check it
   // and warn the user because it would introduce race conditions (say they
   // compute a long-enough timeout, but then the call into the alarms interface
   // 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;
 }
 
 }  // namespace
 
 AlarmsCreateFunction::AlarmsCreateFunction()
     : now_(&base::Time::Now) {
 }
 
 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,
               base::TimeDelta::FromMinutes(
                   GetExtension()->location() == Extension::LOAD ?
                   kDevDelayMinimum : kReleaseDelayMinimum),
               now_);
   ExtensionSystem::Get(profile())->alarm_manager()->AddAlarm(
       extension_id(), alarm);
 
@@ skipping 52 matching lines @@
   return true;
 }
 
 bool AlarmsClearAllFunction::RunImpl() {
   ExtensionSystem::Get(profile())->alarm_manager()->RemoveAllAlarms(
      extension_id());
   return true;
 }
 
 }  // namespace extensions