Alarm resolution changed to minutes and minimum delay added.

There is a 5 minute minimum delay enforced for released extensions
and a 0 minute delay enforced for extensions in development.

BUG=122821
TEST=Call chrome.experimental.alarms.create and verify that the alarm is delayed in the number of minutes passed in.


Review URL: http://codereview.chromium.org/10217018

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

11 files changed, 102 insertions, 52 deletions

diff --git a/chrome/browser/extensions/api/alarms/alarm_manager.cc b/chrome/browser/extensions/api/alarms/alarm_manager.cc
index f6c5080..08f0681 100644
--- a/chrome/browser/extensions/api/alarms/alarm_manager.cc
+++ b/chrome/browser/extensions/api/alarms/alarm_manager.cc
@@ -55,8 +55,9 @@ AlarmManager::~AlarmManager() {
 
 void AlarmManager::AddAlarm(const std::string& extension_id,
                             const linked_ptr<Alarm>& alarm) {
-  AddAlarmImpl(extension_id, alarm,
-               base::TimeDelta::FromSeconds(alarm->delay_in_seconds));
+  base::TimeDelta alarm_time = base::TimeDelta::FromMicroseconds(
+      alarm->delay_in_minutes * base::Time::kMicrosecondsPerMinute);
+  AddAlarmImpl(extension_id, alarm, alarm_time);
   WriteToPrefs(extension_id);
 }
 
@@ -141,10 +142,11 @@ void AlarmManager::OnAlarm(const std::string& extension_id,
     // Restart the timer, since it may have been set with a shorter delay
     // initially.
     base::Timer* timer = timers_[alarm].get();
-    timer->Start(FROM_HERE,
-        base::TimeDelta::FromSeconds(alarm->delay_in_seconds),
-        base::Bind(&AlarmManager::OnAlarm, base::Unretained(this),
-                   extension_id, alarm->name));
+    base::TimeDelta alarm_time = base::TimeDelta::FromMicroseconds(
+        alarm->delay_in_minutes * base::Time::kMicrosecondsPerMinute);
+    timer->Start(FROM_HERE, alarm_time,
+                 base::Bind(&AlarmManager::OnAlarm, base::Unretained(this),
+                 extension_id, alarm->name));
   }
 
   WriteToPrefs(extension_id);
diff --git a/chrome/browser/extensions/api/alarms/alarms_api.cc b/chrome/browser/extensions/api/alarms/alarms_api.cc
index 49c3dd6..bea10e3 100644
--- a/chrome/browser/extensions/api/alarms/alarms_api.cc
+++ b/chrome/browser/extensions/api/alarms/alarms_api.cc
@@ -4,6 +4,7 @@
 
 #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"
@@ -18,17 +19,49 @@ namespace {
 
 const char kDefaultAlarmName[] = "";
 const char kAlarmNotFound[] = "No alarm named '*' exists.";
+const char kDelayLessThanMinimum[] = "Delay is less than minimum of * minutes.";
+const char kDelayIsNonInteger[] = "Delay is not an integer value.";
+
+const int kReleaseDelayMinimum = 5;
+const int kDevDelayMinimum = 0;
+
+bool ValidateDelayTime(double delay_in_minutes,
+                       const Extension* extension,
+                       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 = kDelayIsNonInteger;
+      return false;
+    }
+    delay_minimum = kReleaseDelayMinimum;
+  }
 
+  // Validate against our found delay minimum.
+  if (delay_in_minutes < delay_minimum) {
+    *error = ExtensionErrorUtils::FormatErrorMessage(kDelayLessThanMinimum,
+        base::DoubleToString(delay_minimum));
+    return false;
+  }
+  return true;
 }
 
