Chromium Code Reviews
Help | Chromium Project | Sign in
(11)

Issue 12093058: Screensaver implementation for ChromeOS. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 years, 1 month ago by Rahul Chaturvedi
Modified:
2 years ago
Reviewers:
stevenjb, Matt Perry
CC:
chromium-reviews, sadrul, nkostylev+watch_chromium.org, ben+watch_chromium.org, Aaron Boodman, oshima+watch_chromium.org, chromium-apps-reviews_chromium.org, stevenjb+watch_chromium.org, davemoore+watch_chromium.org
Visibility:
Public.

Description

Screensaver implementation for ChromeOS. This is the initial implementation for screensaver extensions. The feature is currently behind a flag and installing an extension with a screensaver permission without the flag enabled will do nothing. This currently allows one Screensaver extension to be installed at a time, and brings up the screensaver after a fixed 2 minutes. Further work to be done is to add different timeouts for power manager if the screensaver is active, add more tests and add security features for the screensaver permission (if needed, this needs to be discussed still). TBR'ed jhawkins for chrome_browser_chromeos.gypi R=mpcomplete@chromium.org,stevenjb@chromium.org BUG=163681 TBR=jhawkins@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=180189

Patch Set 1 #

Patch Set 2 : tests. #

Total comments: 31

Patch Set 3 : #

Patch Set 4 : fixed tests. #

Total comments: 4

Patch Set 5 : #

Total comments: 16

Patch Set 6 : #

Patch Set 7 : #

Patch Set 8 : merge #

Patch Set 9 : build fixes. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+421 lines, -10 lines) Patch
M ash/screensaver/screensaver_view.h View 1 2 3 4 5 6 7 8 3 chunks +4 lines, -0 lines 0 comments Download
M ash/screensaver/screensaver_view.cc View 1 2 3 4 5 6 7 8 2 chunks +15 lines, -0 lines 0 comments Download
M ash/screensaver/screensaver_view_unittest.cc View 1 chunk +1 line, -9 lines 0 comments Download
M chrome/app/generated_resources.grd View 1 chunk +6 lines, -0 lines 0 comments Download
M chrome/browser/about_flags.cc View 1 2 3 4 5 6 7 1 chunk +7 lines, -0 lines 0 comments Download
M chrome/browser/chromeos/chrome_browser_main_chromeos.h View 2 chunks +2 lines, -0 lines 0 comments Download
M chrome/browser/chromeos/chrome_browser_main_chromeos.cc View 1 2 3 4 5 6 7 2 chunks +5 lines, -0 lines 0 comments Download
A chrome/browser/chromeos/screensaver/screensaver_controller.h View 1 2 3 4 5 1 chunk +72 lines, -0 lines 0 comments Download
A chrome/browser/chromeos/screensaver/screensaver_controller.cc View 1 2 3 4 5 6 1 chunk +184 lines, -0 lines 0 comments Download
A chrome/browser/chromeos/screensaver/screensaver_controller_browsertest.cc View 1 2 3 1 chunk +97 lines, -0 lines 0 comments Download
M chrome/chrome_browser_chromeos.gypi View 1 2 3 4 5 6 7 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/chrome_tests.gypi View 1 2 3 4 5 6 7 1 chunk +1 line, -0 lines 0 comments Download
M chrome/common/extensions/api/_permission_features.json View 1 2 3 4 5 6 7 1 chunk +4 lines, -0 lines 0 comments Download
M chrome/common/extensions/permissions/api_permission.h View 1 2 3 4 5 6 7 1 chunk +1 line, -0 lines 0 comments Download
M chrome/common/extensions/permissions/api_permission.cc View 1 2 3 4 5 6 7 1 chunk +1 line, -0 lines 0 comments Download
M chrome/common/extensions/permissions/permission_set_unittest.cc View 1 2 3 4 5 6 7 8 1 chunk +1 line, -0 lines 0 comments Download
M chromeos/chromeos_switches.h View 1 chunk +1 line, -0 lines 0 comments Download
M chromeos/chromeos_switches.cc View 1 chunk +3 lines, -0 lines 0 comments Download
M chromeos/dbus/power_manager_client.cc View 1 2 3 4 5 6 7 3 chunks +14 lines, -1 line 0 comments Download
Commit:

Messages

