[Webkit-unassigned] [Bug 126818] Add AVKit fullscreen video interface.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=126818





--- Comment #9 from Jon Lee <jonlee at apple.com>  2014-01-13 19:13:14 PST ---
(From update of attachment 221087)
View in context: https://bugs.webkit.org/attachment.cgi?id=221087&action=review

> Source/WebCore/page/Settings.cpp:101
> +bool Settings::gMediaPlaybackFullscreenAVKit = false;

This name could be improved. "gMediaPlayerFullscreenAVKitEnabled"? or just "gAVKitEnabled"? Also, this should propagate across all the settings-related names in this patch.

> Source/WebCore/platform/ios/WebVideoFullscreenControllerAVKit.h:13
> + * THIS SOFTWARE IS PROVIDED BY APPLE COMPUTER, INC. ``AS IS'' AND ANY

This is the wrong license to use. (What's Apple Computer, Inc?)

> Source/WebCore/platform/ios/WebVideoFullscreenControllerAVKit.mm:25
> +

Ditto.

> Source/WebCore/platform/ios/WebVideoFullscreenControllerAVKit.mm:162
> +    return ([self rate] != 0.);

Remove parentheses, please.

> Source/WebCore/platform/ios/WebVideoFullscreenControllerAVKit.mm:183
> +        ExceptionCode exceptionCode {};

ExceptionCode ec = 0;

> Source/WebCore/platform/ios/WebVideoFullscreenControllerAVKit.mm:184
> +        _mediaElement.get()->setCurrentTime(time, exceptionCode);

setCurrentTime() takes one parameter here...

> Source/WebCore/platform/ios/WebVideoFullscreenControllerAVKit.mm:265
> +    Event *event = core(evnt);

Event* event.

> Source/WebCore/platform/ios/WebVideoFullscreenControllerAVKit.mm:280
> +        ];

Small enough to combine into one line without losing legibility.

> Source/WebCore/platform/ios/WebVideoFullscreenControllerAVKit.mm:283
> +            || event->type() == eventNames().ratechangeEvent)

I believe we indent 4 spaces, and not align the conditionals.

> Source/WebCore/platform/ios/WebVideoFullscreenControllerAVKit.mm:305
> +    if ((self = [super init])) {

Would be better to do an early return to get rid of a scope.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.