Bug 68421 - Stop calling UpdateSystemActivity in places where we hold power assertions that achieve the same effect
Summary: Stop calling UpdateSystemActivity in places where we hold power assertions th...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Rowe (bdash)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-09-19 20:17 PDT by Mark Rowe (bdash)
Modified: 2011-09-19 21:14 PDT (History)
5 users (show)

See Also:


Attachments
Patch v1 (37.33 KB, patch)
2011-09-19 20:34 PDT, Mark Rowe (bdash)
mitz: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Rowe (bdash) 2011-09-19 20:17:24 PDT
In a couple of locations related to fullscreen media playback we periodically call UpdateSystemActivity.  There’s no need to do this on SnowLeopard and newer as the power assertion that we hold to prevent display sleep also prevents the screensaver kicking in.  It doesn’t prevent the screensaver on Leopard though, so we’ll still need to call UpdateSystemActivity in that case.
Comment 1 Mark Rowe (bdash) 2011-09-19 20:34:21 PDT
Created attachment 107961 [details]
Patch v1
Comment 2 WebKit Review Bot 2011-09-19 20:37:59 PDT
Attachment 107961 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1

Source/WebCore/platform/mac/WebVideoFullscreenController.h:34:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/WebCore/platform/mac/DisplaySleepDisabler.cpp:27:  You should add a blank line after implementation file's own header.  [build/include_order] [4]
Source/WebKit/mac/WebView/WebFullScreenController.h:34:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 3 in 13 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Mark Rowe (bdash) 2011-09-19 20:38:12 PDT
Comment on attachment 107961 [details]
Patch v1

View in context: https://bugs.webkit.org/attachment.cgi?id=107961&action=review

> Source/WebCore/platform/mac/DisplaySleepDisabler.cpp:56
> +void DisplaySleepDisabler::systemActivityTimerFired(Timer<DisplaySleepDisabler>* timer)

I nuked the unused variable name from here.
Comment 4 mitz 2011-09-19 20:46:46 PDT
Comment on attachment 107961 [details]
Patch v1

View in context: https://bugs.webkit.org/attachment.cgi?id=107961&action=review

> Source/WebCore/ChangeLog:27
> +        (-[WebVideoFullscreenController setupVideoOverlay:]): Remove the now-unnecessary explicit use of the WebCore namespace..

..

> Source/WebCore/platform/mac/DisplaySleepDisabler.cpp:27
> +#include "DisplaySleepDisabler.h"

Missing blank line after this.

> Source/WebCore/platform/mac/DisplaySleepDisabler.cpp:42
> +    RetainPtr<CFStringRef> reasonCF(AdoptCF, CFStringCreateWithCString(0, reason, kCFStringEncodingUTF8));

I prefer kCFAllocatorDefault

> Source/WebCore/platform/mac/DisplaySleepDisabler.cpp:44
> +#else

Does this branch need an UNUSED_PARAM(reason)?

> Source/WebCore/platform/mac/DisplaySleepDisabler.cpp:56
> +void DisplaySleepDisabler::systemActivityTimerFired(Timer<DisplaySleepDisabler>* timer)

No need to name the unused parameter here.
Comment 5 Mark Rowe (bdash) 2011-09-19 21:14:05 PDT
Landed in r95513.