Bug 94361 – [gtk] add enable-media-stream to websettings

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Danilo de Paula]

[gtk] add enable-media-stream to websettings

[Danilo de Paula]

Created attachment 159160 [details]
Patch

[Danilo de Paula]

Created attachment 159164 [details]
Patch

[Danilo de Paula]

that's webkit gtk

[Martin Robinson]

Comment on attachment 159164 [details]
Patch

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

> Source/WebKit/gtk/webkit/webkitwebsettings.cpp:970
> +    * WebKitWebSettings:enable-media-stream:
> +    *
> +    * Enable or disable support for Media Streaming on pages. Media Streaming is
> +    * an experimental proposal for allowing web pages to access local video and
> +    * audio input devices.  The standard is currently a work-in-progress as part
> +    * of the Web Applications 1.0 specification from WHATWG.
> +    *
> +    * Since: 1.10.0

Why not enable-web-rtc? I think it makes sense to link to the spec here too.

[Martin Robinson]

Comment on attachment 159164 [details]
Patch

It looks like the setting in this patch doesn't actually change anything.

[Danilo de Paula]

(In reply to comment #4)
> (From update of attachment 159164 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=159164&action=review
> 
> > Source/WebKit/gtk/webkit/webkitwebsettings.cpp:970
> > +    * WebKitWebSettings:enable-media-stream:
> > +    *
> > +    * Enable or disable support for Media Streaming on pages. Media Streaming is
> > +    * an experimental proposal for allowing web pages to access local video and
> > +    * audio input devices.  The standard is currently a work-in-progress as part
> > +    * of the Web Applications 1.0 specification from WHATWG.
> > +    *
> > +    * Since: 1.10.0
> 
> Why not enable-web-rtc? I think it makes sense to link to the spec here too.

webRTC contains two separated features: media-stream/getUserMedia and PeerConnection. I believe it makes sense to have two flags, and in this case, media-stream make more sense. Also, chromium is using the same method (http://www.webrtc.org/running-the-demos).

[Danilo de Paula]

(In reply to comment #6)
> (In reply to comment #4)
> > (From update of attachment 159164 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=159164&action=review
> > 
> > > Source/WebKit/gtk/webkit/webkitwebsettings.cpp:970
> > > +    * WebKitWebSettings:enable-media-stream:
> > > +    *
> > > +    * Enable or disable support for Media Streaming on pages. Media Streaming is
> > > +    * an experimental proposal for allowing web pages to access local video and
> > > +    * audio input devices.  The standard is currently a work-in-progress as part
> > > +    * of the Web Applications 1.0 specification from WHATWG.
> > > +    *
> > > +    * Since: 1.10.0
> > 
> > Why not enable-web-rtc? I think it makes sense to link to the spec here too.
> 
> webRTC contains two separated features: media-stream/getUserMedia and PeerConnection. I believe it makes sense to have two flags, and in this case, media-stream make more sense. Also, chromium is using the same method (http://www.webrtc.org/running-the-demos).

Since the PeerConnection API changed a lot on the last few days, I split the features and I'm pushing only MediaStream for now.

[Danilo de Paula]

(In reply to comment #5)
> (From update of attachment 159164 [details])
> It looks like the setting in this patch doesn't actually change anything.

What do you mean?

That's only the flag, doesn't make sense to use it if the rest of the code is not there yet.

[Martin Robinson]

(In reply to comment #8)

> What do you mean?
> 
> That's only the flag, doesn't make sense to use it if the rest of the code is not there yet.

Typically the settings available in WebKitWebSettings control  the Page settings (Settings.h). If there's no WebCore settings to control, then it doesn't make sense to add a WebKit flag, because the flag is a type of dead code.

[Danilo de Paula]

(In reply to comment #9)
> Typically the settings available in WebKitWebSettings control  the Page settings (Settings.h). If there's no WebCore settings to control, then it doesn't make sense to add a WebKit flag, because the flag is a type of dead code.

I'm using it to control RuntimeEnabledFeatures.h

WebCore::RuntimeEnabledFeatures::setMediaStreamEnabled(settingsPrivate->enableMediaStream);

I can use WebKitWebSettings for that, can't I?

[Danilo de Paula]

Created attachment 159186 [details]
Patch

[Martin Robinson]

(In reply to comment #11)
> Created an attachment (id=159186) [details]
> Patch

No link to the spec? Chromium calls this MediaStream in their UI, but in this patch you refer to it as media-stream and Media Streaming. What's the real name? I think it makes sense in the documentation to also describe how this relates to WebRTC.

[Philippe Normand]

Do you plan the WK2 equivalent of this patch? Should be done in a separate bugzilla entry.

[Danilo de Paula]

Created attachment 159401 [details]
Patch

[Danilo de Paula]

(In reply to comment #12)
> (In reply to comment #11)
> > Created an attachment (id=159186) [details] [details]
> > Patch
> 
> No link to the spec? Chromium calls this MediaStream in their UI, but in this patch you refer to it as media-stream and Media Streaming. What's the real name? I think it makes sense in the documentation to also describe how this relates to WebRTC.

Put the link on code.
The name is Media Stream. And I will stick to that name only, so fixed.

(In reply to comment #13)
> Do you plan the WK2 equivalent of this patch? Should be done in a separate bugzilla entry.

In the near future, yes. But the plan is to have a usable/stable version for webkit 1 frist.

[Martin Robinson]

Comment on attachment 159401 [details]
Patch

Thanks!

[Danilo de Paula]

Comment on attachment 159401 [details]
Patch

Can you grant me commit after the stable release?

[Philippe Normand]

Comment on attachment 159401 [details]
Patch

cq=me since the stable release branched a while ago

[WebKit Review Bot]

Comment on attachment 159401 [details]
Patch

Clearing flags on attachment: 159401

Committed r129000: <http://trac.webkit.org/changeset/129000>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.