[Webkit-unassigned] [Bug 85072] [soup] URL of the ResourceResponse passed to willSendRequest is incorrect

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=85072


Gustavo Noronha (kov) <gns at gnome.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #139782|review?, commit-queue?      |review-, commit-queue-
               Flag|                            |




--- Comment #16 from Gustavo Noronha (kov) <gns at gnome.org>  2012-05-02 08:45:17 PST ---
(From update of attachment 139782)
View in context: https://bugs.webkit.org/attachment.cgi?id=139782&action=review

I agree with Sergio it would be better to land this while unskipping the tests. I had a couple of nits, so I'll mark this r- so it gets out of the review queue, but the code looks good to me.

> Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp:218
> +// Called afer receiving all headers for the message.

I don't think this comment adds value, I suggest removing it. However, it may be puzzling for someone reading the code to understand exactly why we handle got-headers and update d->m_response while doing it, so...

> Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp:236
> +    ResourceResponse response;
> +    response.updateFromSoupMessage(msg);
> +
> +    d->m_response = response;

... if you could add something to the effect of 'We need to have the original response to feed to willSendRequest in case we are redirected, so we store it here.' close to these lines, I think it would help our future readers =)

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.