Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(263)

Issue 10217018: Alarm resolution changed to minutes and minimum delay added. (Closed)

Created:
8 years, 8 months ago by Matt Tytel
Modified:
8 years, 7 months ago
Reviewers:
Yoyo Zhou, Matt Perry
CC:
chromium-reviews, Aaron Boodman, pam+watch_chromium.org, mihaip+watch_chromium.org
Base URL:
http://git.chromium.org/chromium/src.git@master
Visibility:
Public.

Description

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. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=135559

Patch Set 1 : #

Total comments: 11

Patch Set 2 : Made ValidateDelayTime global and used GetExtension instead. #

Total comments: 6

Patch Set 3 : Style changes. #

Patch Set 4 : Synced. #

Patch Set 5 : Fixed type checks for optional string. #

Total comments: 2

Patch Set 6 : Synced #

Patch Set 7 : Sinked. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+100 lines, -50 lines) Patch
M chrome/browser/extensions/api/alarms/alarm_manager.cc View 2 chunks +8 lines, -6 lines 0 comments Download
M chrome/browser/extensions/api/alarms/alarms_api.cc View 1 2 2 chunks +34 lines, -1 line 0 comments Download
M chrome/browser/extensions/api/alarms/alarms_api_unittest.cc View 1 11 chunks +25 lines, -20 lines 0 comments Download
M chrome/browser/extensions/extension_function_test_utils.h View 1 2 chunks +6 lines, -0 lines 0 comments Download
M chrome/browser/extensions/extension_function_test_utils.cc View 1 1 chunk +6 lines, -1 line 0 comments Download
M chrome/common/extensions/api/experimental.alarms.idl View 1 2 chunks +4 lines, -4 lines 0 comments Download
M chrome/common/extensions/docs/examples/api/eventPage/basic.zip View 1 2 3 4 5 0 chunks +-1 lines, --1 lines 0 comments Download
M chrome/common/extensions/docs/examples/api/eventPage/basic/background.js View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M chrome/common/extensions/docs/experimental.alarms.html View 1 2 3 2 chunks +6 lines, -6 lines 0 comments Download
M chrome/common/extensions/docs/samples.json View 1 2 3 4 5 6 2 chunks +2 lines, -2 lines 0 comments Download
M tools/json_schema_compiler/cc_generator.py View 1 2 3 4 3 chunks +7 lines, -10 lines 0 comments Download
M tools/json_schema_compiler/idl_schema.py View 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 17 (0 generated)
Matt Tytel
8 years, 8 months ago (2012-04-25 02:59:36 UTC) #1
Matt Perry
https://chromiumcodereview.appspot.com/10217018/diff/2001/chrome/browser/extensions/api/alarms/alarms_api.cc File chrome/browser/extensions/api/alarms/alarms_api.cc (right): https://chromiumcodereview.appspot.com/10217018/diff/2001/chrome/browser/extensions/api/alarms/alarms_api.cc#newcode28 chrome/browser/extensions/api/alarms/alarms_api.cc:28: const double kDevDelayMinimum = 0.1; Let's allow 0 minute ...
8 years, 8 months ago (2012-04-25 19:11:06 UTC) #2
Matt Perry
BTW, TEST= is for QA. See https://sites.google.com/a/chromium.org/dev/developers/contributing-code?pli=1#TOC-GCL
8 years, 8 months ago (2012-04-25 19:12:01 UTC) #3
Matt Tytel
https://chromiumcodereview.appspot.com/10217018/diff/2001/chrome/browser/extensions/api/alarms/alarms_api.cc File chrome/browser/extensions/api/alarms/alarms_api.cc (right): https://chromiumcodereview.appspot.com/10217018/diff/2001/chrome/browser/extensions/api/alarms/alarms_api.cc#newcode28 chrome/browser/extensions/api/alarms/alarms_api.cc:28: const double kDevDelayMinimum = 0.1; On 2012/04/25 19:11:06, Matt ...
8 years, 7 months ago (2012-04-30 23:36:15 UTC) #4
Matt Perry
https://chromiumcodereview.appspot.com/10217018/diff/2001/tools/json_schema_compiler/cc_generator.py File tools/json_schema_compiler/cc_generator.py (left): https://chromiumcodereview.appspot.com/10217018/diff/2001/tools/json_schema_compiler/cc_generator.py#oldcode460 tools/json_schema_compiler/cc_generator.py:460: ) On 2012/04/30 23:36:21, Matt Tytel wrote: > Ah, ...
8 years, 7 months ago (2012-04-30 23:51:42 UTC) #5
Matt Tytel
Fixed those style issues. https://chromiumcodereview.appspot.com/10217018/diff/10006/chrome/browser/extensions/api/alarms/alarms_api.cc File chrome/browser/extensions/api/alarms/alarms_api.cc (right): https://chromiumcodereview.appspot.com/10217018/diff/10006/chrome/browser/extensions/api/alarms/alarms_api.cc#newcode28 chrome/browser/extensions/api/alarms/alarms_api.cc:28: bool ValidateDelayTime(double delay_in_minutes, const Extension* ...
8 years, 7 months ago (2012-05-01 00:10:34 UTC) #6
Matt Perry
lgtm
8 years, 7 months ago (2012-05-01 00:56:44 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mtytel@chromium.org/10217018/12003
8 years, 7 months ago (2012-05-01 02:01:24 UTC) #8
commit-bot: I haz the power
Try job failure for 10217018-12003 (retry) on mac_rel for step "unit_tests". It's a second try, ...
8 years, 7 months ago (2012-05-01 02:50:52 UTC) #9
Matt Tytel
Optional strings were using GetAsString but they weren't returning false if it didn't pass like ...
8 years, 7 months ago (2012-05-02 02:18:02 UTC) #10
Matt Perry
Not sure I understand the schema code generator, but lgtm
8 years, 7 months ago (2012-05-05 00:24:45 UTC) #11
Yoyo Zhou
A few drive-by comments from the peanut gallery. https://chromiumcodereview.appspot.com/10217018/diff/23002/chrome/browser/extensions/api/alarms/alarm_manager.cc File chrome/browser/extensions/api/alarms/alarm_manager.cc (right): https://chromiumcodereview.appspot.com/10217018/diff/23002/chrome/browser/extensions/api/alarms/alarm_manager.cc#newcode58 chrome/browser/extensions/api/alarms/alarm_manager.cc:58: base::TimeDelta ...
8 years, 7 months ago (2012-05-05 00:48:58 UTC) #12
Yoyo Zhou
Sorry! Matt pointed out that all of my comments are inapplicable. On Fri, May 4, ...
8 years, 7 months ago (2012-05-05 00:53:20 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mtytel@chromium.org/10217018/34002
8 years, 7 months ago (2012-05-05 01:22:19 UTC) #14
commit-bot: I haz the power
Try job failure for 10217018-34002 on win_rel for step "update". http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&number=26105 Step "update" is always ...
8 years, 7 months ago (2012-05-05 01:29:19 UTC) #15
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mtytel@chromium.org/10217018/37002
8 years, 7 months ago (2012-05-05 20:03:49 UTC) #16
commit-bot: I haz the power
8 years, 7 months ago (2012-05-05 21:33:48 UTC) #17
Change committed as 135559

Powered by Google App Engine
This is Rietveld 408576698