Bug 132017 - DocumentLoader::dataReceived assertion failed
Summary: DocumentLoader::dataReceived assertion failed
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-04-22 11:42 PDT by Alex Christensen
Modified: 2014-05-07 07:07 PDT (History)
8 users (show)

See Also:


Attachments
Patch (3.21 KB, patch)
2014-04-23 13:56 PDT, peavo
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Christensen 2014-04-22 11:42:06 PDT
Whenever I open any website via http in any recent WinCairo build, the last assert in DocumentLoader::dataReceived fails.  This is new within the last thousand changes or so.  This is the assertion that fails:

ASSERT(!m_response.isNull());

Does anyone else have this problem?  Does anyone know where it comes from or how to solve it?
Comment 1 peavo 2014-04-23 13:56:25 PDT
Created attachment 230005 [details]
Patch
Comment 2 Brent Fulgham 2014-04-23 16:29:40 PDT
Comment on attachment 230005 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=230005&action=review

Wow -- uninitialized variables.

Double-check that network tests still pass with this change. That 'didReceiveResponse' might be needed to flush the client state in cases where the response wasn't received earlier...

> Source/WebCore/platform/network/curl/ResourceHandleManager.cpp:-687
> -                d->client()->didReceiveResponse(job, d->m_response);

Its possible that this is needed in addition to the one you added above, to handle cases where the client did not receive a valid/complete response, but WebKit needs to be told to "move to the next state".

Maybe double-check that HTTP and network tests work properly after this change.
Comment 3 Alex Christensen 2014-04-23 19:17:34 PDT
> Maybe double-check that HTTP and network tests work properly after this change.

Not all the network tests pass with this change, but it makes WinLauncher usable again.  It undoes some changes from https://bugs.webkit.org/show_bug.cgi?id=127555 so I'd be interested in what they have to say about this.
Comment 4 peavo 2014-04-24 11:18:11 PDT
(In reply to comment #2)

Thanks for reviewing :)

