Summary: | [GTK] Failing test media/video-document-types.html | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Philippe Normand <pnormand> | ||||||
Component: | WebKitGTK | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | gustavo, otte | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | PC | ||||||||
OS: | Linux | ||||||||
Attachments: |
|
Description
Philippe Normand
2009-11-11 07:48:21 PST
Created attachment 42962 [details]
Set document to parsing mode before parsing more data.
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? (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. 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-. (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. 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.
Yes that new patch is much better than what I came up with ;) Thanks for investigating on this Gustavo Comment on attachment 43093 [details]
proposed fix
Makes sense.
*** Bug 30161 has been marked as a duplicate of this bug. *** *** Bug 30363 has been marked as a duplicate of this bug. *** |