Summary: | [GTK] Test fast/events/message-port-postMessage-recursive.html times out | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Carlos Garcia Campos <cgarcia> | ||||||||
Component: | WebKitGTK | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | benjamin, bugs-noreply, cdumez, cmarcelo, commit-queue, dbates, mcatanzaro | ||||||||
Priority: | P2 | Keywords: | Gtk, LayoutTestFailure | ||||||||
Version: | WebKit Local Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Carlos Garcia Campos
2017-02-19 02:08:21 PST
Created attachment 302073 [details]
Patch
I wonder what this will break! (In reply to comment #2) > I wonder what this will break! You always so negative, I wonder what other things will fix. Committed r212622: <http://trac.webkit.org/changeset/212622> Reverted r212622 for reason: Caused several test failures Committed r212627: <http://trac.webkit.org/changeset/212627> So, yes, this introduced a few failures (~10) most of them image only failures. I'm still not sure if they are actual regressions, or the patch actually revealed those bugs. But timer priorities is tricky thing that could also impact the performance, so we should think more carefully about it. (In reply to comment #6) > So, yes, this introduced a few failures (~10) most of them image only > failures. I'm still not sure if they are actual regressions, or the patch > actually revealed those bugs. But timer priorities is tricky thing that > could also impact the performance, so we should think more carefully about > it. I get to be right every once in a while! I'm actually most concerned about the original test, because it's testing the behavior of the timers themselves, and it shows us that our timers are definitely broken without your patch. But clearly we need to think about this some more and understand what's going on with the other tests. Timers are hard. Created attachment 302252 [details]
Patch
Comment on attachment 302252 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=302252&action=review > Source/WTF/wtf/glib/MainThreadGLib.cpp:38 > +class MainThreadDispatcher { Please add a comment. You could take it from your commit message. > Source/WTF/wtf/glib/MainThreadGLib.cpp:43 > + m_timer.setPriority(G_PRIORITY_HIGH_IDLE + 20); Too fragile. It has to be exactly equal to WebCore timers? Then you should fine some common header to define it in. (In reply to comment #9) > Comment on attachment 302252 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=302252&action=review > > > Source/WTF/wtf/glib/MainThreadGLib.cpp:38 > > +class MainThreadDispatcher { > > Please add a comment. You could take it from your commit message. Ok. > > Source/WTF/wtf/glib/MainThreadGLib.cpp:43 > > + m_timer.setPriority(G_PRIORITY_HIGH_IDLE + 20); > > Too fragile. It has to be exactly equal to WebCore timers? Then you should > fine some common header to define it in. Not really, I'm just conservative here and using at least the same priority. I still think we should re-think all the timer priorities anyway. Created attachment 302260 [details]
Patch
Committed r212814: <http://trac.webkit.org/changeset/212814> |