Bug 181070

Summary: [MediaStream] Add screen capture IDL and stub functions
Product: WebKit Reporter: Eric Carlson <eric.carlson>
Component: WebRTCAssignee: Eric Carlson <eric.carlson>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, fred.wang, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Description Eric Carlson 2017-12-20 17:28:53 PST
Add media stream screen capture IDL and stub functions
Comment 1 Radar WebKit Bug Importer 2017-12-20 17:31:00 PST
<rdar://problem/36168724>
Comment 2 Eric Carlson 2017-12-20 17:42:57 PST
Created attachment 329979 [details]
Patch
Comment 3 youenn fablet 2017-12-20 17:54:50 PST
Comment on attachment 329979 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=329979&action=review

> Source/WebCore/page/RuntimeEnabledFeatures.h:270
> +    bool m_isScreenCaptureEnabled {false };

s/{false/{ false/

> Source/WebCore/platform/mediastream/RealtimeMediaSourceCenter.cpp:323
> +    case CaptureDevice::DeviceType::Browser:

I was expecting one ScreenCapture DeviceType.
What is the benefit of bringing these in finer grained types?

> LayoutTests/fast/mediastream/screencapture-disabled-expected.txt:1
> +PASS undefined is undefined.

strange message but I guess it is fine.
Comment 4 Eric Carlson 2017-12-20 18:04:05 PST
Comment on attachment 329979 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=329979&action=review

>> Source/WebCore/page/RuntimeEnabledFeatures.h:270
>> +    bool m_isScreenCaptureEnabled {false };
> 
> s/{false/{ false/

Oops, will fix.

>> Source/WebCore/platform/mediastream/RealtimeMediaSourceCenter.cpp:323
>> +    case CaptureDevice::DeviceType::Browser:
> 
> I was expecting one ScreenCapture DeviceType.
> What is the benefit of bringing these in finer grained types?

A port may not support all types of screen capture, so WebKit will need to signal the UA so it can prompt the user with only the supported types. Further, once the user chooses a screen, application, window, or tab to share, the UA will have to pass the ID of the "display surface" as well as the type.
Comment 5 Eric Carlson 2017-12-20 18:12:50 PST
Created attachment 329984 [details]
Patch
Comment 6 WebKit Commit Bot 2017-12-20 18:45:40 PST
Comment on attachment 329984 [details]
Patch

Clearing flags on attachment: 329984

Committed r226211: <https://trac.webkit.org/changeset/226211>
Comment 7 WebKit Commit Bot 2017-12-20 18:45:41 PST
All reviewed patches have been landed.  Closing bug.
Comment 8 Frédéric Wang (:fredw) 2018-01-08 08:15:35 PST
Comment on attachment 329984 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=329984&action=review

> Source/WebKit/UIProcess/API/Cocoa/WKPreferencesPrivate.h:107
> +@property (nonatomic, setter=_setScreenCaptureEnabled:) BOOL _screenCaptureEnabled WK_API_AVAILABLE(macosx(WK_MAC_TBA), ios(WK_MAC_TBA));

For the record, this should have been ios(WK_IOS_TBA).

I landed

https://trac.webkit.org/changeset/226513

to fix the iOS build with the public SDK.
Comment 9 Eric Carlson 2018-01-08 10:30:53 PST
(In reply to Frédéric Wang (:fredw) from comment #8)
> Comment on attachment 329984 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=329984&action=review
> 
> > Source/WebKit/UIProcess/API/Cocoa/WKPreferencesPrivate.h:107
> > +@property (nonatomic, setter=_setScreenCaptureEnabled:) BOOL _screenCaptureEnabled WK_API_AVAILABLE(macosx(WK_MAC_TBA), ios(WK_MAC_TBA));
> 
> For the record, this should have been ios(WK_IOS_TBA).
> 
> I landed
> 
> https://trac.webkit.org/changeset/226513
> 
> to fix the iOS build with the public SDK.

Thanks Frédéric!