Bug 56586 – Media Stream API: add the getUserMedia method and the Javascript bindings

• Component: DOM
• Status: RESOLVED FIXED

[Leandro Graciá Gil]

This patch adds the getUserMedia method to the navigator with all its intermediate Javascript types and the basic skeleton of the stream controller and the client and listener interfaces.

[Leandro Graciá Gil]

Created attachment 86088 [details]
Patch

[Build Bot]

Attachment 86088 [details] did not build on win:
Build output: http://queues.webkit.org/results/8191773

[Leandro Graciá Gil]

Created attachment 86181 [details]
Patch

[Leandro Graciá Gil]

Adding the webkit prefix to the new introduced methods (getUserMedia). Fixing the build files of other WebKit ports.

[Build Bot]

Attachment 86181 [details] did not build on win:
Build output: http://queues.webkit.org/results/8205242

[Leandro Graciá Gil]

Created attachment 86198 [details]
Patch

[Leandro Graciá Gil]

Fixing windows build.

[Eric Seidel (no email)]

Comment on attachment 86198 [details]
Patch

What spec covers this?  Please update your ChangeLog to explain.

Also, please use webkit-patch or at least svn-create-patch to create your diffs so the ChangeLogs end up at the top. :)

[Peter Beverloo]

The WHATWG HTML version does. This was recently changed by Hixie.

http://www.whatwg.org/specs/web-apps/current-work/multipage/dnd.html#video-conferencing-and-peer-to-peer-communication

[Steve Block]

Comment on attachment 86198 [details]
Patch

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

> Source/WebCore/page/NavigatorUserMediaError.idl:33
> +    // FIXME: delete this interface and implement a custom binding since spec defines it as a NoInterfaceObject.

Any reason not to do this now?

> Source/WebCore/page/NavigatorUserMediaErrorCallback.cpp:46
> +// NavigatorUserMediaSuccessCallback and BlobCallback.

Again, why not do this now?

> Source/WebCore/page/Page.cpp:858
> +        m_streamController.set(new StreamController(m_streamClient, mainFrame()->document()));

Do you plan to add a runtime flag? Other controller/client features only instantiate the controller when the runtime flag is set, as the controller is otherwsise not used. Make sure that the client is still signalled to shut down, if required, in this case though.

Also, why doe we use the main frame's document? How does this work in the case of iframes?

> Source/WebCore/page/Page.h:344
> +        OwnPtr<StreamController> m_streamController;

I understand that the spec refers to this as just 'Stream', but the name seems a little vague. Could you use a more descriptive name, or at least add a comment about what kind of stream it is?

> Source/WebCore/page/StreamListener.h:41
> +// be implemented by both the StreamController and the StreamClientMock.

I don't understand this. Can you explain why the mock client would implement this interface?

[Leandro Graciá Gil]

Created attachment 86487 [details]
Patch

[Leandro Graciá Gil]

Comment on attachment 86198 [details]
Patch

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

>> Source/WebCore/page/NavigatorUserMediaError.idl:33
>> +    // FIXME: delete this interface and implement a custom binding since spec defines it as a NoInterfaceObject.
> 
> Any reason not to do this now?

Fixed for all callback objects. Since the spec is still changing and being discussed we're delaying the removal of the idl file for NavigatorUserMediaError.

>> Source/WebCore/page/NavigatorUserMediaErrorCallback.cpp:46
>> +// NavigatorUserMediaSuccessCallback and BlobCallback.
> 
> Again, why not do this now?

Done. Check CallbackTask.h file in the new patch.

>> Source/WebCore/page/Page.cpp:858
>> +        m_streamController.set(new StreamController(m_streamClient, mainFrame()->document()));
> 
> Do you plan to add a runtime flag? Other controller/client features only instantiate the controller when the runtime flag is set, as the controller is otherwsise not used. Make sure that the client is still signalled to shut down, if required, in this case though.
> 
> Also, why doe we use the main frame's document? How does this work in the case of iframes?

Yes, we plan to add a runtime flag in chromium. The new patch also introduces the EnabledAtRuntime flag to the corresponding idl files.

Fixed the problems with the frame, the security origins and replying using the same execution context that made the call.

>> Source/WebCore/page/Page.h:344
>> +        OwnPtr<StreamController> m_streamController;
> 
> I understand that the spec refers to this as just 'Stream', but the name seems a little vague. Could you use a more descriptive name, or at least add a comment about what kind of stream it is?