> (From update of attachment 230005 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=230005&action=review
> 
> Wow -- uninitialized variables.
> 

The initialization was not needed to fix the problem, I only includeded it to be on the safe side.

> Double-check that network tests still pass with this change. That 'didReceiveResponse' might be needed to flush the client state in cases where the response wasn't received earlier...
> 
> > Source/WebCore/platform/network/curl/ResourceHandleManager.cpp:-687
> > -                d->client()->didReceiveResponse(job, d->m_response);
> 
> Its possible that this is needed in addition to the one you added above, to handle cases where the client did not receive a valid/complete response, but WebKit needs to be told to "move to the next state".
> 
> Maybe double-check that HTTP and network tests work properly after this change.

Where do I find these tests?
Comment 5 Alex Christensen 2014-04-24 13:36:58 PDT
I'm not authorized to cq+ this patch, otherwise I would.  Brent?

To run the tests, I use a command like this one:
perl Tools/Scripts/run-webkit-tests --debug --no-build --root WebKitBuild/Debug_WinCairo/bin32 http
http is the subdirectory of LayoutTests to run.  fast is also a good subdirectory to run if you don't want to run all 23000 tests, many of which fail right now.
Comment 6 peavo 2014-04-24 13:42:35 PDT
(In reply to comment #5)
> I'm not authorized to cq+ this patch, otherwise I would.  Brent?
> 
> To run the tests, I use a command like this one:
> perl Tools/Scripts/run-webkit-tests --debug --no-build --root WebKitBuild/Debug_WinCairo/bin32 http
> http is the subdirectory of LayoutTests to run.  fast is also a good subdirectory to run if you don't want to run all 23000 tests, many of which fail right now.

Ok, thanks :)
Comment 7 peavo 2014-04-30 11:27:06 PDT
These are the http test result I get when running with this patch:

$ perl Tools/Scripts/run-webkit-tests --debug --no-build --root WebKitBuild/Debug_WinCairo/bin32 http

Found 1992 tests; running 223, skipping 1769.
/usr/sbin/lighttpd
The _NT_SYMBOL_PATH environment variable is not set. Using Microsoft Symbol Server.
Running 1 DumpRenderTree.

[25/223] http/tests/media/pdf-served-as-pdf.html failed unexpectedly (text diff)
[28/223] http/tests/media/video-cancel-load.html failed unexpectedly (text diff)
[45/223] http/tests/navigation/response204.html passed unexpectedly
[71/223] http/tests/security/xss-DENIED-getSVGDocument-iframe.html failed unexpectedly (text diff)
[72/223] http/tests/security/xss-DENIED-getSVGDocument-object.html failed unexpectedly (text diff)
[194/223] http/tests/w3c/webperf/approved/navigation-timing/html/test_performance_attributes_exist_in_object.html passed unexpectedly
[223/223] http/tests/xmlhttprequest/redirect-cross-origin-tripmine.html failed unexpectedly (text diff)

Retrying 5 unexpected failure(s) ...

Running 1 DumpRenderTree.

[1/5] http/tests/media/video-cancel-load.html failed unexpectedly (text diff)
[2/5] http/tests/media/pdf-served-as-pdf.html failed unexpectedly (text diff)
[3/5] http/tests/security/xss-DENIED-getSVGDocument-object.html failed unexpectedly (text diff)
[4/5] http/tests/security/xss-DENIED-getSVGDocument-iframe.html failed unexpectedly (text diff)
[5/5] http/tests/xmlhttprequest/redirect-cross-origin-tripmine.html failed unexpectedly (text diff)

216 tests ran as expected, 7 didn't:
Comment 8 peavo 2014-05-01 12:55:55 PDT
Without this patch, I get many failures (crashes) when running the http tests:

$ perl Tools/Scripts/run-webkit-tests --debug --no-build --root WebKitBuild/Debug_WinCairo/bin32 http
Using port 'win-future'
Test configuration: <future, x86, debug>
Placing test results in /cygdrive/c/cygwin/home/per.arne.vollan/WebKit/WebKitBuild/Debug_WinCairo/bin32/layout-test-results
Baseline search path: win -> mac-mountainlion -> mac -> generic
Using Debug build
Pixel tests disabled
Regular timeout: 35000, slow test timeout: 175000
Command line: /cygdrive/c/cygwin/home/per.arne.vollan/WebKit/WebKitBuild/Debug_WinCairo/bin32/DumpRenderTree -

--lint-test-files warnings:
LayoutTests/platform/win/TestExpectations:46 Path does not exist. fast/canvas/canvas-currentPath.html
LayoutTests/platform/win/TestExpectations:1424 Path does not exist. media/track/opera/interfaces/TextTrackList/onremovetrack.html
LayoutTests/platform/win/TestExpectations:2248 Path does not exist. svg/custom/masking-clipping-hidpi.svg
LayoutTests/platform/win/TestExpectations:2329 Path does not exist. fast/dom/is-protocol-handler-registered.html
LayoutTests/platform/win/TestExpectations:2332 Path does not exist. js/regress/emscripten-memops.html
LayoutTests/platform/win/TestExpectations:2336 Path does not exist. js/regress/chain-custom-getter.html
LayoutTests/platform/win/TestExpectations:2338 Path does not exist. js/regress/marsaglia.html
LayoutTests/platform/win/TestExpectations:2339 Path does not exist. js/regress/proto-custom-getter.html
LayoutTests/platform/win/TestExpectations:2341 Path does not exist. js/regress/simple-custom-getter.html
LayoutTests/platform/win/TestExpectations:2532 Path does not exist. csswg/contributors/adobe/submitted/shapes/shape-outside/shape-outside-floats-horizontal-rectangle-002.html

Found 1992 tests; running 223, skipping 1769.
/usr/sbin/lighttpd
The _NT_SYMBOL_PATH environment variable is not set. Using Microsoft Symbol Server.
Running 1 DumpRenderTree.

[1/223] http/tests/cache/subresource-expiration-1.html failed unexpectedly (DumpRenderTree crashed [pid=7120])
[2/223] http/tests/cache/subresource-expiration-2.html failed unexpectedly (DumpRenderTree crashed [pid=9540])
[3/223] http/tests/cookies/multiple-cookies.html failed unexpectedly (DumpRenderTree crashed [pid=6872])
[4/223] http/tests/cookies/single-quoted-value.html failed unexpectedly (DumpRenderTree crashed [pid=1852])
[5/223] http/tests/cookies/third-party-cookie-relaxing.html failed unexpectedly (DumpRenderTree crashed [pid=5196])
[6/223] http/tests/incremental/slow-utf8-text.pl failed unexpectedly (DumpRenderTree crashed [pid=7612])

...

[209/223] http/tests/w3c/webperf/submission/Google/resource-timing/html/test_resource_ignore_data_url.html failed unexpectedly (DumpRenderTree crashed [pid=9412])
[210/223] http/tests/w3c/webperf/submission/Google/resource-timing/html/test_resource_ignore_failures.html failed unexpectedly (DumpRenderTree crashed [pid=5544])
[211/223] http/tests/w3c/webperf/submission/Google/resource-timing/html/test_resource_initiator_types.html failed unexpectedly (DumpRenderTree crashed [pid=7280])
[212/223] http/tests/w3c/webperf/submission/Google/resource-timing/html/test_resource_redirects.html failed unexpectedly (DumpRenderTree crashed [pid=8076])
[213/223] http/tests/w3c/webperf/submission/Google/resource-timing/html/test_resource_reparenting.html failed unexpectedly (DumpRenderTree crashed [pid=7228])
[214/223] http/tests/w3c/webperf/submission/Google/resource-timing/html/test_resource_script_types.html failed unexpectedly (DumpRenderTree crashed [pid=6464])
[215/223] http/tests/xmlhttprequest/abort-should-cancel-load.html failed unexpectedly (DumpRenderTree crashed [pid=10608])
[216/223] http/tests/xmlhttprequest/access-control-basic-non-simple-deny-cached.html failed unexpectedly (DumpRenderTree crashed [pid=7236])
[217/223] http/tests/xmlhttprequest/access-control-basic-whitelist-request-headers.html failed unexpectedly (DumpRenderTree crashed [pid=10668])
[218/223] http/tests/xmlhttprequest/cache-override.html failed unexpectedly (DumpRenderTree crashed [pid=2700])
[219/223] http/tests/xmlhttprequest/cross-origin-no-authorization.html failed unexpectedly (DumpRenderTree crashed [pid=2404])
[220/223] http/tests/xmlhttprequest/redirect-cross-origin-post-sync.html failed unexpectedly (DumpRenderTree crashed [pid=10816])
[221/223] http/tests/xmlhttprequest/redirect-cross-origin-sync-double.html failed unexpectedly (DumpRenderTree crashed [pid=7284])
[222/223] http/tests/xmlhttprequest/redirect-cross-origin-sync.html failed unexpectedly (DumpRenderTree crashed [pid=5488])

18 tests ran as expected, 205 didn't:
Comment 9 peavo 2014-05-02 13:19:07 PDT
cq+?  ;)
Comment 10 WebKit Commit Bot 2014-05-07 07:07:31 PDT
Comment on attachment 230005 [details]
Patch

Clearing flags on attachment: 230005

Committed r168425: <http://trac.webkit.org/changeset/168425>
Comment 11 WebKit Commit Bot 2014-05-07 07:07:38 PDT
All reviewed patches have been landed.  Closing bug.