[webkit-reviews] review granted: [Bug 181575] Make scrolling to the focused element async : [Attachment 332325] Changes the behavior

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted Ryosuke Niwa
<rniwa at webkit.org>'s request for review:
Bug 181575: Make scrolling to the focused element async
https://bugs.webkit.org/show_bug.cgi?id=181575

Attachment 332325: Changes the behavior

https://bugs.webkit.org/attachment.cgi?id=332325&action=review




--- Comment #7 from Simon Fraser (smfr) <simon.fraser at apple.com> ---
Comment on attachment 332325
  --> https://bugs.webkit.org/attachment.cgi?id=332325
Changes the behavior

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

> Source/WebCore/page/FrameView.cpp:270
> +    m_shouldScrollToFocusedElement = false;
> +    m_delayedScrollToFocusedElementTimer.stop();

If it's always true that m_shouldScrollToFocusedElement ==
m_delayedScrollToFocusedElementTimer.stop.isActive(), then you can remove the
bool.

> Source/WebCore/page/FrameView.cpp:2274
> +    scrollToFocusedElementTimerFired();

I'd prefer to see the guts of scrollToFocusedElementTimerFired() in a new
function that you call here, rather than confusingly calling
scrollToFocusedElementTimerFired().