[Webkit-unassigned] [Bug 160450] [GTK][Threaded Compositor] Several flaky tests due to differences in scrollbars

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=160450

--- Comment #4 from Carlos Garcia Campos <cgarcia at igalia.com> ---
(In reply to comment #3)
> Comment on attachment 285207 [details]
> Speculative fix
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=285207&action=review

Thanks for the review. I think Zan was looking at the actual cause of the scrollbars drawing delay, so I'll wait until he confirms this is ok.

> > Source/WebKit2/UIProcess/DrawingAreaProxyImpl.cpp:359
> > +#if PLATFORM(GTK)
> > +    g_signal_connect_swapped(m_webPage.viewWidget(), "draw", reinterpret_cast<GCallback>(webViewDrawCallback), this);
> > +    m_timer.startOneShot(1);
> 
> So we have a DrawingMonitor class that doesn't actually monitor drawing
> unless PLATFORM(GTK). Consider guarding the entire class and putting an
> #ifdef in DrawingAreaProxyImpl::dispatchAfterEnsuringDrawing so it's not
> used at all on other platforms.

This file is actually only used by GTK anyway, what is protected by GTK is the actual use of GTK API. 

> > Source/WebKit2/UIProcess/DrawingAreaProxyImpl.cpp:382
> > +    // We wait up to 1 second for draw events. If there are several draw events queued quickly,
> > +    // we want to wait until all of them have been processed, so after receiving a draw, we wait
> > +    // up to 100ms for the next one or stop.
> 
> I have no clue if this is right or sensible, but OK.

Note that this patch is just an experiment to see if flaky tests are reduced in the bots, so I just took those numbers arbitrarily. I didn't want to delay tests for more than 1 second, and if we could finish earlier we do.

> > Source/WebKit2/UIProcess/DrawingAreaProxyImpl.h:89
> > +    class DrawingMonitor {
> > +        WTF_MAKE_NONCOPYABLE(DrawingMonitor); WTF_MAKE_FAST_ALLOCATED;
> 
> Does it really make sense to use WTF_MAKE_FAST_ALLOCATED for a class that
> will rarely be allocated?

I don't know, I guess it's harmless.

> > Source/WebKit2/UIProcess/DrawingAreaProxyImpl.h:136
> > +    std::unique_ptr<DrawingMonitor> m_drawingMonitor;
> 
> This use of std::unique_ptr appears to be unnecessary, since you're not
> using it for polymorphism, you don't really need the null state, and you
> don't appear to need the lazy initialization. You can just keep a
> DrawingMonitor member, then you don't need to worry about creating it in
> DrawingAreaProxyImpl::dispatchAfterEnsuringDrawing or checking that it's not
> null.

This is only used by tests, so when no running tests, it's better to not have that monitor thing created, that's why I create it on demand.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20160806/2ba2909b/attachment.html>