Bug 110059 – [EFL][WK2] compositing/layer-creation/fixed-position-out-of-view-scaled.html is flaky

• Component: WebKit BREWMP
• Status: RESOLVED FIXED

[Chris Dumez]

compositing/layer-creation/fixed-position-out-of-view-scaled.html sometimes crashes on the bots:
crash log for WebKitTestRunner (pid 26034):
STDOUT: <empty>
STDERR: ASSERTION FAILED: m_fixedToViewport
STDERR: /home/buildslave-1/webkit-buildslave/efl-linux-64-debug-wk2/build/Source/WebCore/platform/graphics/texmap/TextureMapperLayer.cpp(606) : void WebCore::TextureMapperLayer::setScrollPositionDeltaIfNeeded(const WebCore::FloatSize&)
STDERR: 1   0x7fa8631ef93e WebCore::TextureMapperLayer::setScrollPositionDeltaIfNeeded(WebCore::FloatSize const&)
STDERR: 2   0x7fa8631d4f0a WebCore::CoordinatedGraphicsScene::adjustPositionForFixedLayers()
STDERR: 3   0x7fa8631d45c0 WebCore::CoordinatedGraphicsScene::paintToCurrentGLContext(WebCore::TransformationMatrix const&, float, WebCore::FloatRect const&, unsigned int)
STDERR: 4   0x7fa85f8f130c EwkView::paintToCurrentGLContext()
STDERR: 5   0x7fa85f8f1650 EwkView::displayTimerFired(WebCore::Timer<EwkView>*)
STDERR: 6   0x7fa85f8f774c WebCore::Timer<EwkView>::fired()
STDERR: 7   0x7fa86311d521 WebCore::ThreadTimers::sharedTimerFiredInternal()
STDERR: 8   0x7fa86311d435 WebCore::ThreadTimers::sharedTimerFired()
STDERR: 9   0x7fa863c9eba1
STDERR: 10  0x7fa85ea923de _ecore_timer_expired_call
STDERR: 11  0x7fa85ea925ab _ecore_timer_expired_timers_call
STDERR: 12  0x7fa85ea8f4b1
STDERR: 13  0x7fa85ea8fb47 ecore_main_loop_begin
STDERR: 14  0x4363cb WTR::TestController::platformRunUntil(bool&, double)
STDERR: 15  0x420214 WTR::TestController::runUntil(bool&, WTR::TestController::TimeoutDuration)
STDERR: 16  0x427a09 WTR::TestInvocation::invoke()
STDERR: 17  0x41ff34 WTR::TestController::runTest(char const*)
STDERR: 18  0x42006d WTR::TestController::runTestingServerLoop()
STDERR: 19  0x420107 WTR::TestController::run()
STDERR: 20  0x41d7a5 WTR::TestController::TestController(int, char const**)
STDERR: 21  0x436565 main
STDERR: 22  0x7fa85d5ee76d __libc_start_main
STDERR: 23  0x41c089
STDERR: ERROR: Thread name "com.apple.WebKit.ChildProcess.WatchDogQueue" is longer than 31 characters and will be truncated by Visual Studio
STDERR: /home/buildslave-1/webkit-buildslave/efl-linux-64-debug-wk2/build/Source/WTF/wtf/Threading.cpp(78) : WTF::ThreadIdentifier WTF::createThread(WTF::ThreadFunction, void*, const char*)

[Dongseong Hwang]

oops, thank you for reporting.

[Dongseong Hwang]

Created attachment 188779 [details]
Patch

[Kenneth Rohde Christiansen]

Comment on attachment 188779 [details]
Patch

You are not explaining why it is right to call the below code? Should the assert instead be moved into the if sentence?

[Dongseong Hwang]

(In reply to comment #3)
> (From update of attachment 188779 [details])
> You are not explaining why it is right to call the below code? Should the assert instead be moved into the if sentence?

Only one client calls TextureMapperLayer::setScrollPositionDeltaIfNeeded().
void CoordinatedGraphicsScene::adjustPositionForFixedLayers()
{
    ...
    LayerRawPtrMap::iterator end = m_fixedLayers.end();
    for (LayerRawPtrMap::iterator it = m_fixedLayers.begin(); it != end; ++it)
        toTextureMapperLayer(it->value)->setScrollPositionDeltaIfNeeded(delta);
}

It means CoordinatedGraphicsScene calls setScrollPositionDeltaIfNeeded() if TextureMapperLayer is a fixed layer.
So the assertion is correct except for a small moment between updating GraphicsLayer and synchronizing TextureMapperLayer.

After Bug 108294, the assertion will be always correct because updating GraphicsLayer and synchronizing TextureMapperLayer will be performed together.

Could I explain 'why it is right to call the below code' in changelog?

[Chris Dumez]

Please unskip the test as well.

[Dongseong Hwang]

Created attachment 188835 [details]
Patch

[Dongseong Hwang]

(In reply to comment #5)
> Please unskip the test as well.

yes.

Assertion cannot go into if statement.

'if statement' does not check if this layer is fixed layer.
'if statement' checks if this layer is nested fixed layer.

The assertion was created because we wanted to verify setScrollPositionDeltaIfNeeded() must be called when the layer is a fixed position layer.

But the time gap between update and flush makes the assumption not always true.

[Chris Dumez]

I had to skip compositing/layer-creation/fixed-position-out-of-view-scaled-scroll.html as well due to the same assertion.

[Dongseong Hwang]

(In reply to comment #8)
> I had to skip compositing/layer-creation/fixed-position-out-of-view-scaled-scroll.html as well due to the same assertion.

Ah, thank you :)

[Dongseong Hwang]

Created attachment 188841 [details]
Patch

[WebKit Review Bot]

Comment on attachment 188841 [details]
Patch

Clearing flags on attachment: 188841

Committed r143194: <http://trac.webkit.org/changeset/143194>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.