Changeset 293727 in webkit


Ignore:
Timestamp:
May 3, 2022 9:02:48 AM (3 months ago)
Author:
Patrick Angle
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.

Location:
trunk
Files:
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r293725 r293727  
     12022-05-03  Patrick Angle  <pangle@apple.com>
     2
     3        Web Inspector: Importing a timeline leaves timeline overview non-scrollable/non-zoomable until windows is resized
     4        https://bugs.webkit.org/show_bug.cgi?id=239880
     5
     6        Reviewed by Devin Rousso.
     7
     8        * inspector/view/asynchronous-layout-expected.txt:
     9        * inspector/view/asynchronous-layout.html:
     10        - Added test case for calls to `updateLayout()` during asynchronous `layout()`.
     11        - Remove test case for removed `View.prototype.cancelLayout`.
     12
     13        * inspector/view/basics-expected.txt:
     14        * inspector/view/basics.html:
     15        - Added test case to verify that `_dirtyDescendantsCount` is an expected value at various points of
     16        attaching/detaching views.
     17
    1182022-05-03  Antti Koivisto  <antti@apple.com>
    219

  • trunk/LayoutTests/inspector/view/asynchronous-layout-expected.txt

    r222782 r293727  
    2121PASS: View should not have a pending layout.
    2222
     23-- Running test case: View.SyncronousLayoutDuringAsyncronousLayout
     24PASS: Root view should have 2 dirty descendants.
     25PASS: Parent view should have 1 dirty descendant.
     26PASS: Child view should have 0 dirty descendants.
     27PASS: View should have a pending layout.
     28Child view completed a layout.
     29PASS: Root view should have 1 dirty descendant.
     30PASS: Parent view should have 0 dirty descendants.
     31PASS: Child view should have 0 dirty descendants.
     32PASS: Parent view should have started a layout.
     33PASS: Child view should have completed 1 layout.
     34Child view completed a layout.
     35PASS: Root view should have 0 dirty descendants.
     36PASS: Parent view should have 0 dirty descendants.
     37PASS: Child view should have 0 dirty descendants.
     38PASS: Parent view should have started a layout.
     39PASS: Child view should have completed 2 layouts.
     40Parent view completed a layout.
     41PASS: Root view should have 0 dirty descendants.
     42PASS: Root view should have 0 dirty descendants.
     43PASS: Root view should have 0 dirty descendants.
     44PASS: Parent view should have completed 1 layout.
     45PASS: Parent view should not have a pending layout.
     46
    2347-- Running test case: View.needsLayout.propogateToSubview
    2448Schedule parent view update.
    … …  
    2751PASS: Child view should update its layout.
    2852
    29 -- Running test case: View.cancelLayout
    30 Cancel automatic layout.
    31 PASS: View should not have a pending layout.
    32 Cancel scheduled layout.
    33 PASS: View should not have a pending layout.
    34 

  • trunk/LayoutTests/inspector/view/asynchronous-layout.html

    r222782 r293727  
    6363
    6464    suite.addTestCase({
     65        name: "View.SyncronousLayoutDuringAsyncronousLayout",
     66        test(resolve, reject) {
     67            let UpdateLayoutTestView = class UpdateLayoutTestView extends WI.TestView {
     68                layout() {
     69                    super.layout();
     70
     71                    this.subviews[0].updateLayout();
     72                }
     73            }
     74
     75            let rootView = WI.View.rootView();
     76            let parentView = new UpdateLayoutTestView;
     77            let childView = new WI.TestView;
     78
     79            rootView.addSubview(parentView);
     80            parentView.addSubview(childView);
     81
     82            InspectorTest.expectEqual(rootView._dirtyDescendantsCount, 2, "Root view should have 2 dirty descendants.");
     83            InspectorTest.expectEqual(parentView._dirtyDescendantsCount, 1, "Parent view should have 1 dirty descendant.");
     84            InspectorTest.expectEqual(childView._dirtyDescendantsCount, 0, "Child view should have 0 dirty descendants.");
     85            InspectorTest.expectThat(parentView.layoutPending, "View should have a pending layout.");
     86
     87            childView.evaluateAfterLayout(() => {
     88                InspectorTest.log("Child view completed a layout.");
     89
     90                InspectorTest.expectEqual(rootView._dirtyDescendantsCount, 0, "Root view should have 1 dirty descendant.");
     91                InspectorTest.expectEqual(parentView._dirtyDescendantsCount, 0, "Parent view should have 0 dirty descendants.");
     92                InspectorTest.expectEqual(childView._dirtyDescendantsCount, 0, "Child view should have 0 dirty descendants.");
     93
     94                InspectorTest.expectEqual(parentView.layoutCount, 1, "Parent view should have started a layout.");
     95                InspectorTest.expectEqual(childView.layoutCount, 1, "Child view should have completed 1 layout.");
     96
     97                childView.evaluateAfterLayout(() => {
     98                    InspectorTest.log("Child view completed a layout.");
     99                    InspectorTest.expectEqual(rootView._dirtyDescendantsCount, 0, "Root view should have 0 dirty descendants.");
     100                    InspectorTest.expectEqual(parentView._dirtyDescendantsCount, 0, "Parent view should have 0 dirty descendants.");
     101                    InspectorTest.expectEqual(childView._dirtyDescendantsCount, 0, "Child view should have 0 dirty descendants.");
     102
     103                    InspectorTest.expectEqual(parentView.layoutCount, 1, "Parent view should have started a layout.");
     104                    InspectorTest.expectEqual(childView.layoutCount, 2, "Child view should have completed 2 layouts.");
     105                });
     106            });
     107
     108            parentView.evaluateAfterLayout(() => {
     109                InspectorTest.log("Parent view completed a layout.");
     110
     111                InspectorTest.expectEqual(rootView._dirtyDescendantsCount, 0, "Root view should have 0 dirty descendants.");
     112                InspectorTest.expectEqual(parentView._dirtyDescendantsCount, 0, "Root view should have 0 dirty descendants.");
     113                InspectorTest.expectEqual(childView._dirtyDescendantsCount, 0, "Root view should have 0 dirty descendants.");
     114
     115                InspectorTest.expectEqual(parentView.layoutCount, 1, "Parent view should have completed 1 layout.");
     116                InspectorTest.expectFalse(parentView.layoutPending, "Parent view should not have a pending layout.");
     117
     118                resolve();
     119            });
     120        }
     121    });
     122
     123    suite.addTestCase({
    65124        name: "View.needsLayout.propogateToSubview",
    66125        test(resolve, reject) {
    … …  
    81140    });
    82141
    83     suite.addTestCase({
    84         name: "View.cancelLayout",
    85         test(resolve, reject) {
    86             let view = new WI.TestView;
    87             WI.View.rootView().addSubview(view);
    88 
    89             InspectorTest.log("Cancel automatic layout.");
    90             view.cancelLayout();
    91             InspectorTest.expectFalse(view.layoutPending, "View should not have a pending layout.");
    92 
    93             InspectorTest.log("Cancel scheduled layout.");
    94             view.needsLayout();
    95             view.cancelLayout();
    96             InspectorTest.expectFalse(view.layoutPending, "View should not have a pending layout.");
    97             resolve();
    98         }
    99     });
    100 
    101142    suite.runTestCasesAndFinish();
    102143}

  • trunk/LayoutTests/inspector/view/basics-expected.txt

    r242241 r293727  
    6060PASS: Should return null for null element.
    6161
     62-- Running test case: View.DirtyDescendantsCount
     63- Adding parent view to root view.
     64PASS: Root view should have 1 dirty descendant.
     65PASS: Root view should not be dirty.
     66PASS: Parent attached to root view should be dirty.
     67- Adding child view to parent view.
     68PASS: Root view should have 2 dirty descendants.
     69PASS: Parent attached to root view should have 1 dirty descendant.
     70PASS: Root view should not be dirty.
     71PASS: Parent attached to root view should be dirty.
     72PASS: Child attached to parent view should be dirty.
     73- Removing parent view from root view.
     74PASS: Root view should have 0 dirty descendants.
     75PASS: Parent detached from root view should have 0 dirty descendants.
     76PASS: Root view should not be dirty.
     77PASS: Parent detached from root view should not be dirty.
     78PASS: Child attached to detached parent view should not be dirty.
     79- Adding parent view to root view.
     80PASS: Root view should have 1 dirty descendant.
     81PASS: Parent detached from root view should have 0 dirty descendants.
     82PASS: Root view should not be dirty.
     83PASS: Parent attached to root view should be dirty.
     84PASS: Child attached to parent view should not be dirty.
     85

  • trunk/LayoutTests/inspector/view/basics.html

    r242241 r293727  
    180180    });
    181181
     182    suite.addTestCase({
     183        name: "View.DirtyDescendantsCount",
     184        test() {
     185            let rootView = WI.View.rootView();
     186            let parent = new WI.View;
     187            let child = new WI.View;
     188
     189            // The root view may still be dirty from a previous test removing a subview.
     190            rootView._dirty = false;
     191            rootView._dirtyDescendantsCount = 0;
     192
     193            InspectorTest.log("- Adding parent view to root view.");
     194            rootView.addSubview(parent);
     195            InspectorTest.expectEqual(rootView._dirtyDescendantsCount, 1, "Root view should have 1 dirty descendant.");
     196            InspectorTest.expectFalse(rootView._dirty, "Root view should not be dirty.");
     197            InspectorTest.expectTrue(parent._dirty, "Parent attached to root view should be dirty.");
     198
     199            InspectorTest.log("- Adding child view to parent view.");
     200            parent.addSubview(child);
     201            InspectorTest.expectEqual(rootView._dirtyDescendantsCount, 2, "Root view should have 2 dirty descendants.");
     202            InspectorTest.expectEqual(parent._dirtyDescendantsCount, 1, "Parent attached to root view should have 1 dirty descendant.");
     203            InspectorTest.expectFalse(rootView._dirty, "Root view should not be dirty.");
     204            InspectorTest.expectTrue(parent._dirty, "Parent attached to root view should be dirty.");
     205            InspectorTest.expectTrue(child._dirty, "Child attached to parent view should be dirty.");
     206
     207            InspectorTest.log("- Removing parent view from root view.");
     208            rootView.removeSubview(parent);
     209            InspectorTest.expectEqual(rootView._dirtyDescendantsCount, 0, "Root view should have 0 dirty descendants.");
     210            InspectorTest.expectEqual(parent._dirtyDescendantsCount, 0, "Parent detached from root view should have 0 dirty descendants.");
     211            InspectorTest.expectFalse(rootView._dirty, "Root view should not be dirty.");
     212            InspectorTest.expectFalse(parent._dirty, "Parent detached from root view should not be dirty.");
     213            InspectorTest.expectFalse(child._dirty, "Child attached to detached parent view should not be dirty.");
     214
     215            InspectorTest.log("- Adding parent view to root view.");
     216            rootView.addSubview(parent);
     217            InspectorTest.expectEqual(rootView._dirtyDescendantsCount, 1, "Root view should have 1 dirty descendant.");
     218            InspectorTest.expectEqual(parent._dirtyDescendantsCount, 0, "Parent detached from root view should have 0 dirty descendants.");
     219            InspectorTest.expectFalse(rootView._dirty, "Root view should not be dirty.");
     220            InspectorTest.expectTrue(parent._dirty, "Parent attached to root view should be dirty.");
     221            InspectorTest.expectFalse(child._dirty, "Child attached to parent view should not be dirty.");
     222
     223            // Clean up views for the next test.
     224            rootView.removeSubview(parent);
     225
     226            return true;
     227        }
     228    });
     229
    182230    suite.runTestCasesAndFinish();
    183231}

  • trunk/Source/WebInspectorUI/ChangeLog

    r293724 r293727  
     12022-05-03  Patrick Angle  <pangle@apple.com>
     2
     3        Web Inspector: Importing a timeline leaves timeline overview non-scrollable/non-zoomable until windows is resized
     4        https://bugs.webkit.org/show_bug.cgi?id=239880
     5
     6        Reviewed by Devin Rousso.
     7
     8        Cancelling an in-progress layout (or the layout of a child of a view in middle of laying out) led to
     9        `_dirtyDescendantsCount` being in an inconsistent state. Views had their `_dirtyDescendantsCount` cleared before
     10        they are laid out, so every non-zero `_dirtyDescendantsCount` of a view in the process of being laid out meant
     11        that during layout `needsLayout()` was called to dirty that view or one of its children. In this case, a
     12        previous cancellation meant that the TimelineOverview ended up in an inconsistent state where its parent view
     13        tree was not aware that it needed layout because it reached a zero `_dirtyDescendantsCount` while walking down
     14        the view hierarchy, which meant that the TimelineOverview was perpetually stuck until the entire view hierarchy
     15        needed re-laid out due to a window resize or similar event.
     16
     17        Our current accounting for `_dirtyDescendantsCount` in View makes assumptions that are not always true during
     18        layout. Specifically that parents of views undergoing layout will have a non-zero `_dirtyDescendantsCount` still
     19        from which to subtract in `_cancelScheduledLayoutForView`. This wasn't always true because as soon as we began
     20        laying out a parent view, we cleared both the `_dirty` flag as well as zeroed out `_dirtyDescendantsCount`. In
     21        the case of cancelling a layout from within a layout, the cancellation wouldn't actually take effect anyways,
     22        and since `cancelLayout` was only ever invoked from `updateLayout`, the relevant layout flags should instead be
     23        cleared as part of actually performing the layout. `cancelLayout` at most saved us a rAF callback only to
     24        realize we no longer have any views in need of layout, but this benefit was negligible, and in general we should
     25        prefer not to be calling `updateLayout` for performance reasons anyways.
     26
     27        This patch improves the accounting around `_dirtyDescendantsCount`, making sure to decrement it as we perform
     28        layout, instead of all at once. This reduces our reliance on assumptions about who will modify the
     29        `_dirtyDescendantsCount` and at what times relative to layout.
     30
     31        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
     32        attaching/detaching a view. In that case, the parent tree from which we are detaching the view will have the
     33        `_dirtyDescendantsCount` of the detached parent adjusted by the `_dirtyDescendantsCount` of the former child
     34        view. This is done by `_setSelfAndDescendantsNotDirty`, which allows us to avoid an exponential walking of the
     35        tree for this otherwise self-contained operation (we don't call into any overridable functions that could modify
     36        the `_dirty` flag or `_dirtyDescendantsCount`).
     37
     38        After we have cleaned up the former parent branch of the tree, we mark the newly attached view as needing
     39        layout, which will then ensure the child view gets laid out again by marking it as `dirty` and incrementing its
     40        new parent branch of the tree's `_dirtyDescendantsCount`.
     41
     42        * UserInterface/Views/View.js:
     43        (WI.View.prototype.insertSubviewBefore):
     44        - Add assertion that the view is not currently a child of a different view in addition to the existing check
     45        that a view is not already a child of the target view.
     46
     47        (WI.View.prototype._setDirty):
     48        - New utility method through which (almost) all marking of dirty/not dirty should be done in order to ensure
     49        that `_dirtyDescendantsCount` is correctly adjusted.
     50
     51        (WI.View.prototype._didMoveToParent):
     52        - Mark the view and all subviews as not dirty before removing it from its previous parent in order to adjust
     53        `_dirtyDescendantsCount` appropriately, and then mark the view as dirty once attached to its new parent.
     54        - Inline the logic from WI.View.prototype._didMoveToWindow in order to also handle marking children as not dirty
     55        and having a zeroed `_dirtyDescendantsCount`.
     56        - This method contains the exception to the rule that _dirtyDescendantsCount is always incremented/decremented
     57        by one. Here we are able to optimize away the need to walk the entire parent tree for each subview/subviews of
     58        subviews/etc. because the operation takes place all at once with no overridable function being called where we
     59        have to worry about mutations to `_dirty`/`_dirtyDescendantsCount`.
     60
     61        (WI.View.prototype._layoutSubtree):
     62        (WI.View._visitViewTreeForLayout):
     63        - Don't zero out the `_dirtyDescendantsCount`, and instead use the new `_setDirty` helper.
     64
     65        (WI.View._scheduleLayoutForView):
     66        - Don't zero out the `_dirtyDescendantsCount`, and instead use the new `_setDirty` helper.
     67        - Mark the view as dirty after checking if its attached a view so that detached views have no dirty state until
     68        they are attached, at which point the root of the previous detached subtree will be marked as dirty.
     69        - Drive-by change to use a for-loop instead of a while-loop to avoid `Array.prototype.shift()`.
     70
     71        (WI.View.prototype.updateLayout):
     72        (WI.View.prototype.cancelLayout): Deleted.
     73        (WI.View._cancelScheduledLayoutForView): Deleted.
     74        - Remove the concept of "canceling" layout, since it is only used by `updateLayout`, and the call to
     75        `_layoutSubtree` in `updateLayout` will cause the view, dirty or not, to be marked as not dirty.
     76
    1772022-05-03  Michael Saboff  <msaboff@apple.com>
    278

  • trunk/Source/WebInspectorUI/UserInterface/Views/View.js

    r276975 r293727  
    9696        console.assert(view !== WI.View._rootView, "Root view cannot be a subview.");
    9797
     98        console.assert(!view.parentView, view);
     99
    98100        if (this._subviews.includes(view)) {
    99101            console.assert(false, "Cannot add view that is already a subview.", view);
    … …  
    154156    updateLayout(layoutReason)
    155157    {
    156         this.cancelLayout();
    157 
    158158        this._setLayoutReason(layoutReason);
    159159        this._layoutSubtree();
    … …  
    176176
    177177        WI.View._scheduleLayoutForView(this);
    178     }
    179 
    180     cancelLayout()
    181     {
    182         WI.View._cancelScheduledLayoutForView(this);
    183178    }
    184179
    … …  
    231226    // Private
    232227
     228    _setDirty(dirty)
     229    {
     230        if (this._dirty === dirty)
     231            return;
     232
     233        this._dirty = dirty;
     234
     235        for (let parentView = this.parentView; parentView; parentView = parentView.parentView) {
     236            parentView._dirtyDescendantsCount += this._dirty ? 1 : -1;
     237            console.assert(parentView._dirtyDescendantsCount >= 0);
     238        }
     239    }
     240
    233241    _didMoveToParent(parentView)
    234242    {
     243        if (this._parentView === parentView)
     244            return;
     245
     246        console.assert(this._parentView || !(this._isDirty || this._dirtyDescendantsCount));
     247
     248        let dirtyDescendantsCount = this._dirtyDescendantsCount;
     249        if (this._dirty)
     250            ++dirtyDescendantsCount;
     251
     252        if (dirtyDescendantsCount) {
     253            for (let view = this.parentView; view; view = view.parentView) {
     254                view._dirtyDescendantsCount -= dirtyDescendantsCount;
     255                console.assert(view._dirtyDescendantsCount >= 0);
     256            }
     257        }
     258
    235259        this._parentView = parentView;
    236 
    237260        let isAttachedToRoot = this.isDescendantOf(WI.View._rootView);
    238         this._didMoveToWindow(isAttachedToRoot);
    239 
    240         if (!this._parentView)
    241             return;
    242 
    243         let pendingLayoutsCount = this._dirtyDescendantsCount;
    244         if (this._dirty)
    245             pendingLayoutsCount++;
    246 
    247         let view = this._parentView;
    248         while (view) {
    249             view._dirtyDescendantsCount += pendingLayoutsCount;
    250             view = view.parentView;
    251         }
    252     }
    253 
    254     _didMoveToWindow(isAttachedToRoot)
    255     {
    256         if (this._isAttachedToRoot === isAttachedToRoot)
    257             return;
    258 
    259         this._isAttachedToRoot = isAttachedToRoot;
    260         if (this._isAttachedToRoot) {
     261
     262        let views = [this];
     263        for (let i = 0; i < views.length; ++i) {
     264            let view = views[i];
     265            views.pushAll(view.subviews);
     266
     267            view._dirty = false;
     268            view._dirtyDescendantsCount = 0;
     269
     270            if (view._isAttachedToRoot === isAttachedToRoot)
     271                continue;
     272
     273            view._isAttachedToRoot = isAttachedToRoot;
     274            if (view._isAttachedToRoot)
     275                view.attached();
     276            else
     277                view.detached();
     278        }
     279
     280        if (isAttachedToRoot)
    261281            WI.View._scheduleLayoutForView(this);
    262             this.attached();
    263         } else {
    264             if (this._dirty)
    265                 this.cancelLayout();
    266             this.detached();
    267         }
    268 
    269         for (let view of this._subviews)
    270             view._didMoveToWindow(isAttachedToRoot);
    271282    }
    272283
    273284    _layoutSubtree()
    274285    {
    275         this._dirty = false;
    276         this._dirtyDescendantsCount = 0;
     286        this._setDirty(false);
    277287        let isInitialLayout = !this._didInitialLayout;
    278288
    … …  
    345355    static _scheduleLayoutForView(view)
    346356    {
    347         view._dirty = true;
    348 
    349         let parentView = view.parentView;
    350         while (parentView) {
    351             parentView._dirtyDescendantsCount++;
    352             parentView = parentView.parentView;
    353         }
    354 
    355357        if (!view._isAttachedToRoot)
    356358            return;
    357359
     360        view._setDirty(true);
     361
    358362        if (WI.View._scheduledLayoutUpdateIdentifier)
    359363            return;
    … …  
    362366    }
    363367
    364     static _cancelScheduledLayoutForView(view)
    365     {
    366         let cancelledLayoutsCount = view._dirtyDescendantsCount;
    367         if (view.layoutPending)
    368             cancelledLayoutsCount++;
    369 
    370         let parentView = view.parentView;
    371         while (parentView) {
    372             parentView._dirtyDescendantsCount = Math.max(0, parentView._dirtyDescendantsCount - cancelledLayoutsCount);
    373             parentView = parentView.parentView;
    374         }
    375 
    376         view._dirty = false;
    377 
    378         if (!WI.View._scheduledLayoutUpdateIdentifier)
    379             return;
    380 
    381         let rootView = WI.View._rootView;
    382         if (!rootView || rootView._dirtyDescendantsCount)
    383             return;
    384 
    385         // No views need layout, so cancel the pending requestAnimationFrame.
    386         cancelAnimationFrame(WI.View._scheduledLayoutUpdateIdentifier);
     368    static _visitViewTreeForLayout()
     369    {
     370        console.assert(WI.View._rootView, "Cannot layout view tree without a root.");
     371
    387372        WI.View._scheduledLayoutUpdateIdentifier = undefined;
    388     }
    389 
    390     static _visitViewTreeForLayout()
    391     {
    392         console.assert(WI.View._rootView, "Cannot layout view tree without a root.");
    393 
    394         WI.View._scheduledLayoutUpdateIdentifier = undefined;
    395373
    396374        let views = [WI.View._rootView];
    397         while (views.length) {
    398             let view = views.shift();
     375        for (let i = 0; i < views.length; ++i) {
     376            let view = views[i];
    399377            if (view.layoutPending)
    400378                view._layoutSubtree();
    401             else if (view._dirtyDescendantsCount) {
     379            else if (view._dirtyDescendantsCount)
    402380                views.pushAll(view.subviews);
    403                 view._dirtyDescendantsCount = 0;
    404             }
    405381        }
    406382    }
Note: See TracChangeset for help on using the changeset viewer.