Title: [293727] trunk
Revision
293727
Author
[email protected]
Date
2022-05-03 09:02:48 -0700 (Tue, 03 May 2022)

Log Message

Web Inspector: Importing a timeline leaves timeline overview non-scrollable/non-zoomable until windows is resized
https://bugs.webkit.org/show_bug.cgi?id=239880

Reviewed by Devin Rousso.

Source/WebInspectorUI:

Cancelling an in-progress layout (or the layout of a child of a view in middle of laying out) led to
`_dirtyDescendantsCount` being in an inconsistent state. Views had their `_dirtyDescendantsCount` cleared before
they are laid out, so every non-zero `_dirtyDescendantsCount` of a view in the process of being laid out meant
that during layout `needsLayout()` was called to dirty that view or one of its children. In this case, a
previous cancellation meant that the TimelineOverview ended up in an inconsistent state where its parent view
tree was not aware that it needed layout because it reached a zero `_dirtyDescendantsCount` while walking down
the view hierarchy, which meant that the TimelineOverview was perpetually stuck until the entire view hierarchy
needed re-laid out due to a window resize or similar event.

Our current accounting for `_dirtyDescendantsCount` in View makes assumptions that are not always true during
layout. Specifically that parents of views undergoing layout will have a non-zero `_dirtyDescendantsCount` still
from which to subtract in `_cancelScheduledLayoutForView`. This wasn't always true because as soon as we began
laying out a parent view, we cleared both the `_dirty` flag as well as zeroed out `_dirtyDescendantsCount`. In
the case of cancelling a layout from within a layout, the cancellation wouldn't actually take effect anyways,
and since `cancelLayout` was only ever invoked from `updateLayout`, the relevant layout flags should instead be
cleared as part of actually performing the layout. `cancelLayout` at most saved us a rAF callback only to
realize we no longer have any views in need of layout, but this benefit was negligible, and in general we should
prefer not to be calling `updateLayout` for performance reasons anyways.

This patch improves the accounting around `_dirtyDescendantsCount`, making sure to decrement it as we perform
layout, instead of all at once. This reduces our reliance on assumptions about who will modify the
`_dirtyDescendantsCount` and at what times relative to layout.

The only time in this patch that we do not do a +1/-1 adjustment to the layout count is in the special case of
attaching/detaching a view. In that case, the parent tree from which we are detaching the view will have the
`_dirtyDescendantsCount` of the detached parent adjusted by the `_dirtyDescendantsCount` of the former child
view. This is done by `_setSelfAndDescendantsNotDirty`, which allows us to avoid an exponential walking of the
tree for this otherwise self-contained operation (we don't call into any overridable functions that could modify
the `_dirty` flag or `_dirtyDescendantsCount`).

After we have cleaned up the former parent branch of the tree, we mark the newly attached view as needing
layout, which will then ensure the child view gets laid out again by marking it as `dirty` and incrementing its
new parent branch of the tree's `_dirtyDescendantsCount`.

* UserInterface/Views/View.js:
(WI.View.prototype.insertSubviewBefore):
- Add assertion that the view is not currently a child of a different view in addition to the existing check
that a view is not already a child of the target view.

(WI.View.prototype._setDirty):
- New utility method through which (almost) all marking of dirty/not dirty should be done in order to ensure
that `_dirtyDescendantsCount` is correctly adjusted.

(WI.View.prototype._didMoveToParent):
- Mark the view and all subviews as not dirty before removing it from its previous parent in order to adjust
`_dirtyDescendantsCount` appropriately, and then mark the view as dirty once attached to its new parent.
- Inline the logic from WI.View.prototype._didMoveToWindow in order to also handle marking children as not dirty
and having a zeroed `_dirtyDescendantsCount`.
- This method contains the exception to the rule that _dirtyDescendantsCount is always incremented/decremented
by one. Here we are able to optimize away the need to walk the entire parent tree for each subview/subviews of
subviews/etc. because the operation takes place all at once with no overridable function being called where we
have to worry about mutations to `_dirty`/`_dirtyDescendantsCount`.

(WI.View.prototype._layoutSubtree):
(WI.View._visitViewTreeForLayout):
- Don't zero out the `_dirtyDescendantsCount`, and instead use the new `_setDirty` helper.

