Bug 203839

Summary: Integrate scroll event into HTML5 event loop
Product: WebKit Reporter: Ryosuke Niwa <rniwa>
Component: ScrollingAssignee: Ryosuke Niwa <rniwa>
Status: RESOLVED FIXED    
Severity: Normal CC: aakash_jain, cdumez, dbates, esprehn+autocc, ews-watchlist, fred.wang, glenn, kangil.han, koivisto, kondapallykalyan, mifenton, pdr, simon.fraser, tkent, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=204045
https://bugs.webkit.org/show_bug.cgi?id=204062
https://bugs.webkit.org/show_bug.cgi?id=204219
Bug Depends on:    
Bug Blocks: 202843    
Attachments:
Description Flags
WIP
none
WIP2
none
WIP3
none
Patch simon.fraser: review+

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.