<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body><span class="vcard"><a class="email" href="mailto:mcatanzaro@igalia.com" title="Michael Catanzaro <mcatanzaro@igalia.com>"> <span class="fn">Michael Catanzaro</span></a>
</span> changed
<a class="bz_bug_link
bz_status_NEW "
title="NEW - [GTK][Threaded Compositor] Several flaky tests due to differences in scrollbars"
href="https://bugs.webkit.org/show_bug.cgi?id=160450">bug 160450</a>
<br>
<table border="1" cellspacing="0" cellpadding="8">
<tr>
<th>What</th>
<th>Removed</th>
<th>Added</th>
</tr>
<tr>
<td style="text-align:right;">Attachment #285207 Flags</td>
<td>review?
</td>
<td>review+
</td>
</tr></table>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - [GTK][Threaded Compositor] Several flaky tests due to differences in scrollbars"
href="https://bugs.webkit.org/show_bug.cgi?id=160450#c3">Comment # 3</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - [GTK][Threaded Compositor] Several flaky tests due to differences in scrollbars"
href="https://bugs.webkit.org/show_bug.cgi?id=160450">bug 160450</a>
from <span class="vcard"><a class="email" href="mailto:mcatanzaro@igalia.com" title="Michael Catanzaro <mcatanzaro@igalia.com>"> <span class="fn">Michael Catanzaro</span></a>
</span></b>
<pre>Comment on <span class=""><a href="attachment.cgi?id=285207&action=diff" name="attach_285207" title="Speculative fix">attachment 285207</a> <a href="attachment.cgi?id=285207&action=edit" title="Speculative fix">[details]</a></span>
Speculative fix
View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=285207&action=review">https://bugs.webkit.org/attachment.cgi?id=285207&action=review</a>
<span class="quote">> 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);</span >
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.
<span class="quote">> 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.</span >
I have no clue if this is right or sensible, but OK.
<span class="quote">> Source/WebKit2/UIProcess/DrawingAreaProxyImpl.h:89
> + class DrawingMonitor {
> + WTF_MAKE_NONCOPYABLE(DrawingMonitor); WTF_MAKE_FAST_ALLOCATED;</span >
Does it really make sense to use WTF_MAKE_FAST_ALLOCATED for a class that will rarely be allocated?
<span class="quote">> Source/WebKit2/UIProcess/DrawingAreaProxyImpl.h:136
> + std::unique_ptr<DrawingMonitor> m_drawingMonitor;</span >
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.</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>