[webkit-reviews] review granted: [Bug 40939] Full-screened content doesn't keep the display on: Safari not grabbing a power assertion? : [Attachment 59686] Patch

[bugzilla-daemon at webkit.org]

Eric Carlson <eric.carlson at apple.com> has granted Jer Noble
<jer.noble at apple.com>'s request for review:
Bug 40939: Full-screened content doesn't keep the display on: Safari not
grabbing a power assertion?
https://bugs.webkit.org/show_bug.cgi?id=40939

Attachment 59686: Patch
https://bugs.webkit.org/attachment.cgi?id=59686&action=review

------- Additional Comments from Eric Carlson <eric.carlson at apple.com>

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.