Bug 128558 – WK2 AVKit enter fullscreen doesn't work a second time.

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Jeremy Jones]

WebVideoFullscreenInterfaceAVKit's m_playerController get's cleared on exitfullscreen() and never recreated.

[Jeremy Jones]

Created attachment 223757 [details]
Patch

[Jer Noble]

Comment on attachment 223757 [details]
Patch

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

> Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.h:57
> +    WebAVPlayerController *getPlayerController();

This should just be "playerController()", or perhaps "ensurePlayerController()" due to the lazy-creation.

> Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:225
> +    if (m_videoFullscreenModel)
> +        m_playerController.get().delegate = m_videoFullscreenModel;

shouldn't this be in the "if (!m_playerController)" case?

[Jeremy Jones]

(In reply to comment #2)
> (From update of attachment 223757 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=223757&action=review
> 
> > Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.h:57
> > +    WebAVPlayerController *getPlayerController();
> 
> This should just be "playerController()", or perhaps "ensurePlayerController()" due to the lazy-creation.
> 
> > Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:225
> > +    if (m_videoFullscreenModel)
> > +        m_playerController.get().delegate = m_videoFullscreenModel;
> 
> shouldn't this be in the "if (!m_playerController)" case?

Good point. I'll fix that.

[Jeremy Jones]

Created attachment 223759 [details]
Patch

[Jeremy Jones]

(In reply to comment #2)
> (From update of attachment 223757 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=223757&action=review
> 
> > Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.h:57
> > +    WebAVPlayerController *getPlayerController();
> 
> This should just be "playerController()", or perhaps "ensurePlayerController()" due to the lazy-creation.

I thought style guidelines require getters to begin with "get".

[Jeremy Jones]

(In reply to comment #5)
> (In reply to comment #2)
> > (From update of attachment 223757 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=223757&action=review
> > 
> > > Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.h:57
> > > +    WebAVPlayerController *getPlayerController();
> > 
> > This should just be "playerController()", or perhaps "ensurePlayerController()" due to the lazy-creation.
> 
> I thought style guidelines require getters to begin with "get".

Sorry, I'm wrong. I actually originally had playerController(). I'll switch back to that.

[Jeremy Jones]

Created attachment 223761 [details]
Patch

[Jer Noble]

Comment on attachment 223761 [details]
Patch

r=me.

[WebKit Commit Bot]

Comment on attachment 223761 [details]
Patch

Clearing flags on attachment: 223761

Committed r164161: <http://trac.webkit.org/changeset/164161>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.