[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

Michael Catanzaro <mcatanzaro at igalia.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #285207|review?                     |review+
              Flags|                            |

--- Comment #3 from Michael Catanzaro <mcatanzaro at igalia.com> ---
Comment on attachment 285207
  --> https://bugs.webkit.org/attachment.cgi?id=285207
Speculative fix

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

> 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.

> 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.

> 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?

> 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.

-- 
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/20160805/20b598fd/attachment.html>