[webkit-reviews] review granted: [Bug 192977] [GStreamer] Add sharedBuffer utility to GstMappedBuffer, and a testsuite : [Attachment 358495] Patch

[bugzilla-daemon at webkit.org]

Xabier Rodríguez Calvar <calvaris at igalia.com> has granted Charlie Turner
<cturner at igalia.com>'s request for review:
Bug 192977: [GStreamer] Add sharedBuffer utility to GstMappedBuffer, and a
testsuite
https://bugs.webkit.org/show_bug.cgi?id=192977

Attachment 358495: Patch

https://bugs.webkit.org/attachment.cgi?id=358495&action=review




--- Comment #5 from Xabier Rodríguez Calvar <calvaris at igalia.com> ---
Comment on attachment 358495
  --> https://bugs.webkit.org/attachment.cgi?id=358495
Patch

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

> Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.cpp:365
> +    m_cachedSharedBuffer = nullptr;

The smart pointer will to its magic, I don't think you need this.

> Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.cpp:375
> +    if (!m_isValid || m_info.flags & GST_MAP_WRITE)
> +	   return nullptr;

I am quite certain that we want to ASSERT_NOT_REACHED before returning the
nullptr

> Tools/TestWebKitAPI/Tests/WebCore/gstreamer/GStreamerTest.cpp:42
> +	   gst_init(NULL, NULL);

nullptr

> Tools/TestWebKitAPI/Tests/WebCore/gstreamer/GstMappedBuffer.cpp:95
> +    EXPECT_EQ(sharedBuf.get(), mappedBuf.sharedBuffer().get());
> +    EXPECT_EQ(sharedBuf.get(), mappedBuf.sharedBuffer().get());

What's the purpose of running the same line twice?