Skip to content

Commit 4c334d3

Browse files
committed
REGRESSION(311425@main) [GLIB] Flaky view-transition test crashes
https://bugs.webkit.org/show_bug.cgi?id=312629 Reviewed by Nikolas Zimmermann. The problem is that in ThreadedCompositor renderLayerTree can be called after invalidate. Before 311425@main we returned early because m_context is also set to nullptr in invalidate, but now the context is always nullptr when using skia compositor and we are assuming that a null texture mapper means we are using skia. This patch adds an Invalidated state to ThreadedCompositor to make sure we always return early from renderLayerTree after invalidate, and it always uses m_useSkia to check if we are using the skia compositor. Fixes: imported/w3c/web-platform-tests/css/css-view-transitions/navigation/pageswap-push-with-redirect.html imported/w3c/web-platform-tests/css/css-view-transitions/navigation/pageswap-skip-transition.html imported/w3c/web-platform-tests/css/css-view-transitions/navigation/pageswap-replace-navigation.html imported/w3c/web-platform-tests/css/css-view-transitions/navigation/pagereveal-with-view-transition.html * LayoutTests/platform/glib/TestExpectations: * Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/ThreadedCompositor.cpp: (WebKit::ThreadedCompositor::invalidate): (WebKit::ThreadedCompositor::isActive const): (WebKit::ThreadedCompositor::paintToCurrentGLContext): (WebKit::ThreadedCompositor::renderLayerTree): (WebKit::ThreadedCompositor::stateToString): (WebKit::ThreadedCompositor::scheduleUpdateLocked): (WebKit::ThreadedCompositor::frameComplete): * Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/ThreadedCompositor.h: Canonical link: https://commits.webkit.org/311654@main
1 parent 1f41867 commit 4c334d3

3 files changed

Lines changed: 16 additions & 13 deletions

File tree

LayoutTests/platform/glib/TestExpectations

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4683,11 +4683,6 @@ imported/w3c/web-platform-tests/css/css-view-transitions/navigation/custom-scrol
46834683
imported/w3c/web-platform-tests/css/css-view-transitions/navigation/auto-name-from-id.html [ ImageOnlyFailure ]
46844684
imported/w3c/web-platform-tests/css/css-view-transitions/start-skip-start.html [ Pass Failure ]
46854685

4686-
webkit.org/b/312629 imported/w3c/web-platform-tests/css/css-view-transitions/navigation/pageswap-push-with-redirect.html [ Crash Pass ]
4687-
webkit.org/b/312629 imported/w3c/web-platform-tests/css/css-view-transitions/navigation/pageswap-skip-transition.html [ Crash Pass ]
4688-
webkit.org/b/312629 imported/w3c/web-platform-tests/css/css-view-transitions/navigation/pageswap-replace-navigation.html [ Crash Pass ]
4689-
webkit.org/b/312629 imported/w3c/web-platform-tests/css/css-view-transitions/navigation/pagereveal-with-view-transition.html [ Crash Pass ]
4690-
46914686
imported/w3c/web-platform-tests/svg/text/reftests/transform-dynamic-change.html [ ImageOnlyFailure ]
46924687

46934688
# Kerning for SVG not working as expected.

Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/ThreadedCompositor.cpp

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -136,12 +136,12 @@ void ThreadedCompositor::invalidate()
136136
Locker locker { m_state.lock };
137137
stopRenderTimer();
138138
m_state.didCompositeRenderingUpdateFunction = nullptr;
139-
m_state.state = State::Idle;
139+
m_state.state = State::Invalidated;
140140
}
141141

142142
m_didCompositeRunLoopObserver->invalidate();
143143
m_workQueue->dispatchSync([this] {
144-
if (m_textureMapper && (!m_context || !m_context->makeContextCurrent()))
144+
if (!m_useSkia && (!m_context || !m_context->makeContextCurrent()))
145145
return;
146146

147147
// Update the scene at this point ensures the layers state are correctly propagated.
@@ -208,7 +208,7 @@ void ThreadedCompositor::resume()
208208
bool ThreadedCompositor::isActive() const
209209
{
210210
Locker locker { m_state.lock };
211-
return m_state.state != State::Idle;
211+
return m_state.state != State::Idle && m_state.state != State::Invalidated;
212212
}
213213

214214
void ThreadedCompositor::backgroundColorDidChange()
@@ -281,10 +281,10 @@ void ThreadedCompositor::flushCompositingState(const OptionSet<CompositionReason
281281

282282
void ThreadedCompositor::paintToCurrentGLContext(const TransformationMatrix& matrix, const IntSize& size, const OptionSet<CompositionReason>& reasons)
283283
{
284-
if (m_textureMapper)
285-
paintToTextureMapper(matrix, size, reasons);
286-
else
284+
if (m_useSkia)
287285
paintToSkiaCanvas(matrix, size, reasons);
286+
else
287+
paintToTextureMapper(matrix, size, reasons);
288288
}
289289

290290
void ThreadedCompositor::paintToTextureMapper(const TransformationMatrix& matrix, const IntSize& size, const OptionSet<CompositionReason>& reasons)
@@ -431,6 +431,9 @@ void ThreadedCompositor::renderLayerTree()
431431
{
432432
Locker locker { m_state.lock };
433433

434+
if (m_state.state == State::Invalidated)
435+
return;
436+
434437
// The timer has been stopped.
435438
if (!m_state.isRenderTimerActive)
436439
return;
@@ -449,7 +452,7 @@ void ThreadedCompositor::renderLayerTree()
449452
m_state.state = State::InProgress;
450453
}
451454

452-
if (m_textureMapper && (!m_context || !m_context->makeContextCurrent()))
455+
if (!m_useSkia && (!m_context || !m_context->makeContextCurrent()))
453456
return;
454457

455458
// Retrieve the scene attributes in a thread-safe manner.
@@ -531,6 +534,8 @@ ASCIILiteral ThreadedCompositor::stateToString(ThreadedCompositor::State state)
531534
return "InProgress"_s;
532535
case State::ScheduledWhileInProgress:
533536
return "ScheduledWhileInProgress"_s;
537+
case State::Invalidated:
538+
return "Invalidated"_s;
534539
}
535540
RELEASE_ASSERT_NOT_REACHED();
536541
}
@@ -554,6 +559,7 @@ void ThreadedCompositor::scheduleUpdateLocked()
554559
m_state.state = State::ScheduledWhileInProgress;
555560
break;
556561
case State::ScheduledWhileInProgress:
562+
case State::Invalidated:
557563
break;
558564
}
559565
}
@@ -568,6 +574,7 @@ void ThreadedCompositor::frameComplete()
568574
switch (m_state.state) {
569575
case State::Idle:
570576
case State::Scheduled:
577+
case State::Invalidated:
571578
break;
572579
case State::InProgress:
573580
if (m_state.reasons.contains(CompositionReason::RenderingUpdate) && m_state.isWaitingForTiles)

Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/ThreadedCompositor.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,8 @@ class ThreadedCompositor : public ThreadSafeRefCountedAndCanMakeThreadSafeWeakPt
133133
Idle,
134134
Scheduled,
135135
InProgress,
136-
ScheduledWhileInProgress
136+
ScheduledWhileInProgress,
137+
Invalidated
137138
};
138139
static ASCIILiteral stateToString(State);
139140

0 commit comments

Comments
 (0)