Let CancelableRequest Execute callbacks which do delete this.

Let CancelableRequest Execute callbacks which do delete this.

By checking canceled() again before calling NotifyCompleted, we avoid
assumptions that we're not canceled from the callback.  This solves 2 issues:
1) The NOTREACHED in the provider which tries to remove the pending_request.
2) Trying to call DidExecute on a deleted consumer_.

BUG=77777, 82156
R=sky@chromium.org
TEST=python chrome/test/functional/imports.py imports.ImportsTest.testImportFirefoxDataTwice

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

[Sheridan Rawlins, 2011-05-10 22:16:14]

Ok, here's my fix to callbacks which do delete this.

Brett, please have a look as this is in CancelableRequest stuff.

Thanks,

-SCR

[sky, 2011-05-10 22:56:29]

http://codereview.chromium.org/7001015/diff/2002/content/browser/cancelable_r...
File content/browser/cancelable_request.h (right):

http://codereview.chromium.org/7001015/diff/2002/content/browser/cancelable_r...
content/browser/cancelable_request.h:667: if (!canceled_.IsSet())
If 'this' has been deleted by RunWIthParams, than you can't check canceled_ (or
anything).

[brettw, 2011-05-10 22:59:09]

I think this is OK. The thing that's getting deleted is the consumer. The
request itself should be ref'ed as a result of being run currently.

[brettw, 2011-05-10 22:59:19]

LGTM if my above understanding is correct.

[Sheridan Rawlins, 2011-05-10 23:09:48]

Brett, your understanding is correct .   I fixed the comment to make it more
focused on being canceled in the callback rather than the ambiguous |delete
this|.

http://codereview.chromium.org/7001015/diff/2002/content/browser/cancelable_r...
File content/browser/cancelable_request.h (right):

http://codereview.chromium.org/7001015/diff/2002/content/browser/cancelable_r...
content/browser/cancelable_request.h:667: if (!canceled_.IsSet())
Ok, so the comment is wrong.  It does technically do delete this, but
effectively it destroys the consumer_ object which causes the current request to
be canceled.  I'll fix the comment.

On 2011/05/10 22:56:29, sky wrote:
> If 'this' has been deleted by RunWIthParams, than you can't check canceled_
(or
> anything).

[sky, 2011-05-10 23:13:54]

Sorry, Brett's right. LGTM