<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body><span class="vcard"><a class="email" href="mailto:cgarcia@igalia.com" title="Carlos Garcia Campos <cgarcia@igalia.com>"> <span class="fn">Carlos Garcia Campos</span></a>
</span> changed
<a class="bz_bug_link
bz_status_NEW "
title="NEW - [GTK] Add kinetic scrolling"
href="https://bugs.webkit.org/show_bug.cgi?id=155750">bug 155750</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 #281977 Flags</td>
<td>
</td>
<td>review+, commit-queue-
</td>
</tr></table>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - [GTK] Add kinetic scrolling"
href="https://bugs.webkit.org/show_bug.cgi?id=155750#c34">Comment # 34</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - [GTK] Add kinetic scrolling"
href="https://bugs.webkit.org/show_bug.cgi?id=155750">bug 155750</a>
from <span class="vcard"><a class="email" href="mailto:cgarcia@igalia.com" title="Carlos Garcia Campos <cgarcia@igalia.com>"> <span class="fn">Carlos Garcia Campos</span></a>
</span></b>
<pre>Comment on <span class=""><a href="attachment.cgi?id=281977&action=diff" name="attach_281977" title="[GTK] Add kinetic scrolling">attachment 281977</a> <a href="attachment.cgi?id=281977&action=edit" title="[GTK] Add kinetic scrolling">[details]</a></span>
[GTK] Add kinetic scrolling
View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=281977&action=review">https://bugs.webkit.org/attachment.cgi?id=281977&action=review</a>
Looks good to me, thanks! We just need a wk2 owner to approve the changes in WebPageProxy
<span class="quote">> Source/WebCore/platform/ScrollAnimationKinetic.h:74
> + Optional<PerAxisData> m_horizontalData { Nullopt };
> + Optional<PerAxisData> m_verticalData { Nullopt };</span >
You don't need to initialize these, the constructor already sets is to Nullopt.
<span class="quote">> Source/WebCore/platform/gtk/ScrollAnimatorGtk.cpp:60
> + updatePosition(WTFMove(position));
> +#if ENABLE(SMOOTH_SCROLLING)
> + if (m_smoothAnimation)
> + m_smoothAnimation->setCurrentPosition(position);
> +#endif</span >
hmm, this is a problem now that I see it, this is using position after being moved. You can probably just change the order, and call updatePosition after setCurrentPosition
<span class="quote">> Source/WebCore/platform/gtk/ScrollAnimatorGtk.cpp:107
> +#if ENABLE(SMOOTH_SCROLLING)
> + if (m_smoothAnimation)
> + m_smoothAnimation->setCurrentPosition(position);
> +#endif
> +
> + updatePosition(WTFMove(position));</span >
like you do here
<span class="quote">> Source/WebKit2/UIProcess/WebPageProxy.cpp:1893
> +bool WebPageProxy::shouldProcessWheelEventNow(const WebWheelEvent& event)
> +{
> +#if PLATFORM(GTK)
> + // Don't queue events representing a non-trivial scrolling phase to
> + // avoid having them trapped in the queue, potentially preventing a
> + // scrolling session to beginning or end correctly.
> + // This is only needed by platforms whose WebWheelEvent has this phase
> + // information (Cocoa and GTK+) but Cocoa was fine without it.
> + if (event.phase() == WebWheelEvent::Phase::PhaseNone
> + || event.phase() == WebWheelEvent::Phase::PhaseChanged
> + || event.momentumPhase() == WebWheelEvent::Phase::PhaseNone
> + || event.momentumPhase() == WebWheelEvent::Phase::PhaseChanged)
> + return true;
> +#else
> + UNUSED_PARAM(event);
> +#endif
> + if (m_wheelEventQueue.size() >= wheelEventQueueSizeThreshold)
> + return true;
> + return false;
> +}</span >
This is in cross-platform code, we need approval from a WebKit2 owner
<span class="quote">> Source/WebKit2/UIProcess/WebPageProxy.h:1469
> + bool shouldProcessWheelEventNow(const WebWheelEvent&);</span >
This could probably be const</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>