This is not a Stream but the class controlling them and acting as an interface to the client. Streams are introduced in bug 56666. In any case, I've added a comment in the StreamController.h and Stream.h (next patch) files to make clear that this file is about the Media Stream API.

>> Source/WebCore/page/StreamListener.h:41
>> +// be implemented by both the StreamController and the StreamClientMock.
> 
> I don't understand this. Can you explain why the mock client would implement this interface?

Fixed. StreamListener removed, as the StreamController is now used directly.

[Leandro Graciá Gil]

(In reply to comment #8)
> (From update of attachment 86198 [details])
> What spec covers this?  Please update your ChangeLog to explain.
> 
> Also, please use webkit-patch or at least svn-create-patch to create your diffs so the ChangeLogs end up at the top. :)

Fixed.

[Leandro Graciá Gil]

(In reply to comment #9)
> The WHATWG HTML version does. This was recently changed by Hixie.
> 
> http://www.whatwg.org/specs/web-apps/current-work/multipage/dnd.html#video-conferencing-and-peer-to-peer-communication

There seem to be no changes regarding the part of the spec implemented here (the getUserMedia method).

[Steve Block]

Comment on attachment 86487 [details]
Patch

This is a huge patch and the bindings stuff will need some careful review. I think it would speed things up to split it into smaller patches ...
- Add the runtime flag
- Add CallbackTask.h and update Blob use it
- Add the JavaScript bindings
- Add the outline of the controller/client

Smaller patches are almost always better!

[Leandro Graciá Gil]

(In reply to comment #15)
> (From update of attachment 86487 [details])
> This is a huge patch and the bindings stuff will need some careful review. I think it would speed things up to split it into smaller patches ...
> - Add the runtime flag
> - Add CallbackTask.h and update Blob use it
> - Add the JavaScript bindings
> - Add the outline of the controller/client
> 
> Smaller patches are almost always better!

Ok, I'm splitting this in 3 patches:
- Add the runtime flag: https://bugs.webkit.org/show_bug.cgi?id=56921
- Add getUserMedia to the navigator and the required JS bindings: (this bug, updating soon)
- Add the skeleton of MediaStreamController and MediaStreamClient: https://bugs.webkit.org/show_bug.cgi?id=56922

I have also renamed the StreamController and StreamClient classes to MediaStream ones to make their purposes clearer.

[Leandro Graciá Gil]

Created attachment 86614 [details]
Patch

[Leandro Graciá Gil]

Split the bug in to three patches (see comment above).

[Leandro Graciá Gil]

Created attachment 86616 [details]
Patch

[Leandro Graciá Gil]

Rebasing to test the bots. No code changes from last patch.

[WebKit Review Bot]

Attachment 86616 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/8228780

[Steve Block]

Comment on attachment 86616 [details]
Patch

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

first couple of comments ...

> Source/WebCore/page/CallbackTask.h:43
> +// Auxiliar template to schedule calls to callbacks using their own script execution context.

'Auxiliary' or just 'Helper'

> Source/WebCore/page/CallbackTask.h:73
> +        , m_data(adoptRef(data))

Why are you calling adoptRef() here? adoptRef() is intended for use when an object is first created with 'new'. See http://www.webkit.org/coding/RefPtr.html

[Leandro Graciá Gil]

Created attachment 86663 [details]
Patch

[Leandro Graciá Gil]

Rebasing, fixing a problem in WebCore.pro and changing raw pointers into PassRefPtr in CallbackTask.h as part of an update to this and 56922.

[Leandro Graciá Gil]

Comment on attachment 86616 [details]
Patch

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

>> Source/WebCore/page/CallbackTask.h:43
>> +// Auxiliar template to schedule calls to callbacks using their own script execution context.
> 
> 'Auxiliary' or just 'Helper'

Fixed.

>> Source/WebCore/page/CallbackTask.h:73
>> +        , m_data(adoptRef(data))
> 
> Why are you calling adoptRef() here? adoptRef() is intended for use when an object is first created with 'new'. See http://www.webkit.org/coding/RefPtr.html

Fixed. Was part of a temporary fix to avoid cycles while including.

[Leandro Graciá Gil]

Created attachment 86676 [details]
Patch

[Leandro Graciá Gil]

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

Added missing virtual destructors to JavaScriptCore custom binding headers.

[Steve Block]

Comment on attachment 86676 [details]
Patch

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

I think we can avoid the need for any custom callbacks. See Bug 40065.

Also, you should add some tests for the bindings. See LayoutTests/fast/dom/Geolocation/enabled.html and LayoutTests/fast/dom/Geolocation/argument-types.html for example.

> Source/WebCore/bindings/js/JSNavigatorCustom.cpp:50
> +    // FIXME: This check disallows callable objects created via JSC API. It's not clear what exactly the specification intends to allow.

A FIXME is OK here for now, but add a link to https://bugs.webkit.org/show_bug.cgi?id=40012

> Source/WebCore/bindings/js/JSNavigatorCustom.cpp:62
> +    // Argument is optional (hence undefined is allowed), and null is allowed.

Are you sure that null is allowed? I don't see this in the spec.

> Source/WebCore/page/NavigatorUserMediaError.idl:33
> +    // FIXME: delete this interface and implement a custom binding since spec defines it as a NoInterfaceObject.

I don't think that it being marked NoInterfaceObject means that it needs to have custom bindings. It just means that the constructor object shouldn't appear on the global object. You should add a test for this.

[Leandro Graciá Gil]

Created attachment 87905 [details]
Patch

[Leandro Graciá Gil]

Comment on attachment 86676 [details]
Patch

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

>> Source/WebCore/bindings/js/JSNavigatorCustom.cpp:50
>> +    // FIXME: This check disallows callable objects created via JSC API. It's not clear what exactly the specification intends to allow.
> 
> A FIXME is OK here for now, but add a link to https://bugs.webkit.org/show_bug.cgi?id=40012

Fixed.

>> Source/WebCore/bindings/js/JSNavigatorCustom.cpp:62
>> +    // Argument is optional (hence undefined is allowed), and null is allowed.
> 
> Are you sure that null is allowed? I don't see this in the spec.

Fixed.

>> Source/WebCore/page/NavigatorUserMediaError.idl:33
>> +    // FIXME: delete this interface and implement a custom binding since spec defines it as a NoInterfaceObject.
> 
> I don't think that it being marked NoInterfaceObject means that it needs to have custom bindings. It just means that the constructor object shouldn't appear on the global object. You should add a test for this.

Fixed.

[Steve Block]

Comment on attachment 87905 [details]
Patch

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

Need to update the Android makefiles and fix the EFL build.

I don't see a test for that fact that NavigatorUserMediaError is marked NoInterfaceObject? Also, can you add a test that navigator.webkitGetUserMedia() is present and enumerable, like fast/dom/Geolocation/enabled.html.

> LayoutTests/fast/dom/MediaStream/script-tests/argument-types.js:16
> +            shouldThrow(expression, '(function() { return "' + expectedException + '"; })();');

Do you use this functionality?

> LayoutTests/fast/dom/MediaStream/script-tests/argument-types.js:76
> +test('navigator.webkitGetUserMedia("video", emptyFunction, -Infinity)', true);

I don't think that any of these should throw, other than the one using objectThrowingException. I think the argument should just get converted to a string using the normal JS conversions.

> Source/WebCore/DerivedSources.make:780
> +endif

Usually we add new files to the makefiles for all ports, but leave it up to that port to add the machinery for flags etc.

> Source/WebCore/bindings/v8/custom/V8NavigatorCustom.cpp:4
> + * Redistribution and use in source and binary forms, with or without

Wrong license - http://trac.webkit.org/browser/trunk/Source/WebKit/LICENSE - throughout

> Source/WebCore/bindings/v8/custom/V8NavigatorCustom.cpp:48
> +static void throwTypeMismatchException()

This function and constant are copied from Geolocation, right? Is it used elsewhere too? It would be good to move it to a common place and share it. Maybe do this as a separate patch to keep this one manageable.

> Source/WebCore/bindings/v8/custom/V8NavigatorCustom.cpp:53
> +static PassRefPtr<NavigatorUserMediaSuccessCallback> createNavigatorUserMediaSuccessCallback(v8::Local<v8::Value> value, bool& succeeded)

This method and the one below are very similar to each other and to corresponding methods in Geolocation. How about we factor them out by adding allowNull and allowUndefined arguments? Are there other places in existing code that could use such helpers?

> Source/WebCore/bindings/v8/custom/V8NavigatorCustom.cpp:97
> +    if (!succeeded || !successCallback)

Is the check for successCallback required? Can V8NavigatorUserMediaSuccessCallback::create return 0?

> Source/WebCore/page/CallbackTask.h:84
> +class CallbackTask2 : public ScriptExecutionContext::Task {

It looks like CallbackTask2 isn't used in this patch? Also, it's not clear from this patch why the error callback needs to implement CallbackTask1::Scheduler, as the functionality isn't used yet.

I think it's better to take CallbackTask out of this patch. It can then be added separately before we add the UserMedia functionality that actually requires it.

> Source/WebCore/page/NavigatorUserMediaError.h:49
> +    unsigned short code() const { return m_code; }

The constructor param and this getter should use the enum type.

[John Knottenbelt]

Comment on attachment 87905 [details]
Patch

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

>> LayoutTests/fast/dom/MediaStream/script-tests/argument-types.js:76

> 
> I don't think that any of these should throw, other than the one using objectThrowingException. I think the argument should just get converted to a string using the normal JS conversions.

The IDL type of the third argument is "in optional NavigatorUserMediaErrorCallback", which is a function only interface. It's my understanding that string type should not be compatible with this, unless I am mistaken?

[Steve Block]

Comment on attachment 87905 [details]
Patch

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

> LayoutTests/fast/dom/MediaStream/script-tests/argument-types.js:35
> +// 1 Argument

Maybe add a comment that the reason all these throw is not due to the type conversion, but simply because the API requires 2 arguments. Test with emptyFunction too for completeness.

> LayoutTests/fast/dom/MediaStream/script-tests/argument-types.js:40
> +test('navigator.webkitGetUserMedia("video")', true);

Could use "video" for the other arguments below, too.

> LayoutTests/fast/dom/MediaStream/script-tests/argument-types.js:52
> +test('navigator.webkitGetUserMedia(objectThrowingException, emptyFunction)', false);

Shouldn't this one throw 'valueOf threw exception'? Or does toString() not use valueOf()? Maybe try an object with a custom toString() that throws?

>> LayoutTests/fast/dom/MediaStream/script-tests/argument-types.js:76
>> +test('navigator.webkitGetUserMedia("video", emptyFunction, -Infinity)', true);
> 
> I don't think that any of these should throw, other than the one using objectThrowingException. I think the argument should just get converted to a string using the normal JS conversions.

Sorry, just realised that this comment is wrong - ignore it. I was getting confused with the Geolocation API.

[Leandro Graciá Gil]

Created attachment 88637 [details]
Patch

[Leandro Graciá Gil]

Comment on attachment 87905 [details]
Patch

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

>> LayoutTests/fast/dom/MediaStream/script-tests/argument-types.js:16
>> +            shouldThrow(expression, '(function() { return "' + expectedException + '"; })();');
> 
> Do you use this functionality?

We use it now. Fixed.

>> LayoutTests/fast/dom/MediaStream/script-tests/argument-types.js:35
>> +// 1 Argument
> 
> Maybe add a comment that the reason all these throw is not due to the type conversion, but simply because the API requires 2 arguments. Test with emptyFunction too for completeness.

Fixed.

>> LayoutTests/fast/dom/MediaStream/script-tests/argument-types.js:40
>> +test('navigator.webkitGetUserMedia("video")', true);
> 
> Could use "video" for the other arguments below, too.

Fixed.

>> LayoutTests/fast/dom/MediaStream/script-tests/argument-types.js:52
>> +test('navigator.webkitGetUserMedia(objectThrowingException, emptyFunction)', false);
> 
> Shouldn't this one throw 'valueOf threw exception'? Or does toString() not use valueOf()? Maybe try an object with a custom toString() that throws?

Fixed.

>> Source/WebCore/DerivedSources.make:780
>> +endif
> 
> Usually we add new files to the makefiles for all ports, but leave it up to that port to add the machinery for flags etc.

Fixed.

>> Source/WebCore/bindings/v8/custom/V8NavigatorCustom.cpp:4
>> + * Redistribution and use in source and binary forms, with or without
> 
> Wrong license - http://trac.webkit.org/browser/trunk/Source/WebKit/LICENSE - throughout

Fixed.

>> Source/WebCore/bindings/v8/custom/V8NavigatorCustom.cpp:48
>> +static void throwTypeMismatchException()
> 
> This function and constant are copied from Geolocation, right? Is it used elsewhere too? It would be good to move it to a common place and share it. Maybe do this as a separate patch to keep this one manageable.

Fixed. Integrating into V8Utilities in https://bugs.webkit.org/show_bug.cgi?id=57760.

>> Source/WebCore/bindings/v8/custom/V8NavigatorCustom.cpp:53
>> +static PassRefPtr<NavigatorUserMediaSuccessCallback> createNavigatorUserMediaSuccessCallback(v8::Local<v8::Value> value, bool& succeeded)
> 
> This method and the one below are very similar to each other and to corresponding methods in Geolocation. How about we factor them out by adding allowNull and allowUndefined arguments? Are there other places in existing code that could use such helpers?

Fixed. Integrating into V8Utilities in https://bugs.webkit.org/show_bug.cgi?id=57760.

>> Source/WebCore/bindings/v8/custom/V8NavigatorCustom.cpp:97
>> +    if (!succeeded || !successCallback)
> 
> Is the check for successCallback required? Can V8NavigatorUserMediaSuccessCallback::create return 0?

Fixed.

>> Source/WebCore/page/CallbackTask.h:84
>> +class CallbackTask2 : public ScriptExecutionContext::Task {
> 
> It looks like CallbackTask2 isn't used in this patch? Also, it's not clear from this patch why the error callback needs to implement CallbackTask1::Scheduler, as the functionality isn't used yet.
> 
> I think it's better to take CallbackTask out of this patch. It can then be added separately before we add the UserMedia functionality that actually requires it.

Fixed.

>> Source/WebCore/page/NavigatorUserMediaError.h:49
>> +    unsigned short code() const { return m_code; }
> 
> The constructor param and this getter should use the enum type.

Fixed.

[Leandro Graciá Gil]

(In reply to comment #31)
> (From update of attachment 87905 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=87905&action=review
> 
> Need to update the Android makefiles and fix the EFL build.
> 
> I don't see a test for that fact that NavigatorUserMediaError is marked NoInterfaceObject? Also, can you add a test that navigator.webkitGetUserMedia() is present and enumerable, like fast/dom/Geolocation/enabled.html.
> 

Done.

[Steve Block]

Comment on attachment 88637 [details]
Patch

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

> LayoutTests/fast/dom/MediaStream/script-tests/enabled.js:3
> +function hasGeolocationProperty()

Geolocation ?!

> Source/WebCore/bindings/js/JSNavigatorCustom.cpp:54
> +    if (exec->hadException() || !successCallback)

Do you need to check !successCallback? checkFunctionOnlyCallback is called with no flags so will only return null when it sets an exception. Can JSNavigatorUserMediaSuccessCallback::create() return null?

> Source/WebCore/page/NavigatorUserMediaError.h:37
> +        PERMISSION_DENIED = 1

Maybe add a comment that these values need to kept in sync with those in the IDL?

> Source/WebCore/page/NavigatorUserMediaSuccessCallback.idl:2
> + * Copyright (C) 2011 Apple Inc. All rights reserved.

Google Inc - throughout

[Leandro Graciá Gil]

Created attachment 88795 [details]
Patch

[Leandro Graciá Gil]

Comment on attachment 88637 [details]
Patch

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

>> LayoutTests/fast/dom/MediaStream/script-tests/enabled.js:3
>> +function hasGeolocationProperty()
> 
> Geolocation ?!

Sorry, I missed that one. Fixed.

>> Source/WebCore/bindings/js/JSNavigatorCustom.cpp:54
>> +    if (exec->hadException() || !successCallback)
> 
> Do you need to check !successCallback? checkFunctionOnlyCallback is called with no flags so will only return null when it sets an exception. Can JSNavigatorUserMediaSuccessCallback::create() return null?

Fixed.

>> Source/WebCore/page/NavigatorUserMediaError.h:37
>> +        PERMISSION_DENIED = 1
> 
> Maybe add a comment that these values need to kept in sync with those in the IDL?

Fixed.

>> Source/WebCore/page/NavigatorUserMediaSuccessCallback.idl:2
>> + * Copyright (C) 2011 Apple Inc. All rights reserved.
> 
> Google Inc - throughout

Fixed.

[Steve Block]

Comment on attachment 88795 [details]
Patch

r=me

[WebKit Commit Bot]

Comment on attachment 88795 [details]
Patch

Rejecting attachment 88795 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-3', 'apply-..." exit_code: 2

Last 500 characters of output:
/NavigatorUserMediaError.h
patching file Source/WebCore/page/NavigatorUserMediaError.idl
patching file Source/WebCore/page/NavigatorUserMediaErrorCallback.h
patching file Source/WebCore/page/NavigatorUserMediaErrorCallback.idl
patching file Source/WebCore/page/NavigatorUserMediaSuccessCallback.h
patching file Source/WebCore/page/NavigatorUserMediaSuccessCallback.idl

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--reviewer', u'Steve Block', u'--force']" exit_code: 1

Full output: http://queues.webkit.org/results/8373497

[Leandro Graciá Gil]

Created attachment 88798 [details]
Patch

[Leandro Graciá Gil]

Comment on attachment 88798 [details]
Patch

Rebased.

[Steve Block]

Comment on attachment 88798 [details]
Patch

r=me

[WebKit Commit Bot]

Comment on attachment 88798 [details]
Patch

Rejecting attachment 88798 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-7', 'apply-..." exit_code: 2

Last 500 characters of output:
/NavigatorUserMediaError.h
patching file Source/WebCore/page/NavigatorUserMediaError.idl
patching file Source/WebCore/page/NavigatorUserMediaErrorCallback.h
patching file Source/WebCore/page/NavigatorUserMediaErrorCallback.idl
patching file Source/WebCore/page/NavigatorUserMediaSuccessCallback.h
patching file Source/WebCore/page/NavigatorUserMediaSuccessCallback.idl

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--reviewer', u'Steve Block', u'--force']" exit_code: 1

Full output: http://queues.webkit.org/results/8346989

[Steve Block]

Committed r83287: <http://trac.webkit.org/changeset/83287>

[WebKit Review Bot]

http://trac.webkit.org/changeset/83287 might have broken Qt Linux Release
The following tests are not passing:
fast/dom/MediaStream/argument-types.html
fast/dom/MediaStream/enabled.html
fast/dom/MediaStream/no-interface-object.html

[Sergio Villar Senin]

(In reply to comment #47)
> http://trac.webkit.org/changeset/83287 might have broken Qt Linux Release
> The following tests are not passing:
> fast/dom/MediaStream/argument-types.html
> fast/dom/MediaStream/enabled.html
> fast/dom/MediaStream/no-interface-object.html

Same for GTK+ bots

[Steve Block]

Leandro is preparing a patch to add these new tests to all the relevant skip lists

[Leandro Graciá Gil]

Created attachment 88814 [details]
Patch

Adding the new tests to the skip lists of all non-chromium platforms as the Media Stream API is not enabled in them.

[Steve Block]

Comment on attachment 88814 [details]
Patch

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

> LayoutTests/platform/gtk/Skipped:1384
> +fast/dom/MediaStream/no-interface-object.html

You cna just list fast/dom/MediaStream. Also, be sure to add to the appropriate section of each list (usually something like 'unimplemented features')

> LayoutTests/platform/mac-leopard/Skipped:227
> +fast/dom/MediaStream/no-interface-object.html

No need for this. The skipped lists inherit from each other, so platform-foo inherits everything from platform.

[Leandro Graciá Gil]

Created attachment 88820 [details]
Patch

[Leandro Graciá Gil]

Comment on attachment 88814 [details]
Patch

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

>> LayoutTests/platform/gtk/Skipped:1384
>> +fast/dom/MediaStream/no-interface-object.html
> 
> You cna just list fast/dom/MediaStream. Also, be sure to add to the appropriate section of each list (usually something like 'unimplemented features')

Fixed.

>> LayoutTests/platform/mac-leopard/Skipped:227
>> +fast/dom/MediaStream/no-interface-object.html
> 
> No need for this. The skipped lists inherit from each other, so platform-foo inherits everything from platform.

Fixed.

[Steve Block]

Committed r83292: <http://trac.webkit.org/changeset/83292>

[Steve Block]

Comment on attachment 88820 [details]
Patch

Added new tests to skipped lists

[WebKit Review Bot]

http://trac.webkit.org/changeset/83292 might have broken GTK Linux 32-bit Debug