Bug 31352 – [GTK] Failing test media/video-document-types.html

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Philippe Normand]

Since the fix of Bug 30221, that test is re-enabled but crashes on the Debug slaves.

#0  0xf6c30b76 in WebCore::FrameLoader::addData (this=0x8440a7c, 
    bytes=0x8572c68 "\335\332\"\356\350#\325!:Ȣf#Q\223\1\32p+j@\36\3\320px\b\17D0\f\a\200\372\65\70C\36\373 \370\36\3\373\220x\b!\301\340 [", length=8192)
    at ../../WebCore/loader/FrameLoader.cpp:1465
        __PRETTY_FUNCTION__ = "void WebCore::FrameLoader::addData(const char*, int)"
#1  0xf66e5aa9 in WebKit::FrameLoaderClient::committedLoad (this=0x8440988, loader=0x8441db0, 
    data=0x8572c68 "\335\332\"\356\350#\325!:Ȣf#Q\223\1\32p+j@\36\3\320px\b\17D0\f\a\200\372\65\70C\36\373 \370\36\3\373\220x\b!\301\340 [", length=8192)
    at ../../WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:151
        encoding = {m_impl = {<WTF::FastAllocBase> = {<No data fields>}, m_ptr = 0x0}}
        userChosen = false
        frameLoader = 0x8440a7c
        __PRETTY_FUNCTION__ = "virtual void WebKit::FrameLoaderClient::committedLoad(WebCore::DocumentLoader*, const char*, int)"
#2  0xf6c2b2a6 in WebCore::FrameLoader::committedLoad (this=0x8440a7c, loader=0x8441db0, 
    data=0x8572c68 "\335\332\"\356\350#\325!:Ȣf#Q\223\1\32p+j@\36\3\320px\b\17D0\f\a\200\372\65\70C\36\373 \370\36\3\373\220x\b!\301\340 [", length=8192)
    at ../../WebCore/loader/FrameLoader.cpp:3209
No locals.
#3  0xf6c17e5b in WebCore::DocumentLoader::commitLoad (this=0x8441db0, 
    data=0x8572c68 "\335\332\"\356\350#\325!:Ȣf#Q\223\1\32p+j@\36\3\320px\b\17D0\f\a\200\372\65\70C\36\373 \370\36\3\373\220x\b!\301\340 [", length=8192)
    at ../../WebCore/loader/DocumentLoader.cpp:342
        frameLoader = 0x8440a7c
        protect = {<WTF::FastAllocBase> = {<No data fields>}, m_ptr = 0x8441db0}
#4  0xf6c17ec4 in WebCore::DocumentLoader::receivedData (this=0x8441db0, 
    data=0x8572c68 "\335\332\"\356\350#\325!:Ȣf#Q\223\1\32p+j@\36\3\320px\b\17D0\f\a\200\372\65\70C\36\373 \370\36\3\373\220x\b!\301\340 [", length=8192)
    at ../../WebCore/loader/DocumentLoader.cpp:354
No locals.
#5  0xf6c2d9b5 in WebCore::FrameLoader::receivedData (this=0x8440a7c, 
    data=0x8572c68 "\335\332\"\356\350#\325!:Ȣf#Q\223\1\32p+j@\36\3\320px\b\17D0\f\a\200\372\65\70C\36\373 \370\36\3\373\220x\b!\301\340 [", length=8192)
    at ../../WebCore/loader/FrameLoader.cpp:2061
No locals.
#6  0xf6c4564a in WebCore::MainResourceLoader::addData (this=0x8447130, 
    data=0x8572c68 "\335\332\"\356\350#\325!:Ȣf#Q\223\1\32p+j@\36\3\320px\b\17D0\f\a\200\372\65\70C\36\373 \370\36\3\373\220x\b!\301\340 [", length=8192, 
    allAtOnce=false) at ../../WebCore/loader/MainResourceLoader.cpp:143
No locals.
#7  0xf6c5059b in WebCore::ResourceLoader::didReceiveData (this=0x8447130, 
    data=0x8572c68 "\335\332\"\356\350#\325!:Ȣf#Q\223\1\32p+j@\36\3\320px\b\17D0\f\a\200\372\65\70C\36\373 \370\36\3\373\220x\b!\301\340 [", length=8192, 
    lengthReceived=16384, allAtOnce=false) at ../../WebCore/loader/ResourceLoader.cpp:248
        protector = {<WTF::FastAllocBase> = {<No data fields>}, m_ptr = 0x8447130}
