Bug 89576 - Scrolling tree should be testable
Summary: Scrolling tree should be testable
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Sami Kyöstilä
URL:
Keywords:
Depends on:
Blocks: 89216
  Show dependency treegraph
 
Reported: 2012-06-20 09:06 PDT by Sami Kyöstilä
Modified: 2013-09-05 08:32 PDT (History)
11 users (show)

See Also:


Attachments
Patch (17.27 KB, patch)
2012-06-20 09:11 PDT, Sami Kyöstilä
no flags Details | Formatted Diff | Diff
Patch (21.83 KB, patch)
2012-06-21 06:37 PDT, Sami Kyöstilä
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.