New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 602908 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocked on:
issue 600998



Sign in to add a comment

Avoid scheduleRelayout() if during layout

Project Member Reported by kojii@chromium.org, Apr 13 2016

Issue description

LayoutBlockFlow::layoutBlockFlow() and LayoutFlexibleBox::layoutBlock(bool relayoutChildren) were fixed in r386786, but there are a few more.
 

Comment 1 by kojii@chromium.org, Apr 13 2016

Cc: le...@chromium.org
Project Member

Comment 2 by bugdroid1@chromium.org, Apr 13 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8c2ad01a12f64788bacfc2c57125d3857a2b8e26

commit 8c2ad01a12f64788bacfc2c57125d3857a2b8e26
Author: kojii <kojii@chromium.org>
Date: Wed Apr 13 22:16:05 2016

Fix TextAutosizer not to scheduleRelayout() for LayoutGrid::layoutBlock()

This patch fixes TextAutosizer not to scheduleRelayout() when it's
called from LayoutGrid::layoutBlock().

SubtreeLayoutScope was added in the same way as
LayoutFlexibleBox::layoutBlock() does, which is passed to TextAutosizer
to prevent it to call scheduleRelayout().

BUG= 602908 

Review URL: https://codereview.chromium.org/1880343004

Cr-Commit-Position: refs/heads/master@{#387118}

[modify] https://crrev.com/8c2ad01a12f64788bacfc2c57125d3857a2b8e26/third_party/WebKit/Source/core/layout/LayoutGrid.cpp

Comment 3 by kojii@chromium.org, Apr 19 2016

Cc: cbiesin...@chromium.org
Summary: Avoid scheduleRelayout() if during layout (was: TextAutosizer should not scheduleRelayout() if during layout)
Widening the scope, because it turns out that it's not only TextAutosizer that calls markContainerChainForLayout with scheduleRelayout=true during the layout. See crrev.com/1896803002#msg6 and crrev.com/1896803002#msg7.

Project Member

Comment 4 by bugdroid1@chromium.org, Apr 20 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/31c3833651246f431a837c65e8f61b20aa1e00b1

commit 31c3833651246f431a837c65e8f61b20aa1e00b1
Author: kojii <kojii@chromium.org>
Date: Wed Apr 20 02:49:34 2016

Fix TextAutosizer not to scheduleRelayout() by MarkContainerChainInLayout

This patch fixes TextAutosizer not to scheduleRelayout() when its
relayoutBehavior is AlreadyInLayout.

r378639 solved the same issue only when markContainerChainForLayout()
has SubtreeLayoutScope, but TextAutosizer does not always have
SubtreeLayoutScope.

To solve this case, MarkContainerChainInLayout is added to mark
container chain without scheduleRelayout(), even when
SubtreeLayoutScope is missing.

BUG= 602908 

Review URL: https://codereview.chromium.org/1896803002

Cr-Commit-Position: refs/heads/master@{#388410}

[modify] https://crrev.com/31c3833651246f431a837c65e8f61b20aa1e00b1/third_party/WebKit/Source/core/layout/LayoutObject.cpp
[modify] https://crrev.com/31c3833651246f431a837c65e8f61b20aa1e00b1/third_party/WebKit/Source/core/layout/LayoutObject.h
[modify] https://crrev.com/31c3833651246f431a837c65e8f61b20aa1e00b1/third_party/WebKit/Source/core/layout/TextAutosizer.cpp

Project Member

Comment 5 by bugdroid1@chromium.org, Apr 26 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1a0aa2dff16abbd9bc7f906b4d807766668bde17

commit 1a0aa2dff16abbd9bc7f906b4d807766668bde17
Author: kojii <kojii@chromium.org>
Date: Tue Apr 26 04:12:48 2016

Prevent markContainerChainForLayout to scheduleRelayout() if isInPerformLayout()

When we're in layout, markContainerChainForLayout() is marking a
descendant as needing layout with the intention of visiting it during
this layout. We shouldn't be scheduling it to be laid out later.

Also, schedduleRelayout() must not be called while iterating
FrameView::m_layoutSubtreeRootList.

BUG= 602908 , 589773

Review URL: https://codereview.chromium.org/1906803002

Cr-Commit-Position: refs/heads/master@{#389691}

[modify] https://crrev.com/1a0aa2dff16abbd9bc7f906b4d807766668bde17/third_party/WebKit/Source/core/frame/FrameView.cpp
[modify] https://crrev.com/1a0aa2dff16abbd9bc7f906b4d807766668bde17/third_party/WebKit/Source/core/layout/LayoutObject.cpp

Project Member

Comment 6 by bugdroid1@chromium.org, May 2 2016

Labels: merge-merged-2704
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/90b37d4b2f759e1fe6b8de74abd6f6429c16f723

commit 90b37d4b2f759e1fe6b8de74abd6f6429c16f723
Author: Koji Ishii <kojii@chromium.org>
Date: Mon May 02 04:59:28 2016

Prevent markContainerChainForLayout to scheduleRelayout() if isInPerformLayout()

When we're in layout, markContainerChainForLayout() is marking a
descendant as needing layout with the intention of visiting it during
this layout. We shouldn't be scheduling it to be laid out later.

Also, schedduleRelayout() must not be called while iterating
FrameView::m_layoutSubtreeRootList.

BUG= 602908 , 589773

Review URL: https://codereview.chromium.org/1906803002

Cr-Commit-Position: refs/heads/master@{#389691}
(cherry picked from commit 1a0aa2dff16abbd9bc7f906b4d807766668bde17)

Review URL: https://codereview.chromium.org/1940773002 .

Cr-Commit-Position: refs/branch-heads/2704@{#326}
Cr-Branched-From: 6e53600def8f60d8c632fadc70d7c1939ccea347-refs/heads/master@{#386251}

[modify] https://crrev.com/90b37d4b2f759e1fe6b8de74abd6f6429c16f723/third_party/WebKit/Source/core/frame/FrameView.cpp
[modify] https://crrev.com/90b37d4b2f759e1fe6b8de74abd6f6429c16f723/third_party/WebKit/Source/core/layout/LayoutObject.cpp

Project Member

Comment 7 by bugdroid1@chromium.org, May 3 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/eb09dd790c0ca396ef4a0a8c603ada6eb40ab3a0

commit eb09dd790c0ca396ef4a0a8c603ada6eb40ab3a0
Author: kojii <kojii@chromium.org>
Date: Tue May 03 00:37:29 2016

Cleanup partial changes to prevent scheduleRelayout()

crrev.com/389691 prevents scheduleRelayout() during layout
automatically in markContainerChainForLayout(), making callers of
setNeedsLayout() to explicitly avoid scheduleRelayout() unnecessary.

This patch cleans up previous efforts to explicitly avoid it.

BUG= 602908 

Review-Url: https://codereview.chromium.org/1934233002
Cr-Commit-Position: refs/heads/master@{#391123}

[modify] https://crrev.com/eb09dd790c0ca396ef4a0a8c603ada6eb40ab3a0/third_party/WebKit/Source/core/layout/LayoutObject.h
[modify] https://crrev.com/eb09dd790c0ca396ef4a0a8c603ada6eb40ab3a0/third_party/WebKit/Source/core/layout/TextAutosizer.cpp
[modify] https://crrev.com/eb09dd790c0ca396ef4a0a8c603ada6eb40ab3a0/third_party/WebKit/Source/core/layout/TextAutosizer.h

Comment 8 by kojii@chromium.org, May 4 2016

Status: Fixed (was: Assigned)

Sign in to add a comment