[Webkit-unassigned] [Bug 39474] [GStreamer] GTK XOverlay support in GStreamerGWorld

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Aug 2 15:44:04 PDT 2010


Gustavo Noronha (kov) <gns at gnome.org> changed:

           What    |Removed                     |Added
  Attachment #62047|review?                     |review-
               Flag|                            |

--- Comment #28 from Gustavo Noronha (kov) <gns at gnome.org>  2010-08-02 15:44:04 PST ---
(From update of attachment 62047)
 +      g_object_set(m_exitFullscreenAction, "icon-name", "view-restore-symbolic", NULL);
symbolic icons are quite new, are we going to have to shift our dependencies for this?

 294     if (!node->hasTagName(HTMLNames::videoTag))
 295         return;
 297 #if ENABLE(VIDEO)

Might as well only do the check if video is enabled? Otherwise it's just useless work.

311 void webkit_web_view_exit_fullscreen(WebKitWebView* webView)
 312 {
 313 #if ENABLE(VIDEO)
 314     WebKitWebViewPrivate* priv = webView->priv;
 315     if (priv->fullscreenVideoController)
 316         priv->fullscreenVideoController->exitFullscreen();
 317 #endif

Certainly a good idea to NULLify the fullscreenVideoController here. Also, I believe you need to exit fullscreen in case a fullscreenVideoController exists during WebKitWebView's destruction? The callbacks receiving C++ objects as user data still concern me, I'm pretty sure we'll get crashes from the object going away and the callback being called right after that.

These are my last concerns, I think, the rest of the patch looks good, so let's try to clear them up and land =D

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

More information about the webkit-unassigned mailing list