#8  0xf6c44e18 in WebCore::MainResourceLoader::didReceiveData (this=0x8447130, 
    data=0x8572c68 "\335\332\"\356\350#\325!:Ȣf#Q\223\1\32p+j@\36\3\320px\b\17D0\f\a\200\372\65\70C\36\373 \370\36\3\373\220x\b!\301\340 [", length=8192, 
    lengthReceived=16384, allAtOnce=false) at ../../WebCore/loader/MainResourceLoader.cpp:374
        protect = {<WTF::FastAllocBase> = {<No data fields>}, m_ptr = 0x8447130}
        __PRETTY_FUNCTION__ = "virtual void WebCore::MainResourceLoader::didReceiveData(const char*, int, long long int, bool)"
#9  0xf6c4f8e0 in WebCore::ResourceLoader::didReceiveData (this=0x8447130, 
---Type <return> to continue, or q <return> to quit---
    data=0x8572c68 "\335\332\"\356\350#\325!:Ȣf#Q\223\1\32p+j@\36\3\320px\b\17D0\f\a\200\372\65\70C\36\373 \370\36\3\373\220x\b!\301\340 [", length=8192, 
    lengthReceived=16384) at ../../WebCore/loader/ResourceLoader.cpp:398
No locals.
#10 0xf70c97eb in readCallback (source=0x80a96f0, res=0x84098e0) at ../../WebCore/platform/network/soup/ResourceHandleSoup.cpp:736
        handle = {<WTF::FastAllocBase> = {<No data fields>}, m_ptr = 0x8442a18}
        d = 0x8443678
        client = 0x8447130
        error = 0x0
        bytesRead = 8192
#11 0xf4db65cf in async_ready_callback_wrapper (source_object=0x80a96f0, res=0x84098e0, user_data=0x0)
    at /build/buildd-glib2.0_2.22.1-1-i386-tx7y62/glib2.0-2.22.1/gio/ginputstream.c:471
No locals.
#12 0xf4dc4cd9 in IA__g_simple_async_result_complete (simple=0x84098e0)
    at /build/buildd-glib2.0_2.22.1-1-i386-tx7y62/glib2.0-2.22.1/gio/gsimpleasyncresult.c:588
        current_source = <value optimized out>
        current_context = 0x809b390
        __PRETTY_FUNCTION__ = "IA__g_simple_async_result_complete"
#13 0xf4dc500e in complete_in_idle_cb_for_thread (_data=0x80ee430) at /build/buildd-glib2.0_2.22.1-1-i386-tx7y62/glib2.0-2.22.1/gio/gsimpleasyncresult.c:650
        simple = 0x84098e0
#14 0xf4cb90b1 in g_idle_dispatch (source=0xf3300628, callback=0xbbadbeef, user_data=0x80ee430)
    at /build/buildd-glib2.0_2.22.1-1-i386-tx7y62/glib2.0-2.22.1/glib/gmain.c:4065
No locals.
#15 0xf4cbae98 in g_main_dispatch (context=0x809b390) at /build/buildd-glib2.0_2.22.1-1-i386-tx7y62/glib2.0-2.22.1/glib/gmain.c:1960
        __PRETTY_FUNCTION__ = "g_main_dispatch"
#16 IA__g_main_context_dispatch (context=0x809b390) at /build/buildd-glib2.0_2.22.1-1-i386-tx7y62/glib2.0-2.22.1/glib/gmain.c:2513
No locals.
#17 0xf4cbe623 in g_main_context_iterate (context=0x809b390, block=1, dispatch=1, self=0x8076218)
    at /build/buildd-glib2.0_2.22.1-1-i386-tx7y62/glib2.0-2.22.1/glib/gmain.c:2591
        max_priority = 0
        timeout = 0
        some_ready = 1
        nfds = 2
        allocated_nfds = <value optimized out>
        fds = 0xf3300478
        __PRETTY_FUNCTION__ = "g_main_context_iterate"
#18 0xf4cbeaea in IA__g_main_loop_run (loop=0xf3300468) at /build/buildd-glib2.0_2.22.1-1-i386-tx7y62/glib2.0-2.22.1/glib/gmain.c:2799
        self = 0x8076218
        __PRETTY_FUNCTION__ = "IA__g_main_loop_run"
#19 0xf50d8eb9 in gtk_main () from /usr/lib/libgtk-x11-2.0.so.0
No symbol table info available.
#20 0x080567e8 in runTest (testPathOrURL=...) at ../../WebKitTools/DumpRenderTree/gtk/DumpRenderTree.cpp:487
        pathOrURL = {static npos = 4294967295, 
---Type <return> to continue, or q <return> to quit---
          _M_dataplus = {<std::allocator<char>> = {<__gnu_cxx::new_allocator<char>> = {<No data fields>}, <No data fields>}, 
            _M_p = 0x80fdbdc "/tmp/LayoutTests/media/video-document-types.html"}}
        expectedPixelHash = {static npos = 4294967295, 
          _M_dataplus = {<std::allocator<char>> = {<__gnu_cxx::new_allocator<char>> = {<No data fields>}, <No data fields>}, _M_p = 0xf4b6073c ""}}
        separatorPos = 4294967295
        url = 0x0
        testURL = {static npos = 4294967295, 
          _M_dataplus = {<std::allocator<char>> = {<__gnu_cxx::new_allocator<char>> = {<No data fields>}, <No data fields>}, 
            _M_p = 0x80fddfc "file:///tmp/LayoutTests/media/video-document-types.html"}}
        isSVGW3CTest = false
        size = {x = 0, y = 0, width = 800, height = 600}
        bfList = 0x8075d20
        __PRETTY_FUNCTION__ = "void runTest(const std::string&)"
