Skip to content
Permalink
Browse files
[FrameView::layout cleanup] Do not reenter FrameView::performPostLayo…
…utTasks

https://bugs.webkit.org/show_bug.cgi?id=178518
<rdar://problem/35075409>

Reviewed by Antti Koivisto.

This patch tightens existing reentrancy policy on performPostLayoutTasks.

Covered by existing test cases.

* page/FrameView.cpp:
(WebCore::FrameView::FrameView):
(WebCore::FrameView::reset):
(WebCore::FrameView::layout):
(WebCore::FrameView::performPostLayoutTasks):
* page/FrameView.h:


Canonical link: https://commits.webkit.org/194714@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@223696 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
alanbujtas committed Oct 19, 2017
1 parent 2cadfaa commit aeaf814d1ae99bd782cf3a547dd50ab223f48e75
Showing with 29 additions and 14 deletions.
  1. +19 −0 Source/WebCore/ChangeLog
  2. +9 −13 Source/WebCore/page/FrameView.cpp
  3. +1 −1 Source/WebCore/page/FrameView.h
@@ -1,3 +1,22 @@
2017-10-19 Zalan Bujtas <zalan@apple.com>

[FrameView::layout cleanup] Do not reenter FrameView::performPostLayoutTasks
https://bugs.webkit.org/show_bug.cgi?id=178518
<rdar://problem/35075409>

Reviewed by Antti Koivisto.

This patch tightens existing reentrancy policy on performPostLayoutTasks.

Covered by existing test cases.

* page/FrameView.cpp:
(WebCore::FrameView::FrameView):
(WebCore::FrameView::reset):
(WebCore::FrameView::layout):
(WebCore::FrameView::performPostLayoutTasks):
* page/FrameView.h:

2017-10-19 Chris Dumez <cdumez@apple.com>

Unreviewed, revert r223650 as it caused crashes on the bots.
@@ -242,7 +242,6 @@ FrameView::FrameView(Frame& frame)
, m_canHaveScrollbars(true)
, m_layoutTimer(*this, &FrameView::layoutTimerFired)
, m_layoutPhase(OutsideLayout)
, m_inSynchronousPostLayout(false)
, m_postLayoutTasksTimer(*this, &FrameView::performPostLayoutTasks)
, m_updateEmbeddedObjectsTimer(*this, &FrameView::updateEmbeddedObjectsTimerFired)
, m_isTransparent(false)
@@ -341,7 +340,6 @@ void FrameView::reset()
m_needsFullRepaint = true;
m_layoutSchedulingEnabled = true;
m_layoutPhase = OutsideLayout;
m_inSynchronousPostLayout = false;
m_layoutCount = 0;
m_postLayoutTasksTimer.stop();
m_updateEmbeddedObjectsTimer.stop();
@@ -1395,13 +1393,9 @@ void FrameView::layout(bool allowSubtreeLayout)
bool isSubtreeLayout = false;
{
SetForScope<bool> changeSchedulingEnabled(m_layoutSchedulingEnabled, false);

if (!isLayoutNested() && !m_inSynchronousPostLayout && m_postLayoutTasksTimer.isActive() && !isInChildFrameWithFrameFlattening()) {
// This is a new top-level layout. If there are any remaining tasks from the previous
// layout, finish them now.
SetForScope<bool> inSynchronousPostLayoutChange(m_inSynchronousPostLayout, true);
// If this is a new top-level layout and there are any remaining tasks from the previous layout, finish them now.
if (!isLayoutNested() && m_postLayoutTasksTimer.isActive() && !isInChildFrameWithFrameFlattening())
performPostLayoutTasks();
}

// Viewport-dependent media queries may cause us to need completely different style information.
auto* styleResolver = document.styleScope().resolverIfExists();
@@ -1553,16 +1547,14 @@ void FrameView::layout(bool allowSubtreeLayout)
frame().document()->markers().invalidateRectsForAllMarkers();

if (!m_postLayoutTasksTimer.isActive()) {
if (!m_inSynchronousPostLayout) {
if (!m_inPerformPostLayoutTasks) {
if (isInChildFrameWithFrameFlattening())
updateWidgetPositions();
else {
SetForScope<bool> inSynchronousPostLayoutChange(m_inSynchronousPostLayout, true);
else
performPostLayoutTasks(); // Calls resumeScheduledEvents().
}
}

if (!m_postLayoutTasksTimer.isActive() && (needsLayout() || m_inSynchronousPostLayout || isInChildFrameWithFrameFlattening())) {
if (!m_postLayoutTasksTimer.isActive() && (needsLayout() || m_inPerformPostLayoutTasks || isInChildFrameWithFrameFlattening())) {
// If we need layout or are already in a synchronous call to postLayoutTasks(),
// defer widget updates and event dispatch until after we return. postLayoutTasks()
// can make us need to update again, and we can get stuck in a nasty cycle unless
@@ -3501,6 +3493,10 @@ void FrameView::flushPostLayoutTasksQueue()

void FrameView::performPostLayoutTasks()
{
if (m_inPerformPostLayoutTasks)
return;

SetForScope<bool> inPerformPostLayoutTasks(m_inPerformPostLayoutTasks, true);
// FIXME: We should not run any JavaScript code in this function.
LOG(Layout, "FrameView %p performPostLayoutTasks", this);

@@ -805,7 +805,7 @@ class FrameView final : public ScrollView {

LayoutPhase m_layoutPhase;
bool m_layoutSchedulingEnabled;
bool m_inSynchronousPostLayout;
bool m_inPerformPostLayoutTasks { false };
int m_layoutCount;
enum class LayoutNestedState { NotInLayout, NotNested, Nested };
LayoutNestedState m_layoutNestedState { LayoutNestedState::NotInLayout };

0 comments on commit aeaf814

Please sign in to comment.