Bug 152797 - REGRESSION(r191948): [GStreamer] 4 new timeouts on layout tests.
Summary: REGRESSION(r191948): [GStreamer] 4 new timeouts on layout tests.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 150800
  Show dependency treegraph
 
Reported: 2016-01-06 12:23 PST by Carlos Alberto Lopez Perez
Modified: 2016-02-03 02:31 PST (History)
4 users (show)

See Also:


Attachments
Patch (2.22 KB, patch)
2016-02-02 06:00 PST, Carlos Garcia Campos
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Alberto Lopez Perez 2016-01-06 12:23:29 PST
Revision r191948 <http://trac.webkit.org/changeset/191948> has caused 4 new timeouts:

Regressions: Unexpected timeouts (4)
  media/video-controls-no-scripting.html [ Timeout ]
  media/video-source-error.html [ Timeout ]
  media/video-source-load.html [ Timeout ]
  media/video-source-moved.html [ Timeout ]


I have tested to build r191947 and r191948, and I can confirm that this 4 tests started to time out with r191948.


Note: media/video-controls-no-scripting.html was failing before because of bug 141835 (but it was not timing out).
Comment 1 Michael Catanzaro 2016-01-25 17:27:07 PST
Carlos, do you have time to look at this?
Comment 2 Carlos Garcia Campos 2016-01-25 23:11:21 PST
No, but I'll find it.
Comment 3 Carlos Garcia Campos 2016-02-02 06:00:34 PST
Created attachment 270485 [details]
Patch

This seems to be a problem in the very few cases that the gst bus sync handler is called form the main thread. We were avoiding scheduling to the main thread in that case, but for some reason the bus gets stalled or something. So always end the message to the main thread. This fixed the timeouts for me for media/video-source-load.html, media/video-source-moved.html and media/video-controls-no-scripting.html, the others timeout, but they also timeout for me reverting the patch.
Comment 4 Darin Adler 2016-02-02 16:52:54 PST
Comment on attachment 270485 [details]
Patch

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

> Source/WebCore/ChangeLog:9
> +        sync hanlder was called in the main thread. It seems that

typo: hanlder
Comment 5 Darin Adler 2016-02-02 16:53:24 PST
Comment on attachment 270485 [details]
Patch

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

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:1906
> +            GRefPtr<GstMessage> protectMessage(message);

protectedMessage is a better name for this local variable, since it’s not just used to protect the message, but to send it across threads as well
Comment 6 Carlos Garcia Campos 2016-02-03 02:31:41 PST
Committed r196060: <http://trac.webkit.org/changeset/196060>