Bug 120681 - REGRESSION(r154977): Do not urlencode soup message on ResourceRequest::toSoupMessage()
Summary: REGRESSION(r154977): Do not urlencode soup message on ResourceRequest::toSoup...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-09-04 07:38 PDT by Andre Moreira Magalhaes
Modified: 2013-09-04 10:53 PDT (History)
12 users (show)

See Also:

Attachments
Patch (4.81 KB, patch)
2013-09-04 07:47 PDT, Andre Moreira Magalhaes 		gtk-ews: commit-queue-
Details | Formatted Diff | Diff
Patch (4.81 KB, patch)
2013-09-04 09:21 PDT, Andre Moreira Magalhaes 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Andre Moreira Magalhaes 2013-09-04 07:38:30 PDT 
This accidentally regressed in http://trac.webkit.org/changeset/154977 because the old code behaved correctly when toSoupMessage was called.
Comment 1 Andre Moreira Magalhaes 2013-09-04 07:47:03 PDT 
Created attachment 210458 [details]
Patch
Comment 2 kov's GTK+ EWS bot 2013-09-04 07:58:28 PDT 
Comment on attachment 210458 [details]
Patch

Attachment 210458 [details] did not pass gtk-ews (gtk):
Output: http://webkit-queues.appspot.com/results/1697288
Comment 3 EFL EWS Bot 2013-09-04 08:10:33 PDT 
Comment on attachment 210458 [details]
Patch

Attachment 210458 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/1685622
Comment 4 EFL EWS Bot 2013-09-04 08:12:20 PDT 
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
Comment 5 Andre Moreira Magalhaes 2013-09-04 09:21:57 PDT 
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.
Comment 6 Martin Robinson 2013-09-04 10:02:18 PDT 
This probably needs a test or to unskip a test.
Comment 7 Zan Dobersek 2013-09-04 10:42:45 PDT 
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.
Comment 8 Martin Robinson 2013-09-04 10:44:05 PDT 
(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?
Comment 10 Andre Moreira Magalhaes 2013-09-04 10:49:26 PDT 
(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.
Comment 11 Martin Robinson 2013-09-04 10:49:58 PDT 
Comment on attachment 210469 [details]
Patch

Let's do this thing.
Comment 12 WebKit Commit Bot 2013-09-04 10:52:58 PDT 
Comment on attachment 210469 [details]
Patch

Clearing flags on attachment: 210469

Committed r155049: <http://trac.webkit.org/changeset/155049>
Comment 13 WebKit Commit Bot 2013-09-04 10:53:02 PDT 
All reviewed patches have been landed.  Closing bug.