Bug 170693

Summary:

Adopt AVKit name change from AVFunctionBar* to AVTouchBar*

Product:

WebKit

Reporter:

Beth Dakin <bdakin>

Component:

Media

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

aestes, bdakin, jer.noble, mitz, sam, thorton, wenson_hsieh

Priority:

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

See Also:

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

Attachments:

Description	Flags
Patch	none
Patch to try to fix bots	none
Another try for bots	none
Green bots patch?	none
iOS bots patch	none
Another bots patch	none
Patch to only typedef old OS's	sam: review+

Beth Dakin

Reported 2017-04-10 14:10:12 PDT

Adopt AVKit name change from AVFunctionBar* to AVTouchBar* rdar://problem/31230018

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
Show Obsolete (6) View All Add attachment proposed patch, testcase, etc.

Beth Dakin

Comment 1 2017-04-10 14:24:53 PDT

Created attachment 306741 [details] Patch

Tim Horton

Comment 2 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.

Beth Dakin

Comment 3 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.

Beth Dakin

Comment 4 2017-04-10 16:58:38 PDT

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

Beth Dakin

Comment 5 2017-04-10 22:08:18 PDT

Created attachment 306776 [details] Another try for bots

Beth Dakin

Comment 6 2017-04-11 10:35:05 PDT

Created attachment 306831 [details] Green bots patch?

Beth Dakin

Comment 7 2017-04-11 14:45:39 PDT

Created attachment 306854 [details] iOS bots patch

Beth Dakin

Comment 8 2017-04-11 15:35:21 PDT

Created attachment 306867 [details] Another bots patch

Wenson Hsieh

Comment 9 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?

Beth Dakin

Comment 10 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!

Beth Dakin

Comment 11 2017-04-11 21:37:12 PDT

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

Beth Dakin

Comment 12 2017-04-12 11:38:47 PDT

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

mitz

Comment 13 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?

Beth Dakin

Comment 14 2017-04-12 14:22:14 PDT

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

Note You need to log in before you can comment on or make changes to this bug.