Bug 203839 - Integrate scroll event into HTML5 event loop
Summary: Integrate scroll event into HTML5 event loop
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Scrolling (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords: InRadar
Depends on:
Blocks: 202843
  Show dependency tree / graph
 
Reported: 2019-11-04 19:38 PST by Ryosuke Niwa
Modified: 2019-11-14 23:02 PST (History)
16 users (show)

See Also:

Attachments
WIP (9.59 KB, patch)
2019-11-05 20:26 PST, Ryosuke Niwa 		no flags Details | Formatted Diff | Diff
WIP2 (10.13 KB, patch)
2019-11-05 21:25 PST, Ryosuke Niwa 		no flags Details | Formatted Diff | Diff
WIP3 (12.85 KB, patch)
2019-11-05 22:55 PST, Ryosuke Niwa 		no flags Details | Formatted Diff | Diff
Patch (23.19 KB, patch)
2019-11-06 16:40 PST, Ryosuke Niwa 		simon.fraser: review+
Details | Formatted Diff | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Ryosuke Niwa 2019-11-04 19:38:36 PST 
Scroll event should happen during Page::updateRendering.
Comment 1 Radar WebKit Bug Importer 2019-11-04 19:39:01 PST 
<rdar://problem/56890922>
Comment 2 Ryosuke Niwa 2019-11-05 20:26:26 PST 
Created attachment 382889 [details]
WIP
Comment 3 Ryosuke Niwa 2019-11-05 21:25:43 PST 
Created attachment 382891 [details]
WIP2
Comment 4 Ryosuke Niwa 2019-11-05 22:55:56 PST 
Created attachment 382897 [details]
WIP3
Comment 5 Ryosuke Niwa 2019-11-06 16:40:36 PST 
Created attachment 382982 [details]
Patch
Comment 6 Ryosuke Niwa 2019-11-07 12:06:44 PST 
Ping reviewers.
Comment 7 Simon Fraser (smfr) 2019-11-07 12:55:42 PST 
Comment on attachment 382982 [details]
Patch

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

> Source/WebCore/dom/Document.cpp:4018
> +    m_needsVisualViewportScrollEvent = true;
> +    scheduleTimedRenderingUpdate();

Can this avoid calling scheduleTimedRenderingUpdate if m_needsVisualViewportScrollEvent is already true?

> Source/WebCore/dom/Document.cpp:4041
> +            window->visualViewport().dispatchEvent(Event::create(eventNames().scrollEvent, Event::CanBubble::No, Event::IsCancelable::No));

Is window->visualViewport() always non-null?
Comment 8 Ryosuke Niwa 2019-11-07 13:33:22 PST 
(In reply to Simon Fraser (smfr) from comment #7)
> Comment on attachment 382982 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=382982&action=review
> 
> > Source/WebCore/dom/Document.cpp:4018
> > +    m_needsVisualViewportScrollEvent = true;
> > +    scheduleTimedRenderingUpdate();
> 
> Can this avoid calling scheduleTimedRenderingUpdate if
> m_needsVisualViewportScrollEvent is already true?

Done.

> > Source/WebCore/dom/Document.cpp:4041
> > +            window->visualViewport().dispatchEvent(Event::create(eventNames().scrollEvent, Event::CanBubble::No, Event::IsCancelable::No));
> 
> Is window->visualViewport() always non-null?

Yes. We also never reach here unless VisualViewport::update is called.
Comment 9 Ryosuke Niwa 2019-11-07 14:18:39 PST 
Committed r252205: <https://trac.webkit.org/changeset/252205>
Comment 10 Aakash Jain 2019-11-09 15:11:23 PST 
(In reply to Ryosuke Niwa from comment #9)
> Committed r252205: <https://trac.webkit.org/changeset/252205>
One of the added test fast/events/scroll-subframe-in-rendering-update.html is failing consistently on iOS, and slowing down ios-wk2 queue. Tracked in bug 204045.