[webkit-reviews] review denied: [Bug 191004] [GTK] Layout test media/track/track-cue-missing.html is failing : [Attachment 428378] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu May 13 09:10:34 PDT 2021
Alicia Boya García <aboya at igalia.com> has denied Enrique Ocaña
<eocanha at igalia.com>'s request for review:
Bug 191004: [GTK] Layout test media/track/track-cue-missing.html is failing
https://bugs.webkit.org/show_bug.cgi?id=191004
Attachment 428378: Patch
https://bugs.webkit.org/attachment.cgi?id=428378&action=review
--- Comment #14 from Alicia Boya García <aboya at igalia.com> ---
Comment on attachment 428378
--> https://bugs.webkit.org/attachment.cgi?id=428378
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=428378&action=review
>
Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:2859
> + m_pendingTask = PendingTask(WTFMove(task), time, m_pendingTaskMutex);
You're constructing a new PendingTask -- its constructor takes the mutex,
initializes its members while holding its mutex (totally unnecessary, since
during the constructor execution no other code has received a reference to that
instance). Then, the constructor withdraws the mutex, and this code runs
operator=() on PendingTask, which -- now without any mutex protecting anything
-- swaps the data members of the new instance with the members of the old
instance.
r-
> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h:308
> + void invalidateCachedPositionOnNextIteration() const;
invalidateCachedPositionOnNextIteration() can be private, since it's only used
by MediaPlayerPrivateGStreamer. Note playbackPosition() is also private. Both
are things that could be changed easily if ever needed.
More information about the webkit-reviews
mailing list