(WI.View._scheduleLayoutForView):
- Don't zero out the `_dirtyDescendantsCount`, and instead use the new `_setDirty` helper.
- Mark the view as dirty after checking if its attached a view so that detached views have no dirty state until
they are attached, at which point the root of the previous detached subtree will be marked as dirty.
- Drive-by change to use a for-loop instead of a while-loop to avoid `Array.prototype.shift()`.

(WI.View.prototype.updateLayout):
(WI.View.prototype.cancelLayout): Deleted.
(WI.View._cancelScheduledLayoutForView): Deleted.
- Remove the concept of "canceling" layout, since it is only used by `updateLayout`, and the call to
`_layoutSubtree` in `updateLayout` will cause the view, dirty or not, to be marked as not dirty.

LayoutTests:

* inspector/view/asynchronous-layout-expected.txt:
* inspector/view/asynchronous-layout.html:
- Added test case for calls to `updateLayout()` during asynchronous `layout()`.
- Remove test case for removed `View.prototype.cancelLayout`.

* inspector/view/basics-expected.txt:
* inspector/view/basics.html:
- Added test case to verify that `_dirtyDescendantsCount` is an expected value at various points of
attaching/detaching views.

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (293726 => 293727)


--- trunk/LayoutTests/ChangeLog	2022-05-03 16:00:59 UTC (rev 293726)
+++ trunk/LayoutTests/ChangeLog	2022-05-03 16:02:48 UTC (rev 293727)
@@ -1,3 +1,20 @@
+2022-05-03  Patrick Angle  <[email protected]>
+
+        Web Inspector: Importing a timeline leaves timeline overview non-scrollable/non-zoomable until windows is resized
+        https://bugs.webkit.org/show_bug.cgi?id=239880
+
+        Reviewed by Devin Rousso.
+
+        * inspector/view/asynchronous-layout-expected.txt:
+        * inspector/view/asynchronous-layout.html:
+        - Added test case for calls to `updateLayout()` during asynchronous `layout()`.
+        - Remove test case for removed `View.prototype.cancelLayout`.
+
+        * inspector/view/basics-expected.txt:
+        * inspector/view/basics.html:
+        - Added test case to verify that `_dirtyDescendantsCount` is an expected value at various points of
+        attaching/detaching views.
+
 2022-05-03  Antti Koivisto  <[email protected]>
 
         [CSS Cascade Layers] Endless recursion with revert-layer in other tree context

Modified: trunk/LayoutTests/inspector/view/asynchronous-layout-expected.txt (293726 => 293727)


--- trunk/LayoutTests/inspector/view/asynchronous-layout-expected.txt	2022-05-03 16:00:59 UTC (rev 293726)
+++ trunk/LayoutTests/inspector/view/asynchronous-layout-expected.txt	2022-05-03 16:02:48 UTC (rev 293727)
@@ -20,6 +20,30 @@
 PASS: View should update its layout.
 PASS: View should not have a pending layout.
 
+-- Running test case: View.SyncronousLayoutDuringAsyncronousLayout
+PASS: Root view should have 2 dirty descendants.
+PASS: Parent view should have 1 dirty descendant.
+PASS: Child view should have 0 dirty descendants.
+PASS: View should have a pending layout.
+Child view completed a layout.
+PASS: Root view should have 1 dirty descendant.
+PASS: Parent view should have 0 dirty descendants.
+PASS: Child view should have 0 dirty descendants.
+PASS: Parent view should have started a layout.
+PASS: Child view should have completed 1 layout.
+Child view completed a layout.
+PASS: Root view should have 0 dirty descendants.
+PASS: Parent view should have 0 dirty descendants.
+PASS: Child view should have 0 dirty descendants.
+PASS: Parent view should have started a layout.
+PASS: Child view should have completed 2 layouts.
+Parent view completed a layout.
+PASS: Root view should have 0 dirty descendants.
+PASS: Root view should have 0 dirty descendants.
+PASS: Root view should have 0 dirty descendants.
+PASS: Parent view should have completed 1 layout.
+PASS: Parent view should not have a pending layout.
+
 -- Running test case: View.needsLayout.propogateToSubview
 Schedule parent view update.
 Layout complete.
@@ -26,9 +50,3 @@
 PASS: Chlid view should do an initial layout.
 PASS: Child view should update its layout.
 
--- Running test case: View.cancelLayout
-Cancel automatic layout.
-PASS: View should not have a pending layout.
-Cancel scheduled layout.
-PASS: View should not have a pending layout.
-

