Summary: | DisplayCaptureSource doesn't initialize base class correctly | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Eric Carlson <eric.carlson> | ||||||
Component: | WebRTC | Assignee: | Eric Carlson <eric.carlson> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | ews-watchlist, glenn, hta, jer.noble, philipj, sergio, tommyw, webkit-bug-importer, youennf | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=235221 | ||||||||
Attachments: |
|
Description
Eric Carlson
2021-12-01 13:28:59 PST
Created attachment 445614 [details]
Patch
Comment on attachment 445614 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=445614&action=review > Source/WebCore/ChangeLog:8 > + No change in functionality. Is the goal to expose a deviceId in settings? If so, we should call setDeviceId in DisplayCaptureSourceCocoa::settings() and add a test. If not, why do we care about having an empty hashed ID? Looking at other browsers, we are doing like Firefox (no deviceId) but not like Chrome. I guess the spec should be clear about that. Also RealtimeMediaSource::hashedId is checking that m_hashedId is not empty if device is not screen/window. I guess we could update this check (though other sources might still have empty hashedId, canvas or WebAudio for instance). > Source/WebCore/platform/mediastream/RealtimeMediaSourceCenter.h:126 > + void getDisplayMediaDevices(const MediaStreamRequest&, String&&, Vector<DeviceInfo>&, String&); Would be worth changing to Expected<Vector<DeviceInfo>, String> getDisplayMediaDevices(..) at some point (probably not this patch though). Created attachment 445777 [details]
Patch
(In reply to youenn fablet from comment #2) > Comment on attachment 445614 [details] > > Is the goal to expose a deviceId in settings? If so, we should call > setDeviceId in DisplayCaptureSourceCocoa::settings() and add a test. > If not, why do we care about having an empty hashed ID? > Good point, I added a test. > Looking at other browsers, we are doing like Firefox (no deviceId) but not > like Chrome. > I guess the spec should be clear about that. > > Also RealtimeMediaSource::hashedId is checking that m_hashedId is not empty > if device is not screen/window. > I guess we could update this check (though other sources might still have > empty hashedId, canvas or WebAudio for instance). > I updated the check. Committed r286491 (244830@main): <https://commits.webkit.org/244830@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 445777 [details]. |