Bug 120681 – REGRESSION(r154977): Do not urlencode soup message on ResourceRequest::toSoupMessage()

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Andre Moreira Magalhaes]

This accidentally regressed in http://trac.webkit.org/changeset/154977 because the old code behaved correctly when toSoupMessage was called.

[Andre Moreira Magalhaes]

Created attachment 210458 [details]
Patch

[kov's GTK+ EWS bot]

Comment on attachment 210458 [details]
Patch

Attachment 210458 [details] did not pass gtk-ews (gtk):
Output: http://webkit-queues.appspot.com/results/1697288

[EFL EWS Bot]

Comment on attachment 210458 [details]
Patch

Attachment 210458 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/1685622

[EFL EWS Bot]

Comment on attachment 210458 [details]
Patch

Attachment 210458 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/1695468

[Andre Moreira Magalhaes]

Created attachment 210469 [details]
Patch

Updated patch that actually builds (I blame slow builds :D).

What happened is that the buildbot failed for media fragments tests on http://build.webkit.org/builders/GTK%20Linux%2064-bit%20Release/builds/40360 (78 failures over 50 failures on build 40359, all 28 new failures on media fragments tests) with only the changeset from http://trac.webkit.org/changeset/154977.

What I find strange is that all media fragments tests work fine here with and without this new patch to fix the regression (same results):
$ Tools/Scripts/run-webkit-tests --no-new-test-results --debug-rwt-logging --no-sample-on-timeout --webkit-test-runner LayoutTests/media/media-fragments/ --debug --gtk --iteration=1
...
=> Results: 68/68 tests passed (100.0%)

So, not sure what is happening with the buildbot failures.

[Martin Robinson]

This probably needs a test or to unskip a test.

[Zan Dobersek]

It fixes a bunch of media fragment failures on the builders that regressed with r154977. The patch is also just extracting duplicated code into a single method.

[Martin Robinson]

(In reply to comment #7)
> It fixes a bunch of media fragment failures on the builders that regressed with r154977. The patch is also just extracting duplicated code into a single method.

So the tests are failing now or are they skipped?

[Zan Dobersek]

Now.
http://build.webkit.org/results/GTK%20Linux%2064-bit%20Release/r155047%20(40401)/results.html

[Andre Moreira Magalhaes]

(In reply to comment #9)
> Now.
> http://build.webkit.org/results/GTK%20Linux%2064-bit%20Release/r155047%20(40401)/results.html

Point is they don't fail here (local) even without this patch, wonder why :/. In any case I think this patch is correct and restores the behaviour of toSoupMessage (apart from also updating the message to respect acceptEncoding) before the changeset that caused the regressions.

[Martin Robinson]

Comment on attachment 210469 [details]
Patch

Let's do this thing.

[WebKit Commit Bot]

Comment on attachment 210469 [details]
Patch

Clearing flags on attachment: 210469

Committed r155049: <http://trac.webkit.org/changeset/155049>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.