[Webkit-unassigned] [Bug 94373] [gtk] adding methods and signals for usermedia communication between webkit and browser

[bugzilla-daemon at webkit.org]

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





--- Comment #19 from Danilo Cesar Lemes de Paula <danilo.cesar at collabora.co.uk>  2012-10-03 11:14:43 PST ---
(From update of attachment 166722)
View in context: https://bugs.webkit.org/attachment.cgi?id=166722&action=review

>> Source/WebKit/gtk/WebCoreSupport/UserMediaClientGtk.cpp:29
>> +#include "webkitwebusermediarequestprivate.h"
> 
> That was correct, it should be config and then UserMediaClientGtk.h, followed by an empty line and then all other headers. UserMediaClientGtk.h already has #if ENABLE(MEDIA_STREAM) so it's not a problem if it's used before the #if here.

fixed.

>> Source/WebKit/gtk/WebCoreSupport/UserMediaClientGtk.cpp:35
>> +UserMediaClientGtk::UserMediaClientGtk(WebKitWebView *webView)
> 
> WebKitWebView *webView -> WebKitWebView* webView

ok.

>> Source/WebKit/gtk/WebCoreSupport/UserMediaClientGtk.cpp:49
>>  {
> 
> This is not needed, there's using namespace WebCore; in this file

ok

>> Source/WebKit/gtk/WebCoreSupport/UserMediaClientGtk.cpp:59
>> +}
> 
> Why do you remove the comment?

fixed.

>> Source/WebKit/gtk/webkit/webkitwebusermedialist.cpp:28
>> +#include <wtf/text/CString.h>
> 
> This should be
> 
> #include "config.h"
> #include "webkitwebusermedialist.h"
> 
> #if ENABLE(MEDIA_STREAM)
> #include "webkitglobalsprivate.h"
> #include "webkitwebusermedialistprivate.h"
> #include <wtf/text/CString.h>

fixed.

>> Source/WebKit/gtk/webkit/webkitwebusermedialist.cpp:39
>> + * Since: 1.10.0
> 
> 1.10.0 was released already, this should be 2.0

I was thinking that it should be something like Since: TODO, and fix that after the approval.
But ok, changing to 2.0.0

>> Source/WebKit/gtk/webkit/webkitwebusermedialist.cpp:59
>> +}
> 
> I don't think you need this, the vector will be released by the struct destructor in Finalize()

The whole dispose was removed.

>> Source/WebKit/gtk/webkit/webkitwebusermedialist.cpp:69
>> +    GObjectClass* gobject_class = G_OBJECT_CLASS(listClass);
> 
> gobject_class -> gobjectClass

fixed.

>> Source/WebKit/gtk/webkit/webkitwebusermedialist.cpp:95
>> + * Since: 1.10.0
> 
> 2.0

fixed,

>> Source/WebKit/gtk/webkit/webkitwebusermedialist.cpp:114
>> + * Since: 1.10.0
> 
> Ditto.

ok.

>> Source/WebKit/gtk/webkit/webkitwebusermedialist.cpp:135
>> + * Since: 1.10.0
> 
> Ditto.

ok

>> Source/WebKit/gtk/webkit/webkitwebusermedialist.cpp:159
>> +    g_return_if_fail(index < core(list).size());
> 
> Extra new line there

ok

>> Source/WebKit/gtk/webkit/webkitwebusermediarequest.cpp:24
>> +#include "webkitwebusermediarequest.h"
> 
> ame issue here with the headers

fixed.

>> Source/WebKit/gtk/webkit/webkitwebusermediarequest.cpp:42
>> + * Since: 1.10.0
> 
> 2.0

ok

>> Source/WebKit/gtk/webkit/webkitwebusermediarequest.cpp:84
>> + * Return value: True if the request contains an audio request.
> 
> True -> %TRUE
> 
> True if the request contains an audio request or %FALSE otherwise.

ok

>> Source/WebKit/gtk/webkit/webkitwebusermediarequest.cpp:101
>> + * Return value: True if the request contains a video request.
> 
> Ditto

ok

>> Source/WebKit/gtk/webkit/webkitwebusermediarequest.cpp:133
>> +    g_return_val_if_fail(WEBKIT_IS_WEB_USER_MEDIA_REQUEST(request), 0);
> 
> This is not a public API mehod, use ASSERT or nothing instead of g_return macros.

ok

>> Source/WebKit/gtk/webkit/webkitwebusermediarequest.cpp:140
>> +    g_return_val_if_fail(request, 0);
> 
> Ditto.

ok

>> Source/WebKit/gtk/webkit/webkitwebview.cpp:5410
>> +    core(webRequest)->succeed(audioSources, videoSources);
> 
> I wonder why this is a method of WebView if it doesn't use the web view. Looks too me like a method of WebKitWebUserMediaRequest, something like webkit_web_user_media_request_accept();

vestige of an old implementation. fixed by now

>> Source/WebKit/gtk/webkit/webkitwebview.cpp:5427
>> +    core(webRequest)->fail();
> 
> Same here, webView is unused

ditto

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