Modified: trunk/LayoutTests/inspector/view/asynchronous-layout.html (293726 => 293727)


--- trunk/LayoutTests/inspector/view/asynchronous-layout.html	2022-05-03 16:00:59 UTC (rev 293726)
+++ trunk/LayoutTests/inspector/view/asynchronous-layout.html	2022-05-03 16:02:48 UTC (rev 293727)
@@ -62,6 +62,65 @@
     });
 
     suite.addTestCase({
+        name: "View.SyncronousLayoutDuringAsyncronousLayout",
+        test(resolve, reject) {
+            let UpdateLayoutTestView = class UpdateLayoutTestView extends WI.TestView {
+                layout() {
+                    super.layout();
+
+                    this.subviews[0].updateLayout();
+                }
+            }
+
+            let rootView = WI.View.rootView();
+            let parentView = new UpdateLayoutTestView;
+            let childView = new WI.TestView;
+
+            rootView.addSubview(parentView);
+            parentView.addSubview(childView);
+
+            InspectorTest.expectEqual(rootView._dirtyDescendantsCount, 2, "Root view should have 2 dirty descendants.");
+            InspectorTest.expectEqual(parentView._dirtyDescendantsCount, 1, "Parent view should have 1 dirty descendant.");
+            InspectorTest.expectEqual(childView._dirtyDescendantsCount, 0, "Child view should have 0 dirty descendants.");
+            InspectorTest.expectThat(parentView.layoutPending, "View should have a pending layout.");
+
+            childView.evaluateAfterLayout(() => {
+                InspectorTest.log("Child view completed a layout.");
+
+                InspectorTest.expectEqual(rootView._dirtyDescendantsCount, 0, "Root view should have 1 dirty descendant.");
+                InspectorTest.expectEqual(parentView._dirtyDescendantsCount, 0, "Parent view should have 0 dirty descendants.");
+                InspectorTest.expectEqual(childView._dirtyDescendantsCount, 0, "Child view should have 0 dirty descendants.");
+
+                InspectorTest.expectEqual(parentView.layoutCount, 1, "Parent view should have started a layout.");
+                InspectorTest.expectEqual(childView.layoutCount, 1, "Child view should have completed 1 layout.");
+
+                childView.evaluateAfterLayout(() => {
+                    InspectorTest.log("Child view completed a layout.");
+                    InspectorTest.expectEqual(rootView._dirtyDescendantsCount, 0, "Root view should have 0 dirty descendants.");
+                    InspectorTest.expectEqual(parentView._dirtyDescendantsCount, 0, "Parent view should have 0 dirty descendants.");
+                    InspectorTest.expectEqual(childView._dirtyDescendantsCount, 0, "Child view should have 0 dirty descendants.");
+
+                    InspectorTest.expectEqual(parentView.layoutCount, 1, "Parent view should have started a layout.");
+                    InspectorTest.expectEqual(childView.layoutCount, 2, "Child view should have completed 2 layouts.");
+                });
+            });
+
+            parentView.evaluateAfterLayout(() => {
+                InspectorTest.log("Parent view completed a layout.");
+
+                InspectorTest.expectEqual(rootView._dirtyDescendantsCount, 0, "Root view should have 0 dirty descendants.");
+                InspectorTest.expectEqual(parentView._dirtyDescendantsCount, 0, "Root view should have 0 dirty descendants.");
+                InspectorTest.expectEqual(childView._dirtyDescendantsCount, 0, "Root view should have 0 dirty descendants.");
+
+                InspectorTest.expectEqual(parentView.layoutCount, 1, "Parent view should have completed 1 layout.");
+                InspectorTest.expectFalse(parentView.layoutPending, "Parent view should not have a pending layout.");
+
+                resolve();
+            });
+        }
+    });
+
+    suite.addTestCase({
         name: "View.needsLayout.propogateToSubview",
         test(resolve, reject) {
             let parent = new WI.TestView;
@@ -80,24 +139,6 @@
         }
     });
 
-    suite.addTestCase({
-        name: "View.cancelLayout",
-        test(resolve, reject) {
-            let view = new WI.TestView;
-            WI.View.rootView().addSubview(view);
-
-            InspectorTest.log("Cancel automatic layout.");
-            view.cancelLayout();
-            InspectorTest.expectFalse(view.layoutPending, "View should not have a pending layout.");
-
-            InspectorTest.log("Cancel scheduled layout.");
-            view.needsLayout();
-            view.cancelLayout();
-            InspectorTest.expectFalse(view.layoutPending, "View should not have a pending layout.");
-            resolve();
-        }
-    });
-
     suite.runTestCasesAndFinish();
 }
 </script>

