Bug 213546

Created attachment 402621 [details] Patch v1

Comment on attachment 402621 [details] Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=402621&action=review > Source/WebCore/ChangeLog:10 > + Switch from uint64_t to WebCore::MediaSessionContextIdentifier > + for contextId values. Would a better name be WebCore::PlaybackTargetClientContextIdentifier? Also, there were a few uses of parameters named `clientId` that I changed to `contextId` since the majority of the names were already `contextId`. If there is a strong desire to change them back to `clientId`, let me know.

Comment on attachment 402621 [details] Patch v1 LGTM, modulo naming that would be good to get feedback from Eric or Jer. View in context: https://bugs.webkit.org/attachment.cgi?id=402621&action=review >> Source/WebCore/ChangeLog:10 >> + for contextId values. > > Would a better name be WebCore::PlaybackTargetClientContextIdentifier? > > Also, there were a few uses of parameters named `clientId` that I changed to `contextId` since the majority of the names were already `contextId`. If there is a strong desire to change them back to `clientId`, let me know. PlaybackTargetClientContextIdentifier seems to align better with existing usage. MediaSessionContextIdentifier makes me think of identifying MediaSession or PlatformMediaSessionClient. > Source/WebCore/dom/Document.cpp:7402 > +void Document::playbackTargetAvailabilityDidChange(MediaSessionContextIdentifier contentId, bool available) s/content/context/ Here and below > Source/WebCore/dom/Document.h:1953 > + typedef HashMap<MediaSessionContextIdentifier, WebCore::MediaPlaybackTargetClient*> TargetIdToClientMap; s/typedef/using s/TargetIdToClientMap/ContextIdToClientMap > Source/WebKit/UIProcess/WebPageProxy.cpp:9147 > +void WebPageProxy::addPlaybackTargetPickerClient(WebCore::MediaSessionContextIdentifier contextId) s/WebCore::// here and below > Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp:1259 > +void WebChromeClient::addPlaybackTargetPickerClient(WebCore::MediaSessionContextIdentifier contextId) s/WebCore::// > Source/WebKit/WebProcess/WebPage/mac/WebPageMac.mm:1011 > +void WebPage::playbackTargetSelected(WebCore::MediaSessionContextIdentifier contextId, const WebCore::MediaPlaybackTargetContext& targetContext) const s/WebCore:://

Created attachment 402683 [details] Patch for landing

Comment on attachment 402683 [details] Patch for landing Marking cq+ as all bubbles are green.

Committed r263498: <https://trac.webkit.org/changeset/263498> All reviewed patches have been landed. Closing bug and clearing flags on attachment 402683 [details].