+}  // namespace
+
 bool AlarmsCreateFunction::RunImpl() {
   scoped_ptr<alarms::Create::Params> params(
       alarms::Create::Params::Create(*args_));
   EXTENSION_FUNCTION_VALIDATE(params.get());
 
+  double delay_in_minutes = params->alarm_info.delay_in_minutes;
+  if (!ValidateDelayTime(delay_in_minutes, GetExtension(), &error_))
+    return false;
+
   linked_ptr<AlarmManager::Alarm> alarm(new AlarmManager::Alarm());
   alarm->name = params->name.get() ? *params->name : kDefaultAlarmName;
-  alarm->delay_in_seconds = params->alarm_info.delay_in_seconds;
+  alarm->delay_in_minutes = params->alarm_info.delay_in_minutes;
   alarm->repeating = params->alarm_info.repeating.get() ?
       *params->alarm_info.repeating : false;
   ExtensionSystem::Get(profile())->alarm_manager()->AddAlarm(
diff --git a/chrome/browser/extensions/api/alarms/alarms_api_unittest.cc b/chrome/browser/extensions/api/alarms/alarms_api_unittest.cc
index d7124a1..eda0808 100644
--- a/chrome/browser/extensions/api/alarms/alarms_api_unittest.cc
+++ b/chrome/browser/extensions/api/alarms/alarms_api_unittest.cc
@@ -47,7 +47,7 @@ class ExtensionAlarmsTest : public BrowserWithTestWindowTest {
     alarm_delegate_ = new AlarmDelegate();
     alarm_manager_->set_delegate(alarm_delegate_);
 
-    extension_ = utils::CreateEmptyExtension();
+    extension_ = utils::CreateEmptyExtensionWithLocation(Extension::LOAD);
   }
 
   base::Value* RunFunctionWithExtension(
@@ -101,9 +101,9 @@ class ExtensionAlarmsTest : public BrowserWithTestWindowTest {
     CHECK(num_alarms <= 3);
 
     const char* kCreateArgs[] = {
-      "[null, {\"delayInSeconds\": 1, \"repeating\": true}]",
-      "[\"7\", {\"delayInSeconds\": 7, \"repeating\": true}]",
-      "[\"0\", {\"delayInSeconds\": 0}]"
+      "[null, {\"delayInMinutes\": 0.001, \"repeating\": true}]",
+      "[\"7\", {\"delayInMinutes\": 7, \"repeating\": true}]",
+      "[\"0\", {\"delayInMinutes\": 0}]"
     };
     for (size_t i = 0; i < num_alarms; ++i) {
       scoped_ptr<base::DictionaryValue> result(RunFunctionAndReturnDict(
@@ -122,13 +122,13 @@ class ExtensionAlarmsTest : public BrowserWithTestWindowTest {
 
 TEST_F(ExtensionAlarmsTest, Create) {
   // Create 1 non-repeating alarm.
-  RunFunction(new AlarmsCreateFunction(), "[null, {\"delayInSeconds\": 0}]");
+  RunFunction(new AlarmsCreateFunction(), "[null, {\"delayInMinutes\": 0}]");
 
   const AlarmManager::Alarm* alarm =
       alarm_manager_->GetAlarm(extension_->id(), "");
   ASSERT_TRUE(alarm);
   EXPECT_EQ("", alarm->name);
-  EXPECT_EQ(0, alarm->delay_in_seconds);
+  EXPECT_EQ(0, alarm->delay_in_minutes);
   EXPECT_FALSE(alarm->repeating);
 
   // Now wait for the alarm to fire. Our test delegate will quit the
@@ -148,16 +148,14 @@ TEST_F(ExtensionAlarmsTest, Create) {
 
 TEST_F(ExtensionAlarmsTest, CreateRepeating) {
   // Create 1 repeating alarm.
-  // TODO(mpcmoplete): Use a shorter delay if we switch to allow fractions.
-  // A repeating timer with a 0-second delay fires infinitely.
   RunFunction(new AlarmsCreateFunction(),
-              "[null, {\"delayInSeconds\": 1, \"repeating\": true}]");
+              "[null, {\"delayInMinutes\": 0.001, \"repeating\": true}]");
 
   const AlarmManager::Alarm* alarm =
       alarm_manager_->GetAlarm(extension_->id(), "");
   ASSERT_TRUE(alarm);
   EXPECT_EQ("", alarm->name);
-  EXPECT_EQ(1, alarm->delay_in_seconds);
+  EXPECT_EQ(0.001, alarm->delay_in_minutes);
   EXPECT_TRUE(alarm->repeating);
 
   // Now wait for the alarm to fire. Our test delegate will quit the
@@ -173,18 +171,25 @@ TEST_F(ExtensionAlarmsTest, CreateRepeating) {
 
 TEST_F(ExtensionAlarmsTest, CreateDupe) {
   // Create 2 duplicate alarms. The first should be overridden.
-  RunFunction(new AlarmsCreateFunction(), "[\"dup\", {\"delayInSeconds\": 1}]");
-  RunFunction(new AlarmsCreateFunction(), "[\"dup\", {\"delayInSeconds\": 7}]");
+  RunFunction(new AlarmsCreateFunction(), "[\"dup\", {\"delayInMinutes\": 1}]");
+  RunFunction(new AlarmsCreateFunction(), "[\"dup\", {\"delayInMinutes\": 7}]");
 
   {
     const AlarmManager::AlarmList* alarms =
         alarm_manager_->GetAllAlarms(extension_->id());
     ASSERT_TRUE(alarms);
     EXPECT_EQ(1u, alarms->size());
-    EXPECT_EQ(7, (*alarms)[0]->delay_in_seconds);
+    EXPECT_EQ(7, (*alarms)[0]->delay_in_minutes);
   }
 }
 
+TEST_F(ExtensionAlarmsTest, CreateDelayBelowMinimum) {
+  // Create an alarm with delay below the minimum accepted value.
+  std::string error = RunFunctionAndReturnError(new AlarmsCreateFunction(),
+      "[\"negative\", {\"delayInMinutes\": -0.2}]");
+  EXPECT_FALSE(error.empty());
+}
+
 TEST_F(ExtensionAlarmsTest, Get) {
   // Create 2 alarms, and make sure we can query them.
   CreateAlarms(2);
@@ -197,7 +202,7 @@ TEST_F(ExtensionAlarmsTest, Get) {
     ASSERT_TRUE(result.get());
     EXPECT_TRUE(AlarmManager::Alarm::Populate(*result, &alarm));
     EXPECT_EQ("", alarm.name);
-    EXPECT_EQ(1, alarm.delay_in_seconds);
+    EXPECT_EQ(0.001, alarm.delay_in_minutes);
     EXPECT_TRUE(alarm.repeating);
   }
 
@@ -209,7 +214,7 @@ TEST_F(ExtensionAlarmsTest, Get) {
     ASSERT_TRUE(result.get());
     EXPECT_TRUE(AlarmManager::Alarm::Populate(*result, &alarm));
     EXPECT_EQ("7", alarm.name);
-    EXPECT_EQ(7, alarm.delay_in_seconds);
+    EXPECT_EQ(7, alarm.delay_in_minutes);
     EXPECT_TRUE(alarm.repeating);
   }
 
@@ -244,7 +249,7 @@ TEST_F(ExtensionAlarmsTest, GetAll) {
     if (alarm->name != "7")
       alarm = alarms[1].get();
     EXPECT_EQ("7", alarm->name);
-    EXPECT_EQ(7, alarm->delay_in_seconds);
+    EXPECT_EQ(7, alarm->delay_in_minutes);
     EXPECT_TRUE(alarm->repeating);
   }
 }
@@ -260,7 +265,7 @@ TEST_F(ExtensionAlarmsTest, Clear) {
   // Create 3 alarms.
   CreateAlarms(3);
 
-  // Clear all but the 1-second one.
+  // Clear all but the 0.001-minute alarm.
   {
     RunFunction(new AlarmsClearFunction(), "[\"7\"]");
     RunFunction(new AlarmsClearFunction(), "[\"0\"]");
@@ -269,7 +274,7 @@ TEST_F(ExtensionAlarmsTest, Clear) {
         alarm_manager_->GetAllAlarms(extension_->id());
     ASSERT_TRUE(alarms);
     EXPECT_EQ(1u, alarms->size());
-    EXPECT_EQ(1, (*alarms)[0]->delay_in_seconds);
+    EXPECT_EQ(0.001, (*alarms)[0]->delay_in_minutes);
   }
 
   // Now wait for the alarms to fire, and ensure the cancelled alarms don't
@@ -279,13 +284,13 @@ TEST_F(ExtensionAlarmsTest, Clear) {
   ASSERT_EQ(1u, alarm_delegate_->alarms_seen.size());
   EXPECT_EQ("", alarm_delegate_->alarms_seen[0]);
 
-  // Ensure the 1-second alarm is still there, since its repeating.
+  // Ensure the 0.001-minute alarm is still there, since it's repeating.
   {
     const AlarmManager::AlarmList* alarms =
         alarm_manager_->GetAllAlarms(extension_->id());
     ASSERT_TRUE(alarms);
     EXPECT_EQ(1u, alarms->size());
-    EXPECT_EQ(1, (*alarms)[0]->delay_in_seconds);
+    EXPECT_EQ(0.001, (*alarms)[0]->delay_in_minutes);
   }
 }
 
diff --git a/chrome/browser/extensions/extension_function_test_utils.cc b/chrome/browser/extensions/extension_function_test_utils.cc
index f5561f3..40c0c9a 100644
--- a/chrome/browser/extensions/extension_function_test_utils.cc
+++ b/chrome/browser/extensions/extension_function_test_utils.cc
@@ -98,13 +98,18 @@ base::ListValue* ToList(base::Value* val) {
 }
 
 scoped_refptr<Extension> CreateEmptyExtension() {
+  return CreateEmptyExtensionWithLocation(Extension::INTERNAL);
+}
+
+scoped_refptr<Extension> CreateEmptyExtensionWithLocation(
+    Extension::Location location) {
   std::string error;
   const FilePath test_extension_path;
   scoped_ptr<base::DictionaryValue> test_extension_value(
       ParseDictionary("{\"name\": \"Test\", \"version\": \"1.0\"}"));
   scoped_refptr<Extension> extension(Extension::Create(
       test_extension_path,
-      Extension::INTERNAL,
+      location,
       *test_extension_value.get(),
       Extension::NO_FLAGS,
       &error));
diff --git a/chrome/browser/extensions/extension_function_test_utils.h b/chrome/browser/extensions/extension_function_test_utils.h
index 225974d..27336a5 100644
--- a/chrome/browser/extensions/extension_function_test_utils.h
+++ b/chrome/browser/extensions/extension_function_test_utils.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -9,6 +9,7 @@
 #include <string>
 
 #include "base/memory/ref_counted.h"
+#include "chrome/common/extensions/extension.h"
 
 class AsyncExtensionFunction;
 class Browser;
@@ -46,6 +47,11 @@ base::ListValue* ToList(base::Value* val);
 // before running it.
 scoped_refptr<Extension> CreateEmptyExtension();
 
+// Creates an extension instance with a specified location that can be attached
+// to an ExtensionFunction before running.
+scoped_refptr<Extension> CreateEmptyExtensionWithLocation(
+    Extension::Location location);
+
 enum RunFunctionFlags {
   NONE = 0,
   INCLUDE_INCOGNITO = 1 << 0
diff --git a/chrome/common/extensions/api/experimental.alarms.idl b/chrome/common/extensions/api/experimental.alarms.idl
index c23a0ff..b3fdd1a 100644
--- a/chrome/common/extensions/api/experimental.alarms.idl
+++ b/chrome/common/extensions/api/experimental.alarms.idl
@@ -10,10 +10,10 @@ namespace experimental.alarms {
     // Name of this alarm.
     DOMString name;
 
-    // Original length of time in seconds after which the onAlarm event should
+    // Original length of time in minutes after which the onAlarm event should
     // fire.
     // TODO: need minimum=0
-    long delayInSeconds;
+    double delayInMinutes;
 
     // True if the alarm repeatedly fires at regular intervals, false if it
     // only fires once.
@@ -23,12 +23,12 @@ namespace experimental.alarms {
   // TODO(mpcomplete): rename to CreateInfo when http://crbug.com/123073 is
   // fixed.
   dictionary AlarmCreateInfo {
-    // Length of time in seconds after which the onAlarm event should fire.
+    // Length of time in minutes after which the onAlarm event should fire.
     // Note that granularity is not guaranteed: this value is more of a hint to
     // the browser. For performance reasons, alarms may be delayed an arbitrary
     // amount of time before firing.
     // TODO: need minimum=0
-    long delayInSeconds;
+    double delayInMinutes;
 
     // True if the alarm should repeatedly fire at regular intervals. Defaults
     // to false.
diff --git a/chrome/common/extensions/docs/examples/api/eventPage/basic/background.js b/chrome/common/extensions/docs/examples/api/eventPage/basic/background.js
index 6d9917b..57f051b 100644
--- a/chrome/common/extensions/docs/examples/api/eventPage/basic/background.js
+++ b/chrome/common/extensions/docs/examples/api/eventPage/basic/background.js
@@ -58,7 +58,7 @@ chrome.experimental.keybinding.onCommand.addListener(function(command) {
 
 chrome.extension.onMessage.addListener(function(msg, _, sendResponse) {
   if (msg.setAlarm) {
-    chrome.experimental.alarms.create({delayInSeconds: 5});
+    chrome.experimental.alarms.create({delayInMinutes: 0.1});
   } else if (msg.delayedResponse) {
     // Note: setTimeout itself does NOT keep the page awake. We return true
     // from the onMessage event handler, which keeps the message channel open -
diff --git a/chrome/common/extensions/docs/experimental.alarms.html b/chrome/common/extensions/docs/experimental.alarms.html
index 0558682..b020cd1 100644
--- a/chrome/common/extensions/docs/experimental.alarms.html
+++ b/chrome/common/extensions/docs/experimental.alarms.html
@@ -741,21 +741,21 @@
               <div>
                 <div>
           <dt>
-            <var>delayInSeconds</var>
+            <var>delayInMinutes</var>
               <em>
                 <!-- TYPE -->
                 <div style="display:inline">
                   (
                     <span id="typeTemplate">
                       <span>
-                        <span>integer</span>
+                        <span>number</span>
                       </span>
                     </span>
                   )
                 </div>
               </em>
           </dt>
-          <dd>Original length of time in seconds after which the onAlarm event should fire. TODO: need minimum=0</dd>
+          <dd>Original length of time in minutes after which the onAlarm event should fire. TODO: need minimum=0</dd>
           <!-- OBJECT PROPERTIES -->
           <!-- OBJECT METHODS -->
           <!-- OBJECT EVENT FIELDS -->
@@ -841,21 +841,21 @@
               <div>
                 <div>
           <dt>
-            <var>delayInSeconds</var>
+            <var>delayInMinutes</var>
               <em>
                 <!-- TYPE -->
                 <div style="display:inline">
                   (
                     <span id="typeTemplate">
                       <span>
-                        <span>integer</span>
+                        <span>number</span>
                       </span>
                     </span>
                   )
                 </div>
               </em>
           </dt>
-          <dd>Length of time in seconds after which the onAlarm event should fire. Note that granularity is not guaranteed: this value is more of a hint to the browser. For performance reasons, alarms may be delayed an arbitrary amount of time before firing. TODO: need minimum=0</dd>
+          <dd>Length of time in minutes after which the onAlarm event should fire. Note that granularity is not guaranteed: this value is more of a hint to the browser. For performance reasons, alarms may be delayed an arbitrary amount of time before firing. TODO: need minimum=0</dd>
           <!-- OBJECT PROPERTIES -->
           <!-- OBJECT METHODS -->
           <!-- OBJECT EVENT FIELDS -->
diff --git a/chrome/common/extensions/docs/samples.json b/chrome/common/extensions/docs/samples.json
index cb4b1f4..a413f2f 100644
--- a/chrome/common/extensions/docs/samples.json
+++ b/chrome/common/extensions/docs/samples.json
@@ -666,7 +666,7 @@
         "style.css",
         "view.js"
       ],
-      "source_hash": "6317b07927fa12ebf02947b9018af0c9d244fccb",
+      "source_hash": "51200651a807c2c6e365c7b7228373b61ab3230b",
       "zip_path": "examples/extensions/calculator.zip"
     },
     {
@@ -1085,7 +1085,7 @@
         "content.js",
         "manifest.json"
       ],
-      "source_hash": "da33fa65c04b727c11cfab8c46e548f34808aaed",
+      "source_hash": "e9b2ca00aad14c452e444f5972f28124ea67168a",
       "zip_path": "examples/api/eventPage/basic.zip"
     },
     {
diff --git a/tools/json_schema_compiler/cc_generator.py b/tools/json_schema_compiler/cc_generator.py
index b8adaaf..3efad84 100644
--- a/tools/json_schema_compiler/cc_generator.py
+++ b/tools/json_schema_compiler/cc_generator.py
@@ -455,21 +455,16 @@ class CCGenerator(object):
     c = Code()
     c.Sblock('{')
 
-    if check_type and prop.type_ not in (
-        PropertyType.CHOICES, PropertyType.ANY):
-      (c.Append('if (!%(value_var)s->IsType(%(value_type)s))')
-        .Append('  return %(failure_value)s;')
-      )
-
     if self._IsFundamentalOrFundamentalRef(prop):
       if prop.optional:
         (c.Append('%(ctype)s temp;')
-          .Append('if (%s)' %
+          .Append('if (!%s)' %
               cpp_util.GetAsFundamentalValue(
                   self._cpp_type_generator.GetReferencedProperty(prop),
                   value_var,
                   '&temp'))
-          .Append('  %(dst)s->%(name)s.reset(new %(ctype)s(temp));')
+          .Append('  return %(failure_value)s;')
+          .Append('%(dst)s->%(name)s.reset(new %(ctype)s(temp));')
         )
       else:
         (c.Append('if (!%s)' %
@@ -477,7 +472,7 @@ class CCGenerator(object):
                 self._cpp_type_generator.GetReferencedProperty(prop),
                 value_var,
                 '&%s->%s' % (dst, prop.unix_name)))
-          .Append('return %(failure_value)s;')
+          .Append('  return %(failure_value)s;')
         )
     elif self._IsObjectOrObjectRef(prop):
       if prop.optional:
@@ -551,7 +546,9 @@ class CCGenerator(object):
       # This is the same if the property is optional or not. We need a pointer
       # to the BinaryValue to be able to populate it, so a scoped_ptr is used
       # whether it is optional or required.
-      (c.Append('%(dst)s->%(name)s.reset(')
+      (c.Append('if (!%(value_var)s->IsType(%(value_type)s))')
+        .Append('  return %(failure_value)s;')
+        .Append('%(dst)s->%(name)s.reset(')
         .Append('    static_cast<BinaryValue*>(%(value_var)s)->DeepCopy());')
       )
     else:
diff --git a/tools/json_schema_compiler/idl_schema.py b/tools/json_schema_compiler/idl_schema.py
index 3a7e6e4..087b2f4 100644
--- a/tools/json_schema_compiler/idl_schema.py
+++ b/tools/json_schema_compiler/idl_schema.py
@@ -161,6 +161,8 @@ class Typeref(object):
       properties['type'] = 'string'
     elif self.typeref == 'boolean':
       properties['type'] = 'boolean'
+    elif self.typeref == 'double':
+      properties['type'] = 'number'
     elif self.typeref == 'long':
       properties['type'] = 'integer'
     elif self.typeref == 'any':