Bug 110059

Summary: [EFL][WK2] compositing/layer-creation/fixed-position-out-of-view-scaled.html is flaky
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: WebKit BREWMPAssignee: Dongseong Hwang <dongseong.hwang>
Status: RESOLVED FIXED    
Severity: Normal CC: cmarcelo, dongseong.hwang, kenneth, luiz, mikhail.pozdnyakov, noam, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Chris Dumez 2013-02-17 12:03:53 PST
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*)
Comment 1 Dongseong Hwang 2013-02-17 15:40:13 PST
oops, thank you for reporting.
Comment 2 Dongseong Hwang 2013-02-17 15:47:15 PST
Created attachment 188779 [details]
Patch
Comment 3 Kenneth Rohde Christiansen 2013-02-18 01:35:56 PST
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?
Comment 4 Dongseong Hwang 2013-02-18 01:57:29 PST
(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?
Comment 5 Chris Dumez 2013-02-18 02:03:12 PST
Please unskip the test as well.
Comment 6 Dongseong Hwang 2013-02-18 02:36:51 PST
Created attachment 188835 [details]
Patch
Comment 7 Dongseong Hwang 2013-02-18 02:42:00 PST
(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.
Comment 8 Chris Dumez 2013-02-18 02:58:44 PST
I had to skip compositing/layer-creation/fixed-position-out-of-view-scaled-scroll.html as well due to the same assertion.
Comment 9 Dongseong Hwang 2013-02-18 03:00:20 PST
(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 :)
Comment 10 Dongseong Hwang 2013-02-18 03:05:56 PST
Created attachment 188841 [details]
Patch
Comment 11 WebKit Review Bot 2013-02-18 04:13:39 PST
Comment on attachment 188841 [details]
Patch

Clearing flags on attachment: 188841

Committed r143194: <http://trac.webkit.org/changeset/143194>
Comment 12 WebKit Review Bot 2013-02-18 04:13:44 PST
All reviewed patches have been landed.  Closing bug.