Modified: trunk/LayoutTests/inspector/view/basics-expected.txt (293726 => 293727)


--- trunk/LayoutTests/inspector/view/basics-expected.txt	2022-05-03 16:00:59 UTC (rev 293726)
+++ trunk/LayoutTests/inspector/view/basics-expected.txt	2022-05-03 16:02:48 UTC (rev 293727)
@@ -59,3 +59,27 @@
 PASS: Should return null for non-element.
 PASS: Should return null for null element.
 
+-- Running test case: View.DirtyDescendantsCount
+- Adding parent view to root view.
+PASS: Root view should have 1 dirty descendant.
+PASS: Root view should not be dirty.
+PASS: Parent attached to root view should be dirty.
+- Adding child view to parent view.
+PASS: Root view should have 2 dirty descendants.
+PASS: Parent attached to root view should have 1 dirty descendant.
+PASS: Root view should not be dirty.
+PASS: Parent attached to root view should be dirty.
+PASS: Child attached to parent view should be dirty.
+- Removing parent view from root view.
+PASS: Root view should have 0 dirty descendants.
+PASS: Parent detached from root view should have 0 dirty descendants.
+PASS: Root view should not be dirty.
+PASS: Parent detached from root view should not be dirty.
+PASS: Child attached to detached parent view should not be dirty.
+- Adding parent view to root view.
+PASS: Root view should have 1 dirty descendant.
+PASS: Parent detached from root view should have 0 dirty descendants.
+PASS: Root view should not be dirty.
+PASS: Parent attached to root view should be dirty.
+PASS: Child attached to parent view should not be dirty.
+

Modified: trunk/LayoutTests/inspector/view/basics.html (293726 => 293727)


--- trunk/LayoutTests/inspector/view/basics.html	2022-05-03 16:00:59 UTC (rev 293726)
+++ trunk/LayoutTests/inspector/view/basics.html	2022-05-03 16:02:48 UTC (rev 293727)
@@ -179,6 +179,54 @@
         }
     });
 
+    suite.addTestCase({
+        name: "View.DirtyDescendantsCount",
+        test() {
+            let rootView = WI.View.rootView();
+            let parent = new WI.View;
+            let child = new WI.View;
+
+            // The root view may still be dirty from a previous test removing a subview.
+            rootView._dirty = false;
+            rootView._dirtyDescendantsCount = 0;
+
+            InspectorTest.log("- Adding parent view to root view.");
+            rootView.addSubview(parent);
+            InspectorTest.expectEqual(rootView._dirtyDescendantsCount, 1, "Root view should have 1 dirty descendant.");
+            InspectorTest.expectFalse(rootView._dirty, "Root view should not be dirty.");
+            InspectorTest.expectTrue(parent._dirty, "Parent attached to root view should be dirty.");
+
+            InspectorTest.log("- Adding child view to parent view.");
+            parent.addSubview(child);
+            InspectorTest.expectEqual(rootView._dirtyDescendantsCount, 2, "Root view should have 2 dirty descendants.");
+            InspectorTest.expectEqual(parent._dirtyDescendantsCount, 1, "Parent attached to root view should have 1 dirty descendant.");
+            InspectorTest.expectFalse(rootView._dirty, "Root view should not be dirty.");
+            InspectorTest.expectTrue(parent._dirty, "Parent attached to root view should be dirty.");
+            InspectorTest.expectTrue(child._dirty, "Child attached to parent view should be dirty.");
+
+            InspectorTest.log("- Removing parent view from root view.");
+            rootView.removeSubview(parent);
+            InspectorTest.expectEqual(rootView._dirtyDescendantsCount, 0, "Root view should have 0 dirty descendants.");
+            InspectorTest.expectEqual(parent._dirtyDescendantsCount, 0, "Parent detached from root view should have 0 dirty descendants.");
+            InspectorTest.expectFalse(rootView._dirty, "Root view should not be dirty.");
+            InspectorTest.expectFalse(parent._dirty, "Parent detached from root view should not be dirty.");
+            InspectorTest.expectFalse(child._dirty, "Child attached to detached parent view should not be dirty.");
+
+            InspectorTest.log("- Adding parent view to root view.");
+            rootView.addSubview(parent);
+            InspectorTest.expectEqual(rootView._dirtyDescendantsCount, 1, "Root view should have 1 dirty descendant.");
+            InspectorTest.expectEqual(parent._dirtyDescendantsCount, 0, "Parent detached from root view should have 0 dirty descendants.");
+            InspectorTest.expectFalse(rootView._dirty, "Root view should not be dirty.");
+            InspectorTest.expectTrue(parent._dirty, "Parent attached to root view should be dirty.");
+            InspectorTest.expectFalse(child._dirty, "Child attached to parent view should not be dirty.");
+
+            // Clean up views for the next test.
+            rootView.removeSubview(parent);
+
+            return true;
+        }
+    });
+
     suite.runTestCasesAndFinish();
 }
 </script>

