Bug 89576

Summary: Scrolling tree should be testable
Product: WebKit Reporter: Sami Kyöstilä <skyostil>
Component: Tools / TestsAssignee: Sami Kyöstilä <skyostil>
Status: RESOLVED WONTFIX    
Severity: Normal CC: abarth, andersca, cc-bugs, dglazkov, fishd, jamesr, simon.fraser, tkent, tkent+wkapi, tonikitoo, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 89216    
Attachments:
Description Flags
Patch
none
Patch none

Description Sami Kyöstilä 2012-06-20 09:06:45 PDT
Scrolling tree should be testable
Comment 1 Sami Kyöstilä 2012-06-20 09:11:14 PDT
Created attachment 148582 [details]
Patch
Comment 2 WebKit Review Bot 2012-06-20 09:16:13 PDT
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.
Comment 3 Sami Kyöstilä 2012-06-20 09:29:51 PDT
Here's my original patch from bug 89216. It has the problem that the output of scrollingTreeAsText() is very specific to Chromium's implementation of the scrolling tree.

Is there a way to make that output more portable, or should we use some entirely other mechanism to test ScrollingCoordinator?
Comment 4 James Robinson 2012-06-20 10:41:27 PDT
Simon's feedback on bug 89216 was that we should test using the ScrollingTree nodes, but I disagree.  The ScrollingTree notion as it exists is very mac-specific (or in particular CoreAnimation-specific) and is not something that can be supported by the chromium or Qt compositor, so I do not think it is appropriate to rely on for cross-platform tests.  If ScrollingTree was factored differently - perhaps if it was more of a data type rather than encapsulating a specific threading model - then maybe we could use it elsewhere, but today that's not an option.

An alternate approach to dumping a tree would be to dump a serialization of ScrollingCoordinator calls.  That part of the interface is shared everywhere.
Comment 5 Sami Kyöstilä 2012-06-21 06:31:44 PDT
(In reply to comment #4)
> An alternate approach to dumping a tree would be to dump a serialization of ScrollingCoordinator calls.  That part of the interface is shared everywhere.

I like this idea, although it seems like these logs could grow large fairly quickly depending on the complexity of the page. For instance, even for a blank page the non-fast scrollable region is calculated 4 times.

Since we only really care about the end state in tests, I propose we start by dumping just the bits of information we need and work from there. I'll upload a patch that gives the following both with Chromium and the ScrollingTree implementations:

 - Whether non-main-thread scrolling is supported with the current page
   contents.
 - Whether there are registered wheel event handlers on the page.
 - The non-fast scrollable region of the page.
 - Number of fixed position layer containers and layers.
Comment 6 Sami Kyöstilä 2012-06-21 06:37:56 PDT
Created attachment 148787 [details]
Patch

- scrollingTreeAsText() now only returns basic high-level facts about the tree.
- Added test to verify current (buggy) behavior of fixed position containers.
Comment 7 Sami Kyöstilä 2013-04-09 02:27:39 PDT
The scrolling tree is already being tested by layout tests.
Comment 8 Zan Dobersek 2013-09-05 08:32:35 PDT
Comment on attachment 148787 [details]
Patch

The work on this bug has ceased, removing the r? flag on the patch.