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

Issue 12093058: Screensaver implementation for ChromeOS. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 years, 6 months ago by Rahul Chaturvedi
Modified:
2 years, 6 months 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: CQ not working?

Messages

Total messages: 13 (0 generated)
Rahul Chaturvedi
2 years, 6 months 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, 6 months 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, 6 months 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, 6 months 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, 6 months 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, 6 months 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, 6 months 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, 6 months 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, 6 months 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, 6 months ago (2013-02-01 08:37:39 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rkc@chromium.org/12093058/3003
2 years, 6 months ago (2013-02-01 08:37:52 UTC) #11
commit-bot: I haz the power
Presubmit check for 12093058-3003 failed and returned exit status 1. Running presubmit commit checks ...
2 years, 6 months ago (2013-02-01 08:38:02 UTC) #12
commit-bot: I haz the power
2 years, 6 months 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 5fa3ca5