Modified: trunk/Source/WebInspectorUI/ChangeLog (293726 => 293727)


--- trunk/Source/WebInspectorUI/ChangeLog	2022-05-03 16:00:59 UTC (rev 293726)
+++ trunk/Source/WebInspectorUI/ChangeLog	2022-05-03 16:02:48 UTC (rev 293727)
@@ -1,3 +1,79 @@
+2022-05-03  Patrick Angle  <[email protected]>
+
+        Web Inspector: Importing a timeline leaves timeline overview non-scrollable/non-zoomable until windows is resized
+        https://bugs.webkit.org/show_bug.cgi?id=239880
+
+        Reviewed by Devin Rousso.
+
+        Cancelling an in-progress layout (or the layout of a child of a view in middle of laying out) led to
+        `_dirtyDescendantsCount` being in an inconsistent state. Views had their `_dirtyDescendantsCount` cleared before
+        they are laid out, so every non-zero `_dirtyDescendantsCount` of a view in the process of being laid out meant
+        that during layout `needsLayout()` was called to dirty that view or one of its children. In this case, a
+        previous cancellation meant that the TimelineOverview ended up in an inconsistent state where its parent view
+        tree was not aware that it needed layout because it reached a zero `_dirtyDescendantsCount` while walking down
+        the view hierarchy, which meant that the TimelineOverview was perpetually stuck until the entire view hierarchy
+        needed re-laid out due to a window resize or similar event.
+
+        Our current accounting for `_dirtyDescendantsCount` in View makes assumptions that are not always true during
+        layout. Specifically that parents of views undergoing layout will have a non-zero `_dirtyDescendantsCount` still
+        from which to subtract in `_cancelScheduledLayoutForView`. This wasn't always true because as soon as we began
+        laying out a parent view, we cleared both the `_dirty` flag as well as zeroed out `_dirtyDescendantsCount`. In
+        the case of cancelling a layout from within a layout, the cancellation wouldn't actually take effect anyways,
+        and since `cancelLayout` was only ever invoked from `updateLayout`, the relevant layout flags should instead be
+        cleared as part of actually performing the layout. `cancelLayout` at most saved us a rAF callback only to
+        realize we no longer have any views in need of layout, but this benefit was negligible, and in general we should
+        prefer not to be calling `updateLayout` for performance reasons anyways.
+
+        This patch improves the accounting around `_dirtyDescendantsCount`, making sure to decrement it as we perform
+        layout, instead of all at once. This reduces our reliance on assumptions about who will modify the
+        `_dirtyDescendantsCount` and at what times relative to layout.
+
+        The only time in this patch that we do not do a +1/-1 adjustment to the layout count is in the special case of
+        attaching/detaching a view. In that case, the parent tree from which we are detaching the view will have the
+        `_dirtyDescendantsCount` of the detached parent adjusted by the `_dirtyDescendantsCount` of the former child
+        view. This is done by `_setSelfAndDescendantsNotDirty`, which allows us to avoid an exponential walking of the
+        tree for this otherwise self-contained operation (we don't call into any overridable functions that could modify
+        the `_dirty` flag or `_dirtyDescendantsCount`).
+
+        After we have cleaned up the former parent branch of the tree, we mark the newly attached view as needing
+        layout, which will then ensure the child view gets laid out again by marking it as `dirty` and incrementing its
+        new parent branch of the tree's `_dirtyDescendantsCount`.
+
+        * UserInterface/Views/View.js:
+        (WI.View.prototype.insertSubviewBefore):
+        - Add assertion that the view is not currently a child of a different view in addition to the existing check
+        that a view is not already a child of the target view.
+
+        (WI.View.prototype._setDirty):
+        - New utility method through which (almost) all marking of dirty/not dirty should be done in order to ensure
+        that `_dirtyDescendantsCount` is correctly adjusted.
+
+        (WI.View.prototype._didMoveToParent):
+        - Mark the view and all subviews as not dirty before removing it from its previous parent in order to adjust
+        `_dirtyDescendantsCount` appropriately, and then mark the view as dirty once attached to its new parent.
+        - Inline the logic from WI.View.prototype._didMoveToWindow in order to also handle marking children as not dirty
+        and having a zeroed `_dirtyDescendantsCount`.
+        - This method contains the exception to the rule that _dirtyDescendantsCount is always incremented/decremented
+        by one. Here we are able to optimize away the need to walk the entire parent tree for each subview/subviews of
+        subviews/etc. because the operation takes place all at once with no overridable function being called where we
+        have to worry about mutations to `_dirty`/`_dirtyDescendantsCount`.
+
+        (WI.View.prototype._layoutSubtree):
+        (WI.View._visitViewTreeForLayout):
+        - Don't zero out the `_dirtyDescendantsCount`, and instead use the new `_setDirty` helper.
+
+        (WI.View._scheduleLayoutForView):
+        - Don't zero out the `_dirtyDescendantsCount`, and instead use the new `_setDirty` helper.
+        - Mark the view as dirty after checking if its attached a view so that detached views have no dirty state until
+        they are attached, at which point the root of the previous detached subtree will be marked as dirty.
+        - Drive-by change to use a for-loop instead of a while-loop to avoid `Array.prototype.shift()`.
+
+        (WI.View.prototype.updateLayout):
+        (WI.View.prototype.cancelLayout): Deleted.
+        (WI.View._cancelScheduledLayoutForView): Deleted.
+        - Remove the concept of "canceling" layout, since it is only used by `updateLayout`, and the call to
+        `_layoutSubtree` in `updateLayout` will cause the view, dirty or not, to be marked as not dirty.
+
 2022-05-03  Michael Saboff  <[email protected]>
 
         WebInspectorUI is missing a symlink to system content path

