Bug 170693 - Adopt AVKit name change from AVFunctionBar* to AVTouchBar*
Summary: Adopt AVKit name change from AVFunctionBar* to AVTouchBar*
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-04-10 14:10 PDT by Beth Dakin
Modified: 2017-04-12 14:22 PDT (History)
7 users (show)

See Also:


Attachments
Patch (27.50 KB, patch)
2017-04-10 14:24 PDT, Beth Dakin
no flags Details | Formatted Diff | Diff
Patch to try to fix bots (27.69 KB, patch)
2017-04-10 16:58 PDT, Beth Dakin
no flags Details | Formatted Diff | Diff
Another try for bots (27.65 KB, patch)
2017-04-10 22:08 PDT, Beth Dakin
no flags Details | Formatted Diff | Diff
Green bots patch? (27.61 KB, patch)
2017-04-11 10:35 PDT, Beth Dakin
no flags Details | Formatted Diff | Diff
iOS bots patch (27.47 KB, patch)
2017-04-11 14:45 PDT, Beth Dakin
no flags Details | Formatted Diff | Diff
Another bots patch (27.34 KB, patch)
2017-04-11 15:35 PDT, Beth Dakin
no flags Details | Formatted Diff | Diff
Patch to only typedef old OS's (26.12 KB, patch)
2017-04-11 21:37 PDT, Beth Dakin
sam: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Beth Dakin 2017-04-10 14:10:12 PDT
Adopt AVKit name change from AVFunctionBar* to AVTouchBar*

rdar://problem/31230018
Comment 1 Beth Dakin 2017-04-10 14:24:53 PDT
Created attachment 306741 [details]
Patch
Comment 2 Tim Horton 2017-04-10 14:35:53 PDT
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.
Comment 3 Beth Dakin 2017-04-10 16:58:18 PDT
(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.
Comment 4 Beth Dakin 2017-04-10 16:58:38 PDT
Created attachment 306758 [details]
Patch to try to fix bots
Comment 5 Beth Dakin 2017-04-10 22:08:18 PDT
Created attachment 306776 [details]
Another try for bots
Comment 6 Beth Dakin 2017-04-11 10:35:05 PDT
Created attachment 306831 [details]
Green bots patch?
Comment 7 Beth Dakin 2017-04-11 14:45:39 PDT
Created attachment 306854 [details]
iOS bots patch
Comment 8 Beth Dakin 2017-04-11 15:35:21 PDT
Created attachment 306867 [details]
Another bots patch
Comment 9 Wenson Hsieh 2017-04-11 16:46:38 PDT
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?
Comment 10 Beth Dakin 2017-04-11 21:25:08 PDT
(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!
Comment 11 Beth Dakin 2017-04-11 21:37:12 PDT
Created attachment 306887 [details]
Patch to only typedef old OS's
Comment 12 Beth Dakin 2017-04-12 11:38:47 PDT
Thanks Sam! https://trac.webkit.org/changeset/215274/webkit
Comment 13 mitz 2017-04-12 11:47:56 PDT
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?