[webkit-reviews] review denied: [Bug 47299] Screensaver starts while watching fullscreen playback : [Attachment 69984] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Jer Noble <jer.noble at apple.com>'s
request for review:
Bug 47299: Screensaver starts while watching fullscreen playback
https://bugs.webkit.org/show_bug.cgi?id=47299

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

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=69984&action=review

> WebKit/mac/WebView/WebVideoFullscreenController.h:53
> +    NSTimer* _tickleTimer;

Our style is to put spaces before the "*" for Objective-C types like this.
Maybe we should change that style, but we have not yet done that.

> WebKit/mac/WebView/WebVideoFullscreenController.mm:46
> +static NSTimeInterval kTickleTimerInterval = 1.0;

This should be static const, not just static. Also, we don’t use “k” for such
things in WebKit.

> WebKit/mac/WebView/WebVideoFullscreenController.mm:86
> +    [_tickleTimer invalidate];

This needs to release the timer.

> WebKit/mac/WebView/WebVideoFullscreenController.mm:387
> +    [_tickleTimer invalidate];
> +    _tickleTimer = [NSTimer
scheduledTimerWithTimeInterval:kTickleTimerInterval target:self
selector:@selector(_tickleTimerFired) userInfo:nil repeats:YES];

This needs to release the old timer and retain the new timer.

> WebKit/mac/WebView/WebVideoFullscreenController.mm:393
> +    [_tickleTimer invalidate];
> +    _tickleTimer = nil;

This needs to release the timer.