#21 0x08056ce7 in main (argc=2, argv=0xffffd784) at ../../WebKitTools/DumpRenderTree/gtk/DumpRenderTree.cpp:817
        i = 1
        options = {{name = 0x805cc6c "notree", has_arg = 0, flag = 0x805f740, val = 0}, {name = 0x805cc73 "pixel-tests", has_arg = 0, flag = 0x80608f4, 
            val = 1}, {name = 0x805cc7f "tree", has_arg = 0, flag = 0x805f740, val = 1}, {name = 0x0, has_arg = 0, flag = 0x0, val = 0}}
        option = -1

[Philippe Normand]

Created attachment 42962 [details]
Set document to parsing mode before parsing more data.

[Jan Alonzo]

Comment on attachment 42962 [details]
Set document to parsing mode before parsing more data.

> --- a/WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp
> +++ b/WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp
> @@ -147,8 +147,11 @@ void FrameLoaderClient::committedLoad(WebCore::DocumentLoader* loader, const cha
>  
>          FrameLoader* frameLoader = loader->frameLoader();
>          frameLoader->setEncoding(encoding, userChosen);
> -        if (data)
> +        if (data) {
> +            frameLoader->frame()->document()->setParsing(true);
>              frameLoader->addData(data, length);
> +            frameLoader->frame()->document()->setParsing(false);

Can you please expound more why we're doing this in the client interface?

[Gustavo Noronha (kov)]

(In reply to comment #2)

> > -        if (data)
> > +        if (data) {
> > +            frameLoader->frame()->document()->setParsing(true);
> >              frameLoader->addData(data, length);
> > +            frameLoader->frame()->document()->setParsing(false);
> 
> Can you please expound more why we're doing this in the client interface?

This looks wrong. Mac and Win do not seem to do this. Hrm.

[Gustavo Noronha (kov)]

Comment on attachment 42962 [details]
Set document to parsing mode before parsing more data.

>          FrameLoader* frameLoader = loader->frameLoader();
>          frameLoader->setEncoding(encoding, userChosen);
> -        if (data)
> +        if (data) {
> +            frameLoader->frame()->document()->setParsing(true);
>              frameLoader->addData(data, length);
> +            frameLoader->frame()->document()->setParsing(false);
> +        }
>      }

This crash seems to be caused by the MediaDocument stopping parsing prematurely:

bool MediaTokenizer::writeRawData(const char*, int)
{
    ASSERT(!m_mediaElement);
    if (m_mediaElement)
        return false;
        
    createDocumentStructure();
    finish();
    return false;
}
    
[...]
    
void MediaTokenizer::finish()
{
    if (!m_parserStopped) 
        m_doc->finishedParsing();
}

I wonder why this is done like this. Maybe Mac and Win are delegating the loading of the video to something else? Anyway, since this is only a problem with Media documents, I think what the patch is doing is fairly certainly hiding a different issue, so r-.

[Gustavo Noronha (kov)]

(In reply to comment #4)
> I wonder why this is done like this. Maybe Mac and Win are delegating the
> loading of the video to something else? Anyway, since this is only a problem
> with Media documents, I think what the patch is doing is fairly certainly
> hiding a different issue, so r-.

http://trac.webkit.org/changeset/36001


ooook... here's what: mac is apparently letting a plugin handle the load, which is why it tells the loader to finish the parsing, and then it cancels the main load in platform-specific code; we should either cancel the load, or not call that finish() there.

[Gustavo Noronha (kov)]

Created attachment 43093 [details]
proposed fix

ok, I found the problem; we need to cancel the main load, like mac does, and we do need to handle the "plugin will handle load" error specially, as we thought we probably should.

[Philippe Normand]

Yes that new patch is much better than what I came up with ;) Thanks for investigating on this Gustavo

[Xan Lopez]

Comment on attachment 43093 [details]
proposed fix

Makes sense.

[Gustavo Noronha (kov)]

Thanks. Landed as r51104.

[Gustavo Noronha (kov)]

*** Bug 30161 has been marked as a duplicate of this bug. ***

[Gustavo Noronha (kov)]

*** Bug 30363 has been marked as a duplicate of this bug. ***