Summary: | Full-screened content doesn't keep the display on: Safari not grabbing a power assertion? | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jer Noble <jer.noble> | ||||||||||
Component: | Media | Assignee: | Jer Noble <jer.noble> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | eric, webkit.review.bot | ||||||||||
Priority: | P2 | Keywords: | InRadar, PlatformOnly | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | PC | ||||||||||||
OS: | OS X 10.5 | ||||||||||||
Attachments: |
|
Description
Jer Noble
2010-06-21 13:43:56 PDT
Created attachment 59285 [details]
Patch
Attachment 59285 [details] did not pass style-queue:
Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
WebKit/mac/WebView/WebVideoFullscreenController.h:51: _idleDisplaySleepAssertion is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
WebKit/mac/WebView/WebVideoFullscreenController.h:52: _idleSystemSleepAssertion is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
Total errors found: 2 in 7 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Attachment 59285 [details] did not build on mac: Build output: http://webkit-commit-queue.appspot.com/results/3303555 Created attachment 59294 [details]
Patch
Attachment 59294 [details] did not pass style-queue:
Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
WebKit/mac/WebView/WebVideoFullscreenController.h:51: _idleDisplaySleepAssertion is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
WebKit/mac/WebView/WebVideoFullscreenController.h:52: _idleSystemSleepAssertion is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
Total errors found: 2 in 7 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Attachment 59294 [details] did not build on mac: Build output: http://webkit-commit-queue.appspot.com/results/3341571 Created attachment 59315 [details]
Patch
Attachment 59315 [details] did not pass style-queue:
Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
WebKit/mac/WebView/WebVideoFullscreenController.h:51: _idleDisplaySleepAssertion is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
WebKit/mac/WebView/WebVideoFullscreenController.h:52: _idleSystemSleepAssertion is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
Total errors found: 2 in 7 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 59686 [details]
Patch
Attachment 59686 [details] did not pass style-queue:
Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
WebKit/mac/WebView/WebVideoFullscreenController.h:51: _idleDisplaySleepAssertion is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
WebKit/mac/WebView/WebVideoFullscreenController.h:52: _idleSystemSleepAssertion is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
Total errors found: 2 in 7 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 59686 [details]
Patch
WebKit/WebKit.xcodeproj/project.pbxproj:777
+ CD8EAC7211CAC9A300774075 /* IOKit.framework */,
Should probably ask mrowe if it worth linking with the a Framework or if you should load it dynamically.
WebKit/mac/WebView/WebVideoFullscreenController.mm:235
+ if (_mediaElement && _mediaElement->platformMedia().type == WebCore::PlatformMedia::QTMovieType)
+ rate = [_mediaElement->platformMedia().media.qtMovie rate];
It might be worth having an accessor function for the QTMovie since "[_mediaElement->platformMedia().media.qtMovie" is used in the file in several places now
WebKit/mac/WebView/WebVideoFullscreenController.mm:238
+ if (rate && !_isEndingFullscreen) {
+ [self _disableIdleSystemSleep];
+ [self _disableIdleDisplaySleep];
+ } else {
+ [self _enableIdleSystemSleep];
+ [self _enableIdleDisplaySleep];
+ }
Is there any reason to have separate functions for system and display sleep? You always enabled or disabled so you may want to have just one function to disable and one to enable.
Committed r61796: <http://trac.webkit.org/changeset/61796> Comment on attachment 59686 [details]
Patch
This should not statically link to the IOKit framework. We should soft link instead. Unless someone more expert than I on the Safari team said otherwise.
(In reply to comment #14) > (From update of attachment 59686 [details]) > This should not statically link to the IOKit framework. We should soft link instead. Unless someone more expert than I on the Safari team said otherwise. I talked this over with Eric and Mark before checking in; AppKit.framework statically links against IOKit.framework, and WebKit.framework statically links against AppKit.framework. So, it should be safe to link to this statically without affecting startup time, since we effectively already statically link against IOKit.framework. |