Bug 29736 – fast/xmlhttprequest/xmlhttprequest-missing-file-exception.html fails since r48752

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Andras Becsi]

If input text has length==0 the returned QString does not get initialized so a nullstring is returned.

[Andras Becsi]

Created attachment 40111 [details]
proposed patch

Fix decode method to return an empty string and not a null string if the length of the input is 0.

[Tor Arne Vestbø]

Comment on attachment 40111 [details]
proposed patch

Hmm, where is the code that gets affected by this? Ie, is there something that checks isNull() no a QString? It should be doing isEmpty or check length I'm guessing...

[Andras Becsi]

(In reply to comment #2)
> (From update of attachment 40111 [details])
> Hmm, where is the code that gets affected by this? Ie, is there something that
> checks isNull() no a QString? It should be doing isEmpty or check length I'm
> guessing...

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

Previously m_codec->toUnicode(bytes, length, &m_state) always returned a nonnull QString which was then returned. If length was 0 the string returned was empty but not null.
r48752 changed that. It creates a null string and if length is 0 the while loop does not run, so the returned string remains null, and here: http://build.webkit.org/results/Qt%20Linux%20Release/r48752%20%281911%29/results.html
you can see the failure in the layout test which is caused by this.

Now I see it probably would have been better to ping ariya whith this since he r+-sed this, but anyway.

[Andras Becsi]

Created attachment 40228 [details]
proposed patch v2

Update Changelog text not to be misleading.

[Andras Becsi]

Created attachment 40302 [details]
gdb dump of the backtrace on TextCodecQt::decode

[Andras Becsi]

(In reply to comment #5)
> Created an attachment (id=40302) [details]
> gdb dump of the backtrace on TextCodecQt::decode
Run on

[Andras Becsi]

(In reply to comment #2)
> (From update of attachment 40111 [details])
> Hmm, where is the code that gets affected by this? Ie, is there something that
> checks isNull() no a QString? It should be doing isEmpty or check length I'm
> guessing...

Here is the gdb backtrace:

#0  WebCore::TextCodecQt::decode (this=0x8286970, bytes=0x0, length=0, flush=false, sawError=@0x823e998) at ../../../WebCore/platform/text/qt/TextCodecQt.cpp:95
#1  0xb6e118e1 in WebCore::TextResourceDecoder::flush (this=0x823e968) at ../../../WebCore/loader/TextResourceDecoder.cpp:847
#2  0xb706d415 in WebCore::XMLHttpRequest::didFinishLoading (this=0x8288728, identifier=2) at ../../../WebCore/xml/XMLHttpRequest.cpp:815
#3  0xb6dc14f4 in WebCore::DocumentThreadableLoader::didFinishLoading (this=0x8289928, identifier=2) at ../../../WebCore/loader/DocumentThreadableLoader.cpp:240
#4  0xb6dc18df in WebCore::DocumentThreadableLoader::loadRequest (this=0x8289928, request=@0xbf8049b8, skipCanLoadCheck=false) at ../../../WebCore/loader/DocumentThreadableLoader.cpp:340
#5  0xb6dc324f in DocumentThreadableLoader (this=0x8289928, document=0x8252308, client=0x8288734, blockingBehavior=WebCore::DocumentThreadableLoader::LoadSynchronously, request=@0xbf8049b8,
    options=@0xbf804a58) at ../../../WebCore/loader/DocumentThreadableLoader.cpp:74
#6  0xb6dc36fa in WebCore::DocumentThreadableLoader::loadResourceSynchronously (document=0x8252308, request=@0xbf8049b8, client=@0x8288734, options=@0xbf804a58)
    at ../../../WebCore/loader/DocumentThreadableLoader.cpp:50
#7  0xb6e13706 in WebCore::ThreadableLoader::loadResourceSynchronously (context=0x8252338, request=@0xbf8049b8, client=@0x8288734, options=@0xbf804a58) at ../../../WebCore/loader/ThreadableLoader.cpp:69
#8  0xb706e7d3 in WebCore::XMLHttpRequest::createRequest (this=0x8288728, ec=@0xbf804b8c) at ../../../WebCore/xml/XMLHttpRequest.cpp:493
#9  0xb706ecc8 in WebCore::XMLHttpRequest::send (this=0x8288728, body=@0xbf804b28, ec=@0xbf804b8c) at ../../../WebCore/xml/XMLHttpRequest.cpp:407
#10 0xb706ed08 in WebCore::XMLHttpRequest::send (this=0x8288728, ec=@0xbf804b8c) at ../../../WebCore/xml/XMLHttpRequest.cpp:350
#11 0xb69ef36d in WebCore::JSXMLHttpRequest::send (this=0xb2042e40, exec=0xb20cf0f8, args=@0xbf804c00) at ../../../WebCore/bindings/js/JSXMLHttpRequestCustom.cpp:112
#12 0xb75ff687 in WebCore::jsXMLHttpRequestPrototypeFunctionSend (exec=0xb20cf0f8, thisValue=
          {u = {asEncodedJSValue = -5603316160, asDouble = -nan(0xffffeb2042e40), asBits = {payload = -1308348864, tag = -2}}}, args=@0xbf804c00) at generated/debug/JSXMLHttpRequest.cpp:385

(In reply to comment #6)
> (In reply to comment #5)
> > Created an attachment (id=40302) [details] [details]
> > gdb dump of the backtrace on TextCodecQt::decode
Run on fast/xmlhttprequest/xmlhttprequest-missing-file-exception.html

[Andras Becsi]

Sorry for the multiple posts.

[Andras Becsi]

Created attachment 40368 [details]
proposed patch v3

[WebKit Commit Bot]

Comment on attachment 40368 [details]
proposed patch v3

Rejecting patch 40368 from commit-queue.

Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--quiet', '--exit-after-n-failures=1']" exit_code: 1
Running build-dumprendertree
Running tests from /Users/eseidel/Projects/CommitQueue/LayoutTests
Testing 11355 test cases.
http/tests/workers/text-encoding.html -> crashed

Exiting early after 1 failures. 8950 tests run.
430.82s total testing time

8949 test cases (99%) succeeded
1 test case (<1%) crashed
5 test cases (<1%) had stderr output

[Eric Seidel (no email)]

Comment on attachment 40368 [details]
proposed patch v3

The commit queue already rejected this patch because it caused a layout test to crash? Why is this set commit-queue=? again?

[Eric Seidel (no email)]

Comment on attachment 40368 [details]
proposed patch v3

Actually, given that this does not affect any non-qt code, the crash makes no sense.  Maybe the text-encoding test is flakey.  cq+

[Eric Seidel (no email)]

I filed bug 29926 about the crash.  Sorry for the confusion.

[Andras Becsi]

(In reply to comment #13)
> I filed bug 29926 about the crash.  Sorry for the confusion.
Thanks.

[WebKit Commit Bot]

Comment on attachment 40368 [details]
proposed patch v3

Clearing flags on attachment: 40368

Committed r48929: <http://trac.webkit.org/changeset/48929>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.