Overview: EWebLaucher crashes on start loading google page. In fact in my case it is redirected to google.pl. Steps to reproduce: Run EWebLauncher without any parameters. The crash happens in WebCore/loader/DocumentWriter.cpp in line: m_parser->flush(this); if (!m_parser) return; Moving NULL-check above m_parser->flush(this); fixes this problem.
a backtrace: Program received signal SIGSEGV, Segmentation fault. 0xb58c8595 in WebCore::DocumentWriter::endIfNotLoadingMainResource (this=0x8212a28) at /home/michal/source/host/autobuilder/profiles/autobuilder/builds/WebKit/WebKit/Source/WebCore/loader/DocumentWriter.cpp:223 223 m_parser->flush(this); (gdb) bt #0 0xb58c8595 in WebCore::DocumentWriter::endIfNotLoadingMainResource (this=0x8212a28) at /home/michal/source/host/autobuilder/profiles/autobuilder/builds/WebKit/WebKit/Source/WebCore/loader/DocumentWriter.cpp:223 #1 0xb58c8518 in WebCore::DocumentWriter::end (this=0x8212a28) at /home/michal/source/host/autobuilder/profiles/autobuilder/builds/WebKit/WebKit/Source/WebCore/loader/DocumentWriter.cpp:209 #2 0xb58be053 in WebCore::DocumentLoader::finishedLoading (this=0x8212980) at /home/michal/source/host/autobuilder/profiles/autobuilder/builds/WebKit/WebKit/Source/WebCore/loader/DocumentLoader.cpp:288 #3 0xb58d86b8 in WebCore::FrameLoader::finishedLoading (this=0x820db68) at /home/michal/source/host/autobuilder/profiles/autobuilder/builds/WebKit/WebKit/Source/WebCore/loader/FrameLoader.cpp:2077 #4 0xb58eb9c4 in WebCore::MainResourceLoader::didFinishLoading (this=0x8211348, finishTime=0) at /home/michal/source/host/autobuilder/profiles/autobuilder/builds/WebKit/WebKit/Source/WebCore/loader/MainResourceLoader.cpp:485 #5 0xb58fd083 in WebCore::ResourceLoader::didFinishLoading (this=0x8211348, finishTime=0) at /home/michal/source/host/autobuilder/profiles/autobuilder/builds/WebKit/WebKit/Source/WebCore/loader/ResourceLoader.cpp:449 #6 0xb5eb0b5c in WebCore::readCallback (source=0x8162520, asyncResult=0xb2817488, data=0x0) at /home/michal/source/host/autobuilder/profiles/autobuilder/builds/WebKit/WebKit/Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp:792 #7 0xb7cb81e5 in async_ready_callback_wrapper (source_object=0x8162520, res=0xb2817488, user_data=0x0) at ginputstream.c:470 #8 0xb7cca114 in g_simple_async_result_complete (simple=0xb2817488) at gsimpleasyncresult.c:749 #9 0xb7da7b98 in read_async_done (stream=0x8162520) at soup-http-input-stream.c:723 #10 0xb7da70ae in soup_http_input_stream_finished (msg=0x8160d08, stream=0x8162520) at soup-http-input-stream.c:310 #11 0xb7c42e8c in g_cclosure_marshal_VOID__VOID (closure=0x8214418, return_value=0x0, n_param_values=1, param_values=0xb2817218, invocation_hint=0xbfffd830, marshal_data=0xb7da7070) at gmarshal.c:79 #12 0xb7c332ca in g_closure_invoke (closure=0x8214418, return_value=0x0, n_param_values=1, param_values=0xb2817218, invocation_hint=0xbfffd830) at gclosure.c:771 #13 0xb7c4c013 in signal_emit_unlocked_R (node=<value optimized out>, detail=<value optimized out>, instance=0x8160d08, emission_return=0x0, instance_and_params=0xb2817218) at gsignal.c:3256 #14 0xb7c4d4f0 in g_signal_emit_valist (instance=0x8160d08, signal_id=23, detail=0, var_args=0xbfffd9fc "\364\377ܷ\271\220ڷ\364\377ܷh\332\377\277\262\302۷\b\r\026\bL\332\377\277\322>") at gsignal.c:2987 #15 0xb7c4dc02 in g_signal_emit (instance=0x8160d08, signal_id=23, detail=0) at gsignal.c:3044 #16 0xb7da90df in soup_message_finished (msg=0x8160d08) at soup-message.c:1086 #17 0xb7dbc2b2 in process_queue_item (item=0x8162e08, should_prune=<value optimized out>, loop=1) at soup-session-async.c:376 #18 0xb7dbc5f6 in run_queue (sa=<value optimized out>) at soup-session-async.c:418 #19 0xb7dbc6b8 in idle_run_queue (sa=0x807d430) at soup-session-async.c:441 #20 0xb7b3aeb1 in g_idle_dispatch (source=0x8208b38, callback=0, user_data=0x807d430) at gmain.c:4844 #21 0xb7b3dbe5 in g_main_dispatch (context=0x8163868) at gmain.c:2477 #22 g_main_context_dispatch (context=0x8163868) at gmain.c:3050 #23 0xb7e07483 in _ecore_glib_select__locked (ecore_fds=8, rfds=0xbfffdf48, wfds=0xbfffdec8, efds=0xbfffde48, ecore_timeout=0xbfffdfc8) at ecore_glib.c:154 #24 _ecore_glib_select (ecore_fds=8, rfds=0xbfffdf48, wfds=0xbfffdec8, efds=0xbfffde48, ecore_timeout=0xbfffdfc8) at ecore_glib.c:182 #25 0xb7e0240e in _ecore_main_select (timeout=0.042227788828313351) at ecore_main.c:1161 #26 0xb7e02d6f in _ecore_main_loop_iterate_internal (once_only=0) at ecore_main.c:1516 #27 0xb7e02db7 in ecore_main_loop_begin () at ecore_main.c:698 #28 0x0804c688 in main (argc=2, argv=0xbffff1d4) at /home/michal/source/host/autobuilder/profiles/autobuilder/builds/WebKit/WebKit/Tools/EWebLauncher/main.c:906
Created attachment 97095 [details] proposed patch
Attachment 97095 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/ChangeLog:1: ChangeLog entry has no bug number [changelog/bugnumber] [5] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 97095 [details] proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=97095&action=review > Source/WebCore/ChangeLog:4 > + Add Bug Title and Address
LGTM. This crash comes from Bug 62499. m_parser->flush(this) is called before checking if m_parser is null.
(In reply to comment #4) > (From update of attachment 97095 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=97095&action=review > > > Source/WebCore/ChangeLog:4 > > + > > Add Bug Title and Address Please upload new patch with new ChangeLog.
Comment on attachment 97095 [details] proposed patch Attachment 97095 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/8841205 New failing tests: fast/frames/set-parent-src-synchronously.xhtml fast/parser/iframe-sets-parent-to-javascript-url.html
Created attachment 97096 [details] Archive of layout-test-results from ec2-cr-linux-03 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: ec2-cr-linux-03 Port: Chromium Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
We have experienced the same crash here as well. It'd be nice if abarth could comment if the proposed patch makes sense.
Created attachment 97101 [details] fixed Changelog
Comment on attachment 97101 [details] fixed Changelog Attachment 97101 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/8833900 New failing tests: fast/frames/set-parent-src-synchronously.xhtml fast/parser/iframe-sets-parent-to-javascript-url.html
Created attachment 97104 [details] Archive of layout-test-results from ec2-cr-linux-03 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: ec2-cr-linux-03 Port: Chromium Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Comment on attachment 97101 [details] fixed Changelog Makes sense. How do we test this?
@abarth: looks like the cr-linux-ews is just confused.
(In reply to comment #14) > @abarth: looks like the cr-linux-ews is just confused. It's not confused. Those test failures are real. The patch is wrong.
Comment on attachment 97101 [details] fixed Changelog This patch causes crashes. Can you write a test that demonstrates the issue? It seems likely that this is a bug in the EFL WebKit Layer. You're probably calling some sort of end-ish function too many times.
(In reply to comment #16) > (From update of attachment 97101 [details]) > This patch causes crashes. > > Can you write a test that demonstrates the issue? It seems likely that this is a bug in the EFL WebKit Layer. You're probably calling some sort of end-ish function too many times. (In reply to comment #15) > (In reply to comment #14) > > @abarth: looks like the cr-linux-ews is just confused. > > It's not confused. Those test failures are real. The patch is wrong. The patch may be wrong, but this piece of code is wrong as well. m_parser cannot disappear after calling m_parser->flush(this), so why checking if m_parser is 0 after that(In reply to comment #16) > (From update of attachment 97101 [details]) > This patch causes crashes. > > Can you write a test that demonstrates the issue? It seems likely that this is a bug in the EFL WebKit Layer. You're probably calling some sort of end-ish function too many times. Looking the backtrace Michal provided, I can say for sure it's not specific to EFL. If DocumentWriter::endIfNotLoadingMainResource() cannot be called with m_parser == 0, it seems a problem in soup and therefore this can be reproduced in GTK port too. Michal, could you test this on GTK as well? Anyway, the patch might not be right but this piece of code does look wrong. It's not possible that m_parser becomes 0 after calling m_parser->flush(this) and the check should be removed or moved up like the patch Michal sent.
(In reply to comment #17) > Looking the backtrace Michal provided, I can say for sure it's not specific to EFL. If DocumentWriter::endIfNotLoadingMainResource() cannot be called with m_parser == 0, it seems a problem in soup and therefore this can be reproduced in GTK port too. Michal, could you test this on GTK as well? > Hmm, there is no crash on GTK port. (I'm using r88786 based on 2011.06.14.) As Adam said, it seems we need to check if this crash only occurs on EFL layer.
(In reply to comment #18) > (In reply to comment #17) > > Anyway, the patch might not be right but this piece of code does look wrong. It's not possible that m_parser becomes 0 after calling m_parser->flush(this) and the check should be removed or moved up like the patch Michal sent. Although it looks wrong, I think that it can be possible if m_parser->flush make recursion. I tested my assumption like below. @@ -220,11 +221,14 @@ void DocumentWriter::endIfNotLoadingMainResource() RefPtr<Frame> protector(m_frame); // FIXME: m_parser->finish() should imply m_parser->flush(). + static int counter = 0; + printf (" counter %d entered\n", ++counter); m_parser->flush(this); + printf (" counter %d leaved\n", counter--); if (!m_parser) return; m_parser->finish(); m_parser = 0; } # ./WebKitBuild/Release/Programs/EWebLauncher file:///workspace/webkits/efl-webkit/LayoutTests/fast/parser/iframe-sets-parent-to-javascript-url.html counter 1 entered counter 1 leaved counter 1 entered counter 2 entered counter 2 leaved counter 1 leaved counter 1 entered factor=1.000000, intFactor=100, zoomLevels[5]=100, zoomLevels[6]=110 counter 2 entered counter 2 leaved counter 1 leaved Michal, This issue is because FrameLoaderClientEfl doesn't implement makeRepresentation or revertToProvisionalState. WebKit/EFL should check it and clear parser in finishedLoading like other ports. However, I am studying why used setEncoding for it. :)
It seems like regardless, that "if" needs a comment (possibly from Adam?) explaining what its for.
Created attachment 98089 [details] Patch
(In reply to comment #21) > Created an attachment (id=98089) [details] > Patch I prepared patch instead of Michal after discussed. All of other ports call setEncoding in FrameLoderClient::finishedLoading() to remove document and clear parser, but WebKit/EFL didn't. With this patch, this crash will be passed. BTW, I don't know endIfNotLoadingMainResource should check isLoadingMainResource() which already set false in caller(). Only DocumentWriter::end() call endIfNotLoadingMainResource(). void DocumentWriter::end() { m_frame->loader()->didEndDocument(); // <- set m_isLoadingMainResource to false. endIfNotLoadingMainResource(); } void DocumentWriter::endIfNotLoadingMainResource() { if (m_frame->loader()->isLoadingMainResource() || !m_frame->page() || !m_frame->document()) return; ... }
(In reply to comment #22) > (In reply to comment #21) > > Created an attachment (id=98089) [details] [details] > > Patch > > I prepared patch instead of Michal after discussed. Please mark Michal's patch as obsolete then. > All of other ports call setEncoding in FrameLoderClient::finishedLoading() to remove document and clear parser, but WebKit/EFL didn't. Unfortunately the other ports don't say much about why it has been done this way: it was introduced in chromium in revision 50742 which just upstreamed a lot of code, in qt in revision 67268 which just copied chromium's behaviour to fix some tests and in gtk in revision 77596 due to unrelated reasons. The same patch was part of code we were going to upstream with EFL's DumpRenderTree anyway. If it also fixes this crash, I'm all for it.
Comment on attachment 98089 [details] Patch Could you rename m_hasRepresentation to something else?
(In reply to comment #24) > (From update of attachment 98089 [details]) > Could you rename m_hasRepresentation to something else? Not that I disagree, but may I ask why? It's what chromium, gtk and qt use. mac, OTOH, uses representationFinishedLoading.
(In reply to comment #25) > (In reply to comment #24) > > (From update of attachment 98089 [details] [details]) > > Could you rename m_hasRepresentation to something else? > > Not that I disagree, but may I ask why? It's what chromium, gtk and qt use. mac, OTOH, uses representationFinishedLoading. question is: what has a representation? representation of what?
(In reply to comment #26) > (In reply to comment #25) > > (In reply to comment #24) > > > (From update of attachment 98089 [details] [details] [details]) > > > Could you rename m_hasRepresentation to something else? > > > > Not that I disagree, but may I ask why? It's what chromium, gtk and qt use. mac, OTOH, uses representationFinishedLoading. > > question is: what has a representation? representation of what? As my poor reading of mac port, makeRepresentation() create representation which contains MIMEType and whether plugins allowed. (WebHTMLRepresentation? I am not sure.) When finishedLoading is called, representation checks whether it is pluginView, whether it is DisplayingWebArchive and so on. (Below code) (in WebHTMLRepresentation.mm) - (void)finishedLoadingWithDataSource:(WebDataSource *)dataSource { WebFrame* webFrame = [dataSource webFrame]; if (_private->pluginView) { [_private->manualLoader pluginViewFinishedLoading:_private->pluginView]; return; } if (!webFrame) return; if (![self _isDisplayingWebArchive]) { // Telling the frame we received some data and passing nil as the data is our // way to get work done that is normally done when the first bit of data is // received, even for the case of a document with no data (like about:blank). [webFrame _commitData:nil]; } WebView *webView = [webFrame webView]; if ([webView isEditable]) core(webFrame)->editor()->applyEditingStyleToBodyElement(); } WebKit/EFL (and other ports also) doesn't have representaion and just checked similar in FrameLoaderClient::finishedLoading. If you want, I can change it to m_hasDocumentRepresentation or m_hasHTMLRepresentation or your suggestion. But now, I'd like to keep current name like other ports except Mac port.
Comment on attachment 98089 [details] Patch Clearing flags on attachment: 98089 Committed r89572: <http://trac.webkit.org/changeset/89572>
All reviewed patches have been landed. Closing bug.