Minor cleanup.
<rdar://problem/46465458>
Created attachment 356532 [details] Patch
Comment on attachment 356532 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=356532&action=review > Source/WebCore/ChangeLog:12 > + (WebCore::DisplayCaptureManagerCocoa::captureDevices): Initialize Display devices first. Maybe document why we need to do this > Source/WebCore/platform/mediastream/mac/ScreenDisplayCaptureSourceMac.mm:156 > weakThis->frameAvailable(status, displayTime, frameSurface, updateRef); This is existing code but the weakThis check is not thread safe here. weakThis may be dead between if and next call. > Source/WebCore/platform/mediastream/mac/ScreenDisplayCaptureSourceMac.mm:-167 > - }); Do we need the call to frameAvailable to be in a background thread?
Created attachment 356632 [details] Patch for landing.
Comment on attachment 356632 [details] Patch for landing. Clearing flags on attachment: 356632 Committed r238904: <https://trac.webkit.org/changeset/238904>
All reviewed patches have been landed. Closing bug.
Comment on attachment 356632 [details] Patch for landing. View in context: https://bugs.webkit.org/attachment.cgi?id=356632&action=review > Source/WebCore/platform/mediastream/mac/ScreenDisplayCaptureSourceMac.mm:144 > + CFDictionaryRef streamOptions = (__bridge CFDictionaryRef) @{ Doing the cast here may be incompatible with ARC. Instead the local variable should be an NSDictionary * and the cast to CFDictionaryRef should go below at the call site. Or we can use two local variables. > Source/WebCore/platform/mediastream/mac/ScreenDisplayCaptureSourceMac.mm:148 > + (__bridge NSString *)kCGDisplayStreamMinimumFrameTime :@(1 / frameRate()), > + (__bridge NSString *)kCGDisplayStreamQueueDepth:@(screenQueueMaximumLength), > + (__bridge NSString *)kCGDisplayStreamColorSpace:(__bridge id)sRGBColorSpaceRef(), > + (__bridge NSString *)kCGDisplayStreamShowCursor : @(YES), Formatting here is a little messy, inconsistent about whether there is a space before and after the colon.
(In reply to Darin Adler from comment #7) > Comment on attachment 356632 [details] > Patch for landing. > > View in context: > https://bugs.webkit.org/attachment.cgi?id=356632&action=review > > > Source/WebCore/platform/mediastream/mac/ScreenDisplayCaptureSourceMac.mm:144 > > + CFDictionaryRef streamOptions = (__bridge CFDictionaryRef) @{ > > Doing the cast here may be incompatible with ARC. Instead the local variable > should be an NSDictionary * and the cast to CFDictionaryRef should go below > at the call site. Or we can use two local variables. > > > Source/WebCore/platform/mediastream/mac/ScreenDisplayCaptureSourceMac.mm:148 > > + (__bridge NSString *)kCGDisplayStreamMinimumFrameTime :@(1 / frameRate()), > > + (__bridge NSString *)kCGDisplayStreamQueueDepth:@(screenQueueMaximumLength), > > + (__bridge NSString *)kCGDisplayStreamColorSpace:(__bridge id)sRGBColorSpaceRef(), > > + (__bridge NSString *)kCGDisplayStreamShowCursor : @(YES), > > Formatting here is a little messy, inconsistent about whether there is a > space before and after the colon. Thanks, I will fix these in bug 192514.