Summary: | Use ObjectIdentifier<>instead of WebCore::nextPlaybackTargetClientContextId() in Document.cpp | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | David Kilzer (:ddkilzer) <ddkilzer> | ||||||
Component: | WebCore Misc. | Assignee: | David Kilzer (:ddkilzer) <ddkilzer> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | annulen, cdumez, eric.carlson, esprehn+autocc, ews-watchlist, glenn, gyuyoung.kim, jer.noble, kangil.han, peng.liu6, philipj, ryuan.choi, sergio, useafterfree, webkit-bug-importer, youennf | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | Other | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
David Kilzer (:ddkilzer)
2020-06-23 21:50:05 PDT
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]. |