Total messages: 13 (0 generated)
Rahul Chaturvedi
2 years, 1 month ago (2013-01-30 02:29:15 UTC) #1
Matt Perry
https://codereview.chromium.org/12093058/diff/3001/ash/screensaver/screensaver_view.cc File ash/screensaver/screensaver_view.cc (right): https://codereview.chromium.org/12093058/diff/3001/ash/screensaver/screensaver_view.cc#newcode65 ash/screensaver/screensaver_view.cc:65: if (g_instance) nit: can simplify to "return g_instance && ...
2 years, 1 month ago (2013-01-31 00:37:23 UTC) #2
Rahul Chaturvedi
https://codereview.chromium.org/12093058/diff/3001/ash/screensaver/screensaver_view.cc File ash/screensaver/screensaver_view.cc (right): https://codereview.chromium.org/12093058/diff/3001/ash/screensaver/screensaver_view.cc#newcode65 ash/screensaver/screensaver_view.cc:65: if (g_instance) On 2013/01/31 00:37:23, Matt Perry wrote: > ...
2 years, 1 month ago (2013-01-31 02:15:45 UTC) #3
Rahul Chaturvedi
https://codereview.chromium.org/12093058/diff/3001/chrome/browser/chromeos/screensaver/screensaver_controller_browsertest.cc File chrome/browser/chromeos/screensaver/screensaver_controller_browsertest.cc (right): https://codereview.chromium.org/12093058/diff/3001/chrome/browser/chromeos/screensaver/screensaver_controller_browsertest.cc#newcode18 chrome/browser/chromeos/screensaver/screensaver_controller_browsertest.cc:18: const extensions::Extension* extension = InstallExtension( On 2013/01/31 02:15:46, Rahul ...
2 years, 1 month ago (2013-01-31 02:53:18 UTC) #4
Matt Perry
lgtm https://codereview.chromium.org/12093058/diff/3001/chrome/browser/chromeos/screensaver/screensaver_controller.cc File chrome/browser/chromeos/screensaver/screensaver_controller.cc (right): https://codereview.chromium.org/12093058/diff/3001/chrome/browser/chromeos/screensaver/screensaver_controller.cc#newcode1 chrome/browser/chromeos/screensaver/screensaver_controller.cc:1: // Copyright (c) 2012 The Chromium Authors. All ...
2 years, 1 month ago (2013-01-31 22:43:50 UTC) #5
Rahul Chaturvedi
https://codereview.chromium.org/12093058/diff/12002/chrome/browser/chromeos/screensaver/screensaver_controller.cc File chrome/browser/chromeos/screensaver/screensaver_controller.cc (right): https://codereview.chromium.org/12093058/diff/12002/chrome/browser/chromeos/screensaver/screensaver_controller.cc#newcode108 chrome/browser/chromeos/screensaver/screensaver_controller.cc:108: if (!extension->HasAPIPermission(extensions::APIPermission::kScreensaver)) On 2013/01/31 22:43:51, Matt Perry wrote: > ...
2 years, 1 month ago (2013-01-31 22:53:55 UTC) #6
stevenjb
https://codereview.chromium.org/12093058/diff/11019/chrome/browser/chromeos/screensaver/screensaver_controller.cc File chrome/browser/chromeos/screensaver/screensaver_controller.cc (right): https://codereview.chromium.org/12093058/diff/11019/chrome/browser/chromeos/screensaver/screensaver_controller.cc#newcode51 chrome/browser/chromeos/screensaver/screensaver_controller.cc:51: return extension->id(); nit: {} https://codereview.chromium.org/12093058/diff/11019/chrome/browser/chromeos/screensaver/screensaver_controller.cc#newcode66 chrome/browser/chromeos/screensaver/screensaver_controller.cc:66: FindScreensaverExtension(service, current->id())).empty()) { ...
2 years, 1 month ago (2013-01-31 23:42:18 UTC) #7
Rahul Chaturvedi
https://codereview.chromium.org/12093058/diff/11019/chrome/browser/chromeos/screensaver/screensaver_controller.cc File chrome/browser/chromeos/screensaver/screensaver_controller.cc (right): https://codereview.chromium.org/12093058/diff/11019/chrome/browser/chromeos/screensaver/screensaver_controller.cc#newcode51 chrome/browser/chromeos/screensaver/screensaver_controller.cc:51: return extension->id(); On 2013/01/31 23:42:18, stevenjb (chromium) wrote: > ...
2 years, 1 month ago (2013-02-01 00:35:45 UTC) #8
stevenjb
lgtm https://codereview.chromium.org/12093058/diff/11019/chrome/browser/chromeos/screensaver/screensaver_controller.cc File chrome/browser/chromeos/screensaver/screensaver_controller.cc (right): https://codereview.chromium.org/12093058/diff/11019/chrome/browser/chromeos/screensaver/screensaver_controller.cc#newcode66 chrome/browser/chromeos/screensaver/screensaver_controller.cc:66: FindScreensaverExtension(service, current->id())).empty()) { On 2013/02/01 00:35:45, Rahul Chaturvedi ...
2 years, 1 month ago (2013-02-01 04:16:27 UTC) #9
Rahul Chaturvedi
https://codereview.chromium.org/12093058/diff/11019/chrome/browser/chromeos/screensaver/screensaver_controller.cc File chrome/browser/chromeos/screensaver/screensaver_controller.cc (right): https://codereview.chromium.org/12093058/diff/11019/chrome/browser/chromeos/screensaver/screensaver_controller.cc#newcode66 chrome/browser/chromeos/screensaver/screensaver_controller.cc:66: FindScreensaverExtension(service, current->id())).empty()) { On 2013/02/01 04:16:27, stevenjb (chromium) wrote: ...
2 years, 1 month ago (2013-02-01 08:37:39 UTC) #10
I haz the power (commit-bot)
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rkc@chromium.org/12093058/3003
2 years, 1 month ago (2013-02-01 08:37:52 UTC) #11
I haz the power (commit-bot)
Presubmit check for 12093058-3003 failed and returned exit status 1. Running presubmit commit checks ...
2 years, 1 month ago (2013-02-01 08:38:02 UTC) #12
I haz the power (commit-bot)
2 years, 1 month ago (2013-02-01 08:40:46 UTC) #13
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld dd99357-tainted