Bug 40939 – Full-screened content doesn't keep the display on: Safari not grabbing a power assertion?

• Component: Media
• Status: RESOLVED FIXED

[Jer Noble]

Safari doesn't grab a power assertion from IOKit when doing full-screen video content.

See http://developer.apple.com/mac/library/releasenotes/Darwin/RN-IOKitPowerManagment/#//apple_ref/doc/uid/TP40006501-CH1-DontLinkElementID_4

[Jer Noble]

rdar://problem/7996172

[Jer Noble]

Created attachment 59285 [details]
Patch

[WebKit Review Bot]

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.

[Eric Seidel (no email)]

Attachment 59285 [details] did not build on mac:
Build output: http://webkit-commit-queue.appspot.com/results/3303555

[Jer Noble]

Created attachment 59294 [details]
Patch

[WebKit Review Bot]

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.

[Eric Seidel (no email)]

Attachment 59294 [details] did not build on mac:
Build output: http://webkit-commit-queue.appspot.com/results/3341571

[Jer Noble]

Created attachment 59315 [details]
Patch

[WebKit Review Bot]

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.

[Jer Noble]

Created attachment 59686 [details]
Patch

[WebKit Review Bot]

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.

[Eric Carlson]

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.

[Jer Noble]

Committed r61796: <http://trac.webkit.org/changeset/61796>

[Darin Adler]

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.

[Jer Noble]

(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.