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

Issue 12093058: Screensaver implementation for ChromeOS. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year, 2 months ago by Rahul Chaturvedi
Modified:
1 year, 2 months ago
Reviewers:
stevenjb, Matt Perry
CC:
chromium-reviews_chromium.org, 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) Lint Patch
M ash/screensaver/screensaver_view.h View 1 2 3 4 5 6 7 8 3 chunks +4 lines, -0 lines 0 comments ? errors Download
M ash/screensaver/screensaver_view.cc View 1 2 3 4 5 6 7 8 2 chunks +15 lines, -0 lines 0 comments ? errors Download
M ash/screensaver/screensaver_view_unittest.cc View 1 chunk +1 line, -9 lines 0 comments ? errors Download
M chrome/app/generated_resources.grd View 1 chunk +6 lines, -0 lines 0 comments ? errors Download
M chrome/browser/about_flags.cc View 1 2 3 4 5 6 7 1 chunk +7 lines, -0 lines 0 comments 1 errors Download
M chrome/browser/chromeos/chrome_browser_main_chromeos.h View 2 chunks +2 lines, -0 lines 0 comments 0 errors 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 ? errors Download
A chrome/browser/chromeos/screensaver/screensaver_controller.h View 1 2 3 4 5 1 chunk +72 lines, -0 lines 0 comments 1 errors Download
A chrome/browser/chromeos/screensaver/screensaver_controller.cc View 1 2 3 4 5 6 1 chunk +184 lines, -0 lines 0 comments 2 errors Download
A chrome/browser/chromeos/screensaver/screensaver_controller_browsertest.cc View 1 2 3 1 chunk +97 lines, -0 lines 0 comments 3 errors Download
M chrome/chrome_browser_chromeos.gypi View 1 2 3 4 5 6 7 1 chunk +2 lines, -0 lines 0 comments ? errors Download
M chrome/chrome_tests.gypi View 1 2 3 4 5 6 7 1 chunk +1 line, -0 lines 0 comments ? errors 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 ? errors 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 ? errors 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 ? errors 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 ? errors Download
M chromeos/chromeos_switches.h View 1 chunk +1 line, -0 lines 0 comments 0 errors Download
M chromeos/chromeos_switches.cc View 1 chunk +3 lines, -0 lines 0 comments 0 errors Download
M chromeos/dbus/power_manager_client.cc View 1 2 3 4 5 6 7 3 chunks +14 lines, -1 line 0 comments ? errors Download
Commit:

Messages

Total messages: 13
Rahul Chaturvedi
1 year, 2 months ago #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 && ...
1 year, 2 months ago #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: > ...
1 year, 2 months ago #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 ...
1 year, 2 months ago #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 ...
1 year, 2 months ago #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: > ...
1 year, 2 months ago #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()) { ...
1 year, 2 months ago #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: > ...
1 year, 2 months ago #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 ...
1 year, 2 months ago #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: ...
1 year, 2 months ago #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
1 year, 2 months ago #11
I haz the power (commit-bot)
Presubmit check for 12093058-3003 failed and returned exit status 1. Running presubmit commit checks ...
1 year, 2 months ago #12
I haz the power (commit-bot)
1 year, 2 months ago #13
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1280:2d3e6564b7b6