Fix a crash caused by a race condition between the time the UrlmonUrlRequest:...

Fix a crash caused by a race condition between the time the UrlmonUrlRequest::EndRequestInternal task
executing and the OnStopBinding function getting called by Urlmon. If our task executes first we essentially
operate on an invalid object.

Fixes http://code.google.com/p/chromium/issues/detail?id=29033

Added a comment to the EndRequest function indicating that it is invalid to access any members after it
is called. Added a helper function to release the bindings and revoke the bind status callback.

Bug=29033
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=33551

[tommi (sloooow) - chröme, 2009-12-01 19:59:24]

http://codereview.chromium.org/456013/diff/1/2
File chrome_frame/urlmon_url_request.cc (right):

http://codereview.chromium.org/456013/diff/1/2#newcode311
chrome_frame/urlmon_url_request.cc:311: DLOG(INFO) << "OnStopBinding received
for request id: " << id();
should we remove the dlog above?

http://codereview.chromium.org/456013/diff/1/2#newcode805
chrome_frame/urlmon_url_request.cc:805:
::RevokeBindStatusCallback(bind_context_, this);
fyi - you should test this on all versions of IE.
IIRC on IE7 the BSC won't actually get revoked and you'll still get callbacks.

[ananta, 2009-12-01 22:13:01]

http://codereview.chromium.org/456013/diff/1/2
File chrome_frame/urlmon_url_request.cc (right):

http://codereview.chromium.org/456013/diff/1/2#newcode311
chrome_frame/urlmon_url_request.cc:311: DLOG(INFO) << "OnStopBinding received
for request id: " << id();
On 2009/12/01 19:59:24, tommi wrote:
> should we remove the dlog above?

Done.

http://codereview.chromium.org/456013/diff/1/2#newcode805
chrome_frame/urlmon_url_request.cc:805:
::RevokeBindStatusCallback(bind_context_, this);
On 2009/12/01 19:59:24, tommi wrote:
> fyi - you should test this on all versions of IE.
> IIRC on IE7 the BSC won't actually get revoked and you'll still get callbacks.

I tested on IE6 and IE8 and this seems to work fine. Out of curiosity I went
back in time to look at the revision just before the change to handle urlmon
requests on a background thread and the crash would happen there as well.

[tommi (sloooow) - chröme, 2009-12-02 01:13:22]

cool - lgtm

On Tue, Dec 1, 2009 at 5:13 PM, <ananta@chromium.org> wrote:

>
> http://codereview.chromium.org/456013/diff/1/2
> File chrome_frame/urlmon_url_request.cc (right):
>
> http://codereview.chromium.org/456013/diff/1/2#newcode311
> chrome_frame/urlmon_url_request.cc:311: DLOG(INFO) << "OnStopBinding
> received for request id: " << id();
> On 2009/12/01 19:59:24, tommi wrote:
>
>> should we remove the dlog above?
>>
>
> Done.
>
>
> http://codereview.chromium.org/456013/diff/1/2#newcode805
> chrome_frame/urlmon_url_request.cc:805:
> ::RevokeBindStatusCallback(bind_context_, this);
> On 2009/12/01 19:59:24, tommi wrote:
>
>> fyi - you should test this on all versions of IE.
>> IIRC on IE7 the BSC won't actually get revoked and you'll still get
>>
> callbacks.
>
> I tested on IE6 and IE8 and this seems to work fine. Out of curiosity I
> went back in time to look at the revision just before the change to
> handle urlmon requests on a background thread and the crash would happen
> there as well.
>
>
> http://codereview.chromium.org/456013
>