Modified: trunk/Source/WebInspectorUI/UserInterface/Views/View.js (293726 => 293727)


--- trunk/Source/WebInspectorUI/UserInterface/Views/View.js	2022-05-03 16:00:59 UTC (rev 293726)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/View.js	2022-05-03 16:02:48 UTC (rev 293727)
@@ -95,6 +95,8 @@
         console.assert(!referenceView || referenceView instanceof WI.View);
         console.assert(view !== WI.View._rootView, "Root view cannot be a subview.");
 
+        console.assert(!view.parentView, view);
+
         if (this._subviews.includes(view)) {
             console.assert(false, "Cannot add view that is already a subview.", view);
             return;
@@ -153,8 +155,6 @@
 
     updateLayout(layoutReason)
     {
-        this.cancelLayout();
-
         this._setLayoutReason(layoutReason);
         this._layoutSubtree();
     }
@@ -177,11 +177,6 @@
         WI.View._scheduleLayoutForView(this);
     }
 
-    cancelLayout()
-    {
-        WI.View._cancelScheduledLayoutForView(this);
-    }
-
     // Protected
 
     get layoutReason() { return this._layoutReason; }
@@ -230,50 +225,65 @@
 
     // Private
 
-    _didMoveToParent(parentView)
+    _setDirty(dirty)
     {
-        this._parentView = parentView;
-
-        let isAttachedToRoot = this.isDescendantOf(WI.View._rootView);
-        this._didMoveToWindow(isAttachedToRoot);
-
-        if (!this._parentView)
+        if (this._dirty === dirty)
             return;
 
-        let pendingLayoutsCount = this._dirtyDescendantsCount;
-        if (this._dirty)
-            pendingLayoutsCount++;
+        this._dirty = dirty;
 
-        let view = this._parentView;
-        while (view) {
-            view._dirtyDescendantsCount += pendingLayoutsCount;
-            view = view.parentView;
+        for (let parentView = this.parentView; parentView; parentView = parentView.parentView) {
+            parentView._dirtyDescendantsCount += this._dirty ? 1 : -1;
+            console.assert(parentView._dirtyDescendantsCount >= 0);
         }
     }
 
-    _didMoveToWindow(isAttachedToRoot)
+    _didMoveToParent(parentView)
     {
-        if (this._isAttachedToRoot === isAttachedToRoot)
+        if (this._parentView === parentView)
             return;
 
-        this._isAttachedToRoot = isAttachedToRoot;
-        if (this._isAttachedToRoot) {
-            WI.View._scheduleLayoutForView(this);
-            this.attached();
-        } else {
-            if (this._dirty)
-                this.cancelLayout();
-            this.detached();
+        console.assert(this._parentView || !(this._isDirty || this._dirtyDescendantsCount));
+
+        let dirtyDescendantsCount = this._dirtyDescendantsCount;
+        if (this._dirty)
+            ++dirtyDescendantsCount;
+
+        if (dirtyDescendantsCount) {
+            for (let view = this.parentView; view; view = view.parentView) {
+                view._dirtyDescendantsCount -= dirtyDescendantsCount;
+                console.assert(view._dirtyDescendantsCount >= 0);
+            }
         }
 
-        for (let view of this._subviews)
-            view._didMoveToWindow(isAttachedToRoot);
+        this._parentView = parentView;
+        let isAttachedToRoot = this.isDescendantOf(WI.View._rootView);
+
+        let views = [this];
+        for (let i = 0; i < views.length; ++i) {
+            let view = views[i];
+            views.pushAll(view.subviews);
+
+            view._dirty = false;
+            view._dirtyDescendantsCount = 0;
+
+            if (view._isAttachedToRoot === isAttachedToRoot)
+                continue;
+
+            view._isAttachedToRoot = isAttachedToRoot;
+            if (view._isAttachedToRoot)
+                view.attached();
+            else
+                view.detached();
+        }
+
+        if (isAttachedToRoot)
+            WI.View._scheduleLayoutForView(this);
     }
 
     _layoutSubtree()
     {
-        this._dirty = false;
-        this._dirtyDescendantsCount = 0;
+        this._setDirty(false);
         let isInitialLayout = !this._didInitialLayout;
 
         if (isInitialLayout) {
@@ -344,17 +354,11 @@
 
     static _scheduleLayoutForView(view)
     {
-        view._dirty = true;
-
-        let parentView = view.parentView;
-        while (parentView) {
-            parentView._dirtyDescendantsCount++;
-            parentView = parentView.parentView;
-        }
-
         if (!view._isAttachedToRoot)
             return;
 
+        view._setDirty(true);
+
         if (WI.View._scheduledLayoutUpdateIdentifier)
             return;
 
@@ -361,32 +365,6 @@
         WI.View._scheduledLayoutUpdateIdentifier = requestAnimationFrame(WI.View._visitViewTreeForLayout);
     }
 
-    static _cancelScheduledLayoutForView(view)
-    {
-        let cancelledLayoutsCount = view._dirtyDescendantsCount;
-        if (view.layoutPending)
-            cancelledLayoutsCount++;
-
-        let parentView = view.parentView;
-        while (parentView) {
-            parentView._dirtyDescendantsCount = Math.max(0, parentView._dirtyDescendantsCount - cancelledLayoutsCount);
-            parentView = parentView.parentView;
-        }
-
-        view._dirty = false;
-
-        if (!WI.View._scheduledLayoutUpdateIdentifier)
-            return;
-
-        let rootView = WI.View._rootView;
-        if (!rootView || rootView._dirtyDescendantsCount)
-            return;
-
-        // No views need layout, so cancel the pending requestAnimationFrame.
-        cancelAnimationFrame(WI.View._scheduledLayoutUpdateIdentifier);
-        WI.View._scheduledLayoutUpdateIdentifier = undefined;
-    }
-
     static _visitViewTreeForLayout()
     {
         console.assert(WI.View._rootView, "Cannot layout view tree without a root.");
@@ -394,14 +372,12 @@
         WI.View._scheduledLayoutUpdateIdentifier = undefined;
 
         let views = [WI.View._rootView];
-        while (views.length) {
-            let view = views.shift();
+        for (let i = 0; i < views.length; ++i) {
+            let view = views[i];
             if (view.layoutPending)
                 view._layoutSubtree();
-            else if (view._dirtyDescendantsCount) {
+            else if (view._dirtyDescendantsCount)
                 views.pushAll(view.subviews);
-                view._dirtyDescendantsCount = 0;
-            }
         }
     }
 };
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to