Bug 192395 - Allow control over child order when adding nodes to the scrolling tree
Summary: Allow control over child order when adding nodes to the scrolling tree
Status: RESOLVED DUPLICATE of bug 176914
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Simon Fraser (smfr)
URL:
Keywords:
Depends on: 192398
Blocks:
  Show dependency treegraph
 
Reported: 2018-12-04 21:57 PST by Simon Fraser (smfr)
Modified: 2018-12-05 08:41 PST (History)
4 users (show)

See Also:


Attachments
Patch (22.33 KB, patch)
2018-12-04 21:59 PST, Simon Fraser (smfr)
fred.wang: review-
ews: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews105 for mac-sierra-wk2 (3.08 MB, application/zip)
2018-12-04 23:13 PST, Build Bot
no flags Details
Patch (same as 356581) (22.33 KB, patch)
2018-12-05 00:30 PST, Frédéric Wang (:fredw)
ews: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews107 for mac-sierra-wk2 (2.99 MB, application/zip)
2018-12-05 01:46 PST, Build Bot
no flags Details
Archive of layout-test-results from ews122 for ios-simulator-wk2 (565.33 KB, application/zip)
2018-12-05 02:08 PST, Build Bot
no flags Details
Attachment 356581, rebased on top of bug 192398 (15.16 KB, patch)
2018-12-05 06:00 PST, Frédéric Wang (:fredw)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2018-12-04 21:57:41 PST
Allow control over child order when adding nodes to the scrolling tree
Comment 1 Simon Fraser (smfr) 2018-12-04 21:59:54 PST
Created attachment 356581 [details]
Patch
Comment 2 Build Bot 2018-12-04 23:13:26 PST
Comment on attachment 356581 [details]
Patch

Attachment 356581 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/10274669

New failing tests:
fast/visual-viewport/tiled-drawing/zoomed-fixed-scrolling-layers-state.html
Comment 3 Build Bot 2018-12-04 23:13:27 PST
Created attachment 356584 [details]
Archive of layout-test-results from ews105 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6
Comment 4 Frédéric Wang (:fredw) 2018-12-05 00:17:11 PST
This seems similar to what I tried in https://bugs.webkit.org/attachment.cgi?id=327061&action=review
Comment 5 Frédéric Wang (:fredw) 2018-12-05 00:27:01 PST
Trying to fix the EWS build error on iOS:

https://trac.webkit.org/changeset/238889/webkit
Comment 6 Frédéric Wang (:fredw) 2018-12-05 00:30:38 PST
Created attachment 356589 [details]
Patch (same as 356581)

resending to EWS
Comment 7 Build Bot 2018-12-05 01:46:09 PST
Comment on attachment 356589 [details]
Patch (same as 356581)

Attachment 356589 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/10275650

New failing tests:
fast/visual-viewport/tiled-drawing/zoomed-fixed-scrolling-layers-state.html
Comment 8 Build Bot 2018-12-05 01:46:11 PST
Created attachment 356591 [details]
Archive of layout-test-results from ews107 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6
Comment 9 Build Bot 2018-12-05 02:07:59 PST
Comment on attachment 356589 [details]
Patch (same as 356581)

Attachment 356589 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/10275657

Number of test failures exceeded the failure limit.
Comment 10 Build Bot 2018-12-05 02:08:01 PST
Created attachment 356592 [details]
Archive of layout-test-results from ews122 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews122  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6
Comment 11 Frédéric Wang (:fredw) 2018-12-05 04:49:37 PST
Comment on attachment 356589 [details]
Patch (same as 356581)

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

> Source/WebCore/page/scrolling/ScrollingStateTree.h:-30
> -#include "ScrollingStateFrameScrollingNode.h"

OK, I thought what I fixed in comment 5 was again due to rotating unified build but it seems it was actually this change. I guess several cleanup changes in this patch could be handled outside to facilitate review.

> Source/WebKit/Shared/RemoteLayerTree/RemoteScrollingCoordinatorTransaction.cpp:416
> +        m_scrollingStateTree->attachNode(nodeType, nodeID, parentNodeID, i);

This seems wrong since numNodes is the size of the whole scrolling state tree not of the child list. I guess that's why I had tried to encode the child index in https://bugs.webkit.org/attachment.cgi?id=327061&action=review
Comment 12 Frédéric Wang (:fredw) 2018-12-05 06:00:46 PST
Created attachment 356599 [details]
Attachment 356581 [details], rebased on top of bug 192398

This is the patch rebased on top of bug 192398, but I think my comment 11 regarding RemoteScrollingCoordinatorTransaction still holds.
Comment 13 Simon Fraser (smfr) 2018-12-05 08:27:08 PST
Sorry for the duplicate patch; let's work on your version.

*** This bug has been marked as a duplicate of bug 176914 ***
Comment 14 Frédéric Wang (:fredw) 2018-12-05 08:41:42 PST
(In reply to Simon Fraser (smfr) from comment #13)
> Sorry for the duplicate patch; let's work on your version.
> 
> *** This bug has been marked as a duplicate of bug 176914 ***

OK, my patch is from one year ago though so probably we could just work from your version. BTW, I extracted the minor refactoring parts in bug 192398.