Clean up GMutexLocker
authorzandobersek@gmail.com <zandobersek@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 12 Aug 2014 10:39:07 +0000 (10:39 +0000)
committerzandobersek@gmail.com <zandobersek@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 12 Aug 2014 10:39:07 +0000 (10:39 +0000)
https://bugs.webkit.org/show_bug.cgi?id=135833

Reviewed by Carlos Garcia Campos.

Source/WebCore:

Don't dynamically allocate GMutex objects. Update GMutexLocker
initializations to pass in a GMutex reference, not a pointer.

* platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:
(WebCore::MediaPlayerPrivateGStreamerBase::MediaPlayerPrivateGStreamerBase):
(WebCore::MediaPlayerPrivateGStreamerBase::~MediaPlayerPrivateGStreamerBase):
* platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.h:
* platform/graphics/gstreamer/VideoSinkGStreamer.cpp:
(_WebKitVideoSinkPrivate::_WebKitVideoSinkPrivate): Initialize the GMutex.
(_WebKitVideoSinkPrivate::~_WebKitVideoSinkPrivate): Clear the GMutex.
(webkit_video_sink_init):
(webkitVideoSinkRender):
(webkitVideoSinkDispose):
* platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:
(webKitWebSrcGetProperty):
(webKitWebSrcStop):
(webKitWebSrcStart):
(webKitWebSrcChangeState):
(webKitWebSrcQueryWithParent):
(webKitWebSrcGetUri):
(webKitWebSrcSetUri):
(webKitWebSrcNeedDataMainCb):
(webKitWebSrcNeedDataCb):
(webKitWebSrcEnoughDataMainCb):
(webKitWebSrcEnoughDataCb):
(webKitWebSrcSeekDataCb):
(webKitWebSrcSetMediaPlayer):
(StreamingClient::createReadBuffer):
(StreamingClient::handleResponseReceived):
(StreamingClient::handleDataReceived):
(StreamingClient::handleNotifyFinished):
(ResourceHandleStreamingClient::wasBlocked):
(ResourceHandleStreamingClient::cannotShowURL):

Source/WTF:

Place the GMutexLocker into the WTF namespace. There's no need for this
class to use FastMalloc since it's always allocated on the stack. The
constructor and class now operate on a GMutex reference. There's little
need for an additional inline specifier for methods defined in the header.
The mutex() method is removed as it was not used. m_val is renamed to a
more descriptive m_locked and is made a boolean.

* wtf/gobject/GMutexLocker.h:
(WTF::GMutexLocker::GMutexLocker):
(WTF::GMutexLocker::~GMutexLocker):
(WTF::GMutexLocker::lock):
(WTF::GMutexLocker::unlock):
(WebCore::GMutexLocker::GMutexLocker): Deleted.
(WebCore::GMutexLocker::~GMutexLocker): Deleted.
(WebCore::GMutexLocker::lock): Deleted.
(WebCore::GMutexLocker::unlock): Deleted.
(WebCore::GMutexLocker::mutex): Deleted.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@172441 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WTF/ChangeLog
Source/WTF/wtf/gobject/GMutexLocker.h
Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp
Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.h
Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp
Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp

index e5e37fafd0a96423f438dae55682e836d6f158c8..03bd788e02b76fdc14cb43345420ccfcfa399f6b 100644 (file)
@@ -1,3 +1,28 @@
+2014-08-12  Zan Dobersek  <zdobersek@igalia.com>
+
+        Clean up GMutexLocker
+        https://bugs.webkit.org/show_bug.cgi?id=135833
+
+        Reviewed by Carlos Garcia Campos.
+
+        Place the GMutexLocker into the WTF namespace. There's no need for this
+        class to use FastMalloc since it's always allocated on the stack. The
+        constructor and class now operate on a GMutex reference. There's little
+        need for an additional inline specifier for methods defined in the header.
+        The mutex() method is removed as it was not used. m_val is renamed to a
+        more descriptive m_locked and is made a boolean.
+
+        * wtf/gobject/GMutexLocker.h:
+        (WTF::GMutexLocker::GMutexLocker):
+        (WTF::GMutexLocker::~GMutexLocker):
+        (WTF::GMutexLocker::lock):
+        (WTF::GMutexLocker::unlock):
+        (WebCore::GMutexLocker::GMutexLocker): Deleted.
+        (WebCore::GMutexLocker::~GMutexLocker): Deleted.
+        (WebCore::GMutexLocker::lock): Deleted.
+        (WebCore::GMutexLocker::unlock): Deleted.
+        (WebCore::GMutexLocker::mutex): Deleted.
+
 2014-08-12  Zan Dobersek  <zdobersek@igalia.com>
 
         Make GRefPtr move-able
index bf7864ae136f7a58dc076c3575af702d5f03404f..0dd1f76f164dcb463963b576089aaaabc1c3f4ab 100644 (file)
 #if USE(GLIB)
 
 #include <glib.h>
-
-#include <wtf/FastMalloc.h>
 #include <wtf/Noncopyable.h>
 
-namespace WebCore {
+namespace WTF {
 
 class GMutexLocker {
-    WTF_MAKE_NONCOPYABLE(GMutexLocker); WTF_MAKE_FAST_ALLOCATED;
-
+    WTF_MAKE_NONCOPYABLE(GMutexLocker);
 public:
-    inline explicit GMutexLocker(GMutex* mutex)
+    explicit GMutexLocker(GMutex& mutex)
         : m_mutex(mutex)
-        , m_val(0)
+        , m_locked(false)
     {
         lock();
     }
 
-    inline ~GMutexLocker() { unlock(); }
+    ~GMutexLocker()
+    {
+        unlock();
+    }
 
-    inline void lock()
+    void lock()
     {
-        if (m_mutex && !m_val) {
-            g_mutex_lock(m_mutex);
-            m_val = 1;
+        if (!m_locked) {
+            g_mutex_lock(&m_mutex);
+            m_locked = true;
         }
     }
 
-    inline void unlock()
+    void unlock()
     {
-        if (m_mutex && m_val) {
-            m_val = 0;
-            g_mutex_unlock(m_mutex);
+        if (m_locked) {
+            m_locked = false;
+            g_mutex_unlock(&m_mutex);
         }
     }
 
-    inline GMutex* mutex() const { return m_mutex; }
-
 private:
-    GMutex* m_mutex;
-    uint8_t m_val;
+    GMutex& m_mutex;
+    bool m_locked;
 };
 
-}
+} // namespace WTF
+
+using WTF::GMutexLocker;
 
 #endif // USE(GLIB)
 
index 30f1e5a897be88e169b26aeef351c09386b41cbb..9891091fed6a02b2df0b71185b54715b2a44fce1 100644 (file)
@@ -1,3 +1,44 @@
+2014-08-12  Zan Dobersek  <zdobersek@igalia.com>
+
+        Clean up GMutexLocker
+        https://bugs.webkit.org/show_bug.cgi?id=135833
+
+        Reviewed by Carlos Garcia Campos.
+
+        Don't dynamically allocate GMutex objects. Update GMutexLocker
+        initializations to pass in a GMutex reference, not a pointer.
+
+        * platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:
+        (WebCore::MediaPlayerPrivateGStreamerBase::MediaPlayerPrivateGStreamerBase):
+        (WebCore::MediaPlayerPrivateGStreamerBase::~MediaPlayerPrivateGStreamerBase):
+        * platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.h:
+        * platform/graphics/gstreamer/VideoSinkGStreamer.cpp:
+        (_WebKitVideoSinkPrivate::_WebKitVideoSinkPrivate): Initialize the GMutex.
+        (_WebKitVideoSinkPrivate::~_WebKitVideoSinkPrivate): Clear the GMutex.
+        (webkit_video_sink_init):
+        (webkitVideoSinkRender):
+        (webkitVideoSinkDispose):
+        * platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:
+        (webKitWebSrcGetProperty):
+        (webKitWebSrcStop):
+        (webKitWebSrcStart):
+        (webKitWebSrcChangeState):
+        (webKitWebSrcQueryWithParent):
+        (webKitWebSrcGetUri):
+        (webKitWebSrcSetUri):
+        (webKitWebSrcNeedDataMainCb):
+        (webKitWebSrcNeedDataCb):
+        (webKitWebSrcEnoughDataMainCb):
+        (webKitWebSrcEnoughDataCb):
+        (webKitWebSrcSeekDataCb):
+        (webKitWebSrcSetMediaPlayer):
+        (StreamingClient::createReadBuffer):
+        (StreamingClient::handleResponseReceived):
+        (StreamingClient::handleDataReceived):
+        (StreamingClient::handleNotifyFinished):
+        (ResourceHandleStreamingClient::wasBlocked):
+        (ResourceHandleStreamingClient::cannotShowURL):
+
 2014-08-12  Eduardo Lima Mitev  <elima@igalia.com>
         [GTK] Adds dependency on GnuTLS 3.0+ for the implementation of subtle crypto algorithms
         https://bugs.webkit.org/show_bug.cgi?id=133317
index 2e6953b6d768565a85b18cddadd90e7d8377a7d3..11de72f65eab128f9b26fadb2db49c0b2cbcb3bc 100644 (file)
@@ -94,8 +94,7 @@ MediaPlayerPrivateGStreamerBase::MediaPlayerPrivateGStreamerBase(MediaPlayer* pl
     , m_volumeSignalHandler(0)
     , m_muteSignalHandler(0)
 {
-    m_bufferMutex = new GMutex;
-    g_mutex_init(m_bufferMutex);
+    g_mutex_init(&m_bufferMutex);
 }
 
 MediaPlayerPrivateGStreamerBase::~MediaPlayerPrivateGStreamerBase()
@@ -105,8 +104,7 @@ MediaPlayerPrivateGStreamerBase::~MediaPlayerPrivateGStreamerBase()
         m_repaintHandler = 0;
     }
 
-    g_mutex_clear(m_bufferMutex);
-    delete m_bufferMutex;
+    g_mutex_clear(&m_bufferMutex);
 
     if (m_buffer)
         gst_buffer_unref(m_buffer);
index f21d68076ce80a0072c0d7f29ff20a07ae60ac6d..3a108617e89ed1b6f106febfb91a12828a05864d 100644 (file)
@@ -123,7 +123,7 @@ protected:
     MediaPlayer::ReadyState m_readyState;
     MediaPlayer::NetworkState m_networkState;
     IntSize m_size;
-    GMutex* m_bufferMutex;
+    GMutex m_bufferMutex;
     GstBuffer* m_buffer;
     GMainLoopSource m_volumeTimerHandler;
     GMainLoopSource m_muteTimerHandler;
index 9d9c68a0977f009cc0c14f3a99754a8766739033..fcfec3fb6d65af41ba82ad469638b643a2f83db9 100644 (file)
@@ -74,9 +74,19 @@ enum {
 static guint webkitVideoSinkSignals[LAST_SIGNAL] = { 0, };
 
 struct _WebKitVideoSinkPrivate {
+    _WebKitVideoSinkPrivate()
+    {
+        g_mutex_init(&bufferMutex);
+    }
+
+    ~_WebKitVideoSinkPrivate()
+    {
+        g_mutex_clear(&bufferMutex);
+    }
+
     GstBuffer* buffer;
     GMainLoopSource timeoutSource;
-    GMutex* bufferMutex;
+    GMutex bufferMutex;
     GCond* dataCondition;
 
     GstVideoInfo info;
@@ -104,8 +114,6 @@ static void webkit_video_sink_init(WebKitVideoSink* sink)
     new (sink->priv) WebKitVideoSinkPrivate();
     sink->priv->dataCondition = new GCond;
     g_cond_init(sink->priv->dataCondition);
-    sink->priv->bufferMutex = new GMutex;
-    g_mutex_init(sink->priv->bufferMutex);
 
     gst_video_info_init(&sink->priv->info);
 }
@@ -220,7 +228,7 @@ static GstFlowReturn webkitVideoSinkRender(GstBaseSink* baseSink, GstBuffer* buf
     priv->timeoutSource.schedule("[WebKit] webkitVideoSinkTimeoutCallback", std::function<void()>(std::bind(webkitVideoSinkTimeoutCallback, sink)), G_PRIORITY_DEFAULT,
         [sink] { gst_object_unref(sink); });
 
-    g_cond_wait(priv->dataCondition, priv->bufferMutex);
+    g_cond_wait(priv->dataCondition, &priv->bufferMutex);
     return GST_FLOW_OK;
 }
 
@@ -235,12 +243,6 @@ static void webkitVideoSinkDispose(GObject* object)
         priv->dataCondition = 0;
     }
 
-    if (priv->bufferMutex) {
-        g_mutex_clear(priv->bufferMutex);
-        delete priv->bufferMutex;
-        priv->bufferMutex = 0;
-    }
-
     G_OBJECT_CLASS(parent_class)->dispose(object);
 }
 
index 506e1d33eb3e23fb3964f9dd83a230e2660632b4..efc2e37a5026250d79b696b302493bfa2c35aba6 100644 (file)
@@ -352,7 +352,7 @@ static void webKitWebSrcGetProperty(GObject* object, guint propID, GValue* value
     WebKitWebSrc* src = WEBKIT_WEB_SRC(object);
     WebKitWebSrcPrivate* priv = src->priv;
 
-    GMutexLocker locker(GST_OBJECT_GET_LOCK(src));
+    GMutexLocker locker(*GST_OBJECT_GET_LOCK(src));
     switch (propID) {
     case PROP_IRADIO_NAME:
         g_value_set_string(value, priv->iradioName);
@@ -391,7 +391,7 @@ static void webKitWebSrcStop(WebKitWebSrc* src)
 
     ASSERT(isMainThread());
 
-    GMutexLocker locker(GST_OBJECT_GET_LOCK(src));
+    GMutexLocker locker(*GST_OBJECT_GET_LOCK(src));
 
     bool seeking = priv->seekSource.isActive();
 
@@ -447,7 +447,7 @@ static void webKitWebSrcStart(WebKitWebSrc* src)
 
     ASSERT(isMainThread());
 
-    GMutexLocker locker(GST_OBJECT_GET_LOCK(src));
+    GMutexLocker locker(*GST_OBJECT_GET_LOCK(src));
 
     priv->corsAccessCheck = CORSNoCheck;
 
@@ -541,7 +541,7 @@ static GstStateChangeReturn webKitWebSrcChangeState(GstElement* element, GstStat
         return ret;
     }
 
-    GMutexLocker locker(GST_OBJECT_GET_LOCK(src));
+    GMutexLocker locker(*GST_OBJECT_GET_LOCK(src));
     switch (transition) {
     case GST_STATE_CHANGE_READY_TO_PAUSED:
         GST_DEBUG_OBJECT(src, "READY->PAUSED");
@@ -576,7 +576,7 @@ static gboolean webKitWebSrcQueryWithParent(GstPad* pad, GstObject* parent, GstQ
         gst_query_parse_duration(query, &format, NULL);
 
         GST_DEBUG_OBJECT(src, "duration query in format %s", gst_format_get_name(format));
-        GMutexLocker locker(GST_OBJECT_GET_LOCK(src));
+        GMutexLocker locker(*GST_OBJECT_GET_LOCK(src));
         if (format == GST_FORMAT_BYTES && src->priv->size > 0) {
             gst_query_set_duration(query, format, src->priv->size);
             result = TRUE;
@@ -584,7 +584,7 @@ static gboolean webKitWebSrcQueryWithParent(GstPad* pad, GstObject* parent, GstQ
         break;
     }
     case GST_QUERY_URI: {
-        GMutexLocker locker(GST_OBJECT_GET_LOCK(src));
+        GMutexLocker locker(*GST_OBJECT_GET_LOCK(src));
         gst_query_set_uri(query, src->priv->uri);
         result = TRUE;
         break;
@@ -625,7 +625,7 @@ static gchar* webKitWebSrcGetUri(GstURIHandler* handler)
     WebKitWebSrc* src = WEBKIT_WEB_SRC(handler);
     gchar* ret;
 
-    GMutexLocker locker(GST_OBJECT_GET_LOCK(src));
+    GMutexLocker locker(*GST_OBJECT_GET_LOCK(src));
     ret = g_strdup(src->priv->uri);
     return ret;
 }
@@ -640,7 +640,7 @@ static gboolean webKitWebSrcSetUri(GstURIHandler* handler, const gchar* uri, GEr
         return FALSE;
     }
 
-    GMutexLocker locker(GST_OBJECT_GET_LOCK(src));
+    GMutexLocker locker(*GST_OBJECT_GET_LOCK(src));
 
     g_free(priv->uri);
     priv->uri = 0;
@@ -676,7 +676,7 @@ static void webKitWebSrcNeedDataMainCb(WebKitWebSrc* src)
 
     ASSERT(isMainThread());
 
-    GMutexLocker locker(GST_OBJECT_GET_LOCK(src));
+    GMutexLocker locker(*GST_OBJECT_GET_LOCK(src));
     priv->paused = FALSE;
     locker.unlock();
 
@@ -691,7 +691,7 @@ static void webKitWebSrcNeedDataCb(GstAppSrc*, guint length, gpointer userData)
 
     GST_DEBUG_OBJECT(src, "Need more data: %u", length);
 
-    GMutexLocker locker(GST_OBJECT_GET_LOCK(src));
+    GMutexLocker locker(*GST_OBJECT_GET_LOCK(src));
     if (priv->needDataSource.isScheduled() || !priv->paused)
         return;
 
@@ -706,7 +706,7 @@ static void webKitWebSrcEnoughDataMainCb(WebKitWebSrc* src)
 
     ASSERT(isMainThread());
 
-    GMutexLocker locker(GST_OBJECT_GET_LOCK(src));
+    GMutexLocker locker(*GST_OBJECT_GET_LOCK(src));
     priv->paused = TRUE;
     locker.unlock();
 
@@ -721,7 +721,7 @@ static void webKitWebSrcEnoughDataCb(GstAppSrc*, gpointer userData)
 
     GST_DEBUG_OBJECT(src, "Have enough data");
 
-    GMutexLocker locker(GST_OBJECT_GET_LOCK(src));
+    GMutexLocker locker(*GST_OBJECT_GET_LOCK(src));
     if (priv->enoughDataSource.isScheduled() || priv->paused)
         return;
 
@@ -744,7 +744,7 @@ static gboolean webKitWebSrcSeekDataCb(GstAppSrc*, guint64 offset, gpointer user
     WebKitWebSrcPrivate* priv = src->priv;
 
     GST_DEBUG_OBJECT(src, "Seeking to offset: %" G_GUINT64_FORMAT, offset);
-    GMutexLocker locker(GST_OBJECT_GET_LOCK(src));
+    GMutexLocker locker(*GST_OBJECT_GET_LOCK(src));
     if (offset == priv->offset && priv->requestedOffset == priv->offset)
         return TRUE;
 
@@ -763,7 +763,7 @@ static gboolean webKitWebSrcSeekDataCb(GstAppSrc*, guint64 offset, gpointer user
 void webKitWebSrcSetMediaPlayer(WebKitWebSrc* src, WebCore::MediaPlayer* player)
 {
     ASSERT(player);
-    GMutexLocker locker(GST_OBJECT_GET_LOCK(src));
+    GMutexLocker locker(*GST_OBJECT_GET_LOCK(src));
     src->priv->player = player;
 }
 
@@ -792,7 +792,7 @@ char* StreamingClient::createReadBuffer(size_t requestedSize, size_t& actualSize
 
     mapGstBuffer(buffer);
 
-    GMutexLocker locker(GST_OBJECT_GET_LOCK(src));
+    GMutexLocker locker(*GST_OBJECT_GET_LOCK(src));
     priv->buffer = adoptGRef(buffer);
     locker.unlock();
 
@@ -818,7 +818,7 @@ void StreamingClient::handleResponseReceived(const ResourceResponse& response, C
         return;
     }
 
-    GMutexLocker locker(GST_OBJECT_GET_LOCK(src));
+    GMutexLocker locker(*GST_OBJECT_GET_LOCK(src));
 
     priv->corsAccessCheck = corsAccessCheck;
 
@@ -917,7 +917,7 @@ void StreamingClient::handleDataReceived(const char* data, int length)
     WebKitWebSrc* src = WEBKIT_WEB_SRC(m_src.get());
     WebKitWebSrcPrivate* priv = src->priv;
 
-    GMutexLocker locker(GST_OBJECT_GET_LOCK(src));
+    GMutexLocker locker(*GST_OBJECT_GET_LOCK(src));
 
     GST_LOG_OBJECT(src, "Have %lld bytes of data", priv->buffer ? static_cast<long long>(gst_buffer_get_size(priv->buffer.get())) : length);
 
@@ -986,7 +986,7 @@ void StreamingClient::handleNotifyFinished()
 
     GST_DEBUG_OBJECT(src, "Have EOS");
 
-    GMutexLocker locker(GST_OBJECT_GET_LOCK(src));
+    GMutexLocker locker(*GST_OBJECT_GET_LOCK(src));
     if (!priv->seekSource.isActive()) {
         locker.unlock();
         gst_app_src_end_of_stream(priv->appsrc);
@@ -1145,7 +1145,7 @@ void ResourceHandleStreamingClient::wasBlocked(ResourceHandle*)
 
     GST_ERROR_OBJECT(src, "Request was blocked");
 
-    GMutexLocker locker(GST_OBJECT_GET_LOCK(src));
+    GMutexLocker locker(*GST_OBJECT_GET_LOCK(src));
     uri.reset(g_strdup(src->priv->uri));
     locker.unlock();
 
@@ -1159,7 +1159,7 @@ void ResourceHandleStreamingClient::cannotShowURL(ResourceHandle*)
 
     GST_ERROR_OBJECT(src, "Cannot show URL");
 
-    GMutexLocker locker(GST_OBJECT_GET_LOCK(src));
+    GMutexLocker locker(*GST_OBJECT_GET_LOCK(src));
     uri.reset(g_strdup(src->priv->uri));
     locker.unlock();