Bug 170693 – Adopt AVKit name change from AVFunctionBar* to AVTouchBar*

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

[Beth Dakin]

Adopt AVKit name change from AVFunctionBar* to AVTouchBar*

rdar://problem/31230018

[Beth Dakin]

Created attachment 306741 [details]
Patch

[Tim Horton]

Comment on attachment 306741 [details]
Patch

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

> Source/WebCore/platform/spi/cocoa/AVKitSPI.h:161
> +typedef AVTouchBarMediaSelectionOption WKAVTouchBarMediaSelectionOption;

Though I think it was my suggestion to do the WK-, now I wonder if it would be better to just typedef to the new names on the old OS, and use the new names everywhere? We know they won't magically appear in the old SDK.

[Beth Dakin]

(In reply to Tim Horton from comment #2)
> Comment on attachment 306741 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=306741&action=review
> 
> > Source/WebCore/platform/spi/cocoa/AVKitSPI.h:161
> > +typedef AVTouchBarMediaSelectionOption WKAVTouchBarMediaSelectionOption;
> 
> Though I think it was my suggestion to do the WK-, now I wonder if it would
> be better to just typedef to the new names on the old OS, and use the new
> names everywhere? We know they won't magically appear in the old SDK.

Oh yeah, that's a pretty good idea. I'm going to attach a new patch that doesn't do that yet just to see if I can get the bots green.

[Beth Dakin]

Created attachment 306758 [details]
Patch to try to fix bots

[Beth Dakin]

Created attachment 306776 [details]
Another try for bots

[Beth Dakin]

Created attachment 306831 [details]
Green bots patch?

[Beth Dakin]

Created attachment 306854 [details]
iOS bots patch

[Beth Dakin]

Created attachment 306867 [details]
Another bots patch

[Wenson Hsieh]

Comment on attachment 306867 [details]
Another bots patch

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

> Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:5008
> +- (id)_mediaPlaybackControlsView

Can these be something a bit more specific, like NSView?

[Beth Dakin]

(In reply to Wenson Hsieh from comment #9)
> Comment on attachment 306867 [details]
> Another bots patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=306867&action=review
> 
> > Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:5008
> > +- (id)_mediaPlaybackControlsView
> 
> Can these be something a bit more specific, like NSView?

Yeah, the id is not great. I think this can be fixed if I adopt Tim's suggestion of only typedef'ing on the old OS, and using the name from the new OS. That way there is only one name to forward declare, which is what we used to do.

Patch forthcoming!

[Beth Dakin]

Created attachment 306887 [details]
Patch to only typedef old OS's

[Beth Dakin]

Thanks Sam! https://trac.webkit.org/changeset/215274/webkit

Comment on attachment 306887 [details]
Patch to only typedef old OS's

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

> Source/WebCore/platform/spi/cocoa/AVKitSPI.h:156
> +#if __MAC_OS_X_VERSION_MIN_REQUIRED < 101300

Shouldn’t this check the SDK version rather than the deployment version?

[Beth Dakin]

Just a few build fixes:

https://trac.webkit.org/changeset/215274/webkit
https://trac.webkit.org/changeset/215275/webkit
https://trac.webkit.org/changeset/215277/webkit
https://trac.webkit.org/changeset/215279/webkit
https://trac.webkit.org/changeset/215281/webkit
https://trac.webkit.org/changeset/215282/webkit
https://trac.webkit.org/changeset/215283/webkit
https://trac.webkit.org/changeset/215284/webkit
https://trac.webkit.org/changeset/215285/webkit