[Webkit-unassigned] [Bug 56586] Media Stream API patch 1: adding the getUserMedia method and the basic skeleton

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=56586





--- Comment #10 from Steve Block <steveblock at google.com>  2011-03-21 03:18:25 PST ---
(From update of attachment 86198)
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?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.