Bug 226453

Summary:

add Internals method to expose synchronous scrolling reasons for individual scrolling tree scrolling nodes

Product:

WebKit

Reporter:

Cameron McCormack (:heycam) <heycam>

Component:

Scrolling

Assignee:

Cameron McCormack (:heycam) <heycam>

Status:

RESOLVED WONTFIX

Severity:

Normal

CC:

changseok, cmarcelo, darin, esprehn+autocc, ews-watchlist, fred.wang, glenn, jamesr, kondapallykalyan, luiz, pdr, simon.fraser, tonikitoo

Priority:

Version:

WebKit Local Build

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

226399

Attachments:

Description	Flags
Patch	fred.wang: review+

Description Cameron McCormack (:heycam) 2021-05-30 22:17:58 PDT

So we can test the synchronous scrolling reasons are set correctly on scrolling tree nodes for overflow:scroll elements.

Comment 1 Cameron McCormack (:heycam) 2021-05-30 22:20:45 PDT

Created attachment 430174 [details]
Patch

Comment 2 Frédéric Wang (:fredw) 2021-06-01 06:26:15 PDT

Comment on attachment 430174 [details]
Patch

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

> Source/WebCore/testing/Internals.h:441
> +    ExceptionOr<String> synchronousScrollingReasonsForScrollableElement(Element&) const;

Is it possible to make this a const Element&?

Comment 3 Simon Fraser (smfr) 2021-06-01 10:20:38 PDT

Doesn't dumping the scrolling state tree suffice?

Comment 4 Cameron McCormack (:heycam) 2021-06-01 13:59:07 PDT

(In reply to Simon Fraser (smfr) from comment #3)
> Doesn't dumping the scrolling state tree suffice?

As I discovered from looking at other tests, yes it does.  But it includes a lot of other irrelevant information for my tests.  Seems like it's better to avoid having to update test expectations for changes in other parts of the scrolling state tree, but if you think it's better to just use the existing thing and avoid adding something new, I can do that.

Comment 5 Darin Adler 2021-06-01 14:31:09 PDT

Comment on attachment 430174 [details]
Patch

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

>> Source/WebCore/testing/Internals.h:441
>> +    ExceptionOr<String> synchronousScrollingReasonsForScrollableElement(Element&) const;
> 
> Is it possible to make this a const Element&?

Why does that matter? This is designed to be called from JavaScript.

Comment 6 Cameron McCormack (:heycam) 2021-06-01 15:22:19 PDT

(In reply to Simon Fraser (smfr) from comment #3)
> Doesn't dumping the scrolling state tree suffice?

I've updated my tests in bug 226399 to use the scrolling state tree dump so I'm not going to use this patch.