Fixed crash in ResourceDispatcher::OnReceivedRedirect()

Fixed crash caused by ResourceDispatcher::OnReceivedRedirect() indirectly running a nested mesage loop in request_info->peer->OnReceivedRedirect() call. The loop might kill the instance of PendingRequestInfo what caused this crash. The fix simply checks for validity of PendingRequestInfo one more time within this OnReceivedRedirect() method.

BUG=chromium:65345, chromium-os:10721
TEST=sign up with gmail, try to switch account to another one - there should be no crash. see http://crosbug.com/10721 for more detailed repro steps


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

[jam, 2011-01-12 05:48:01]

lgtm, please add a small comment in the code as it's not obvious when
reading the code
On Jan 11, 2011 6:42 PM, <zelidrag@chromium.org> wrote:
> Reviewers: John Abd-El-Malek,
>
> Description:
> Fixed crash caused by ResourceDispatcher::OnReceivedRedirect() indirectly
> running a nested mesage loop in request_info->peer->OnReceivedRedirect()
> call.
> The loop might kill the instance of PendingRequestInfo what caused this
> crash.
> The fix simply checks for validity of PendingRequestInfo one more time
> within
> this OnReceivedRedirect() method.
>
> BUG=chromium:65345, chromium-os:10721
> TEST=sign up with gmail, try to switch account to another one - there
> should be
> no crash. see http://crosbug.com/10721 for more detailed repro steps
>
>
> Please review this at http://codereview.chromium.org/6211005/
>
> SVN Base: svn://svn.chromium.org/chrome/trunk/src/
>
> Affected files:
> M chrome/common/resource_dispatcher.cc
>
>
> Index: chrome/common/resource_dispatcher.cc
> ===================================================================
> --- chrome/common/resource_dispatcher.cc (revision 71052)
> +++ chrome/common/resource_dispatcher.cc (working copy)
> @@ -402,6 +402,9 @@
> if (request_info->peer->OnReceivedRedirect(new_url, info,
>
> &has_new_first_party_for_cookies,
> &new_first_party_for_cookies))
> {
> + request_info = GetPendingRequestInfo(request_id);
> + if (!request_info)
> + return;
> request_info->pending_redirect_message.reset(
> new ViewHostMsg_FollowRedirect(routing_id, request_id,
> has_new_first_party_for_cookies,
>
>

[jam, 2011-01-12 08:08:23]

also, while you have this reproducing, would be great to know exactly what
triggered the nested message loop.  that can cause other bugs.

On Tue, Jan 11, 2011 at 9:47 PM, John Abd-El-Malek <jam@chromium.org> wrote:

> lgtm, please add a small comment in the code as it's not obvious when
> reading the code
> On Jan 11, 2011 6:42 PM, <zelidrag@chromium.org> wrote:
> > Reviewers: John Abd-El-Malek,
> >
> > Description:
> > Fixed crash caused by ResourceDispatcher::OnReceivedRedirect() indirectly
> > running a nested mesage loop in request_info->peer->OnReceivedRedirect()
> > call.
> > The loop might kill the instance of PendingRequestInfo what caused this
> > crash.
> > The fix simply checks for validity of PendingRequestInfo one more time
> > within
> > this OnReceivedRedirect() method.
> >
> > BUG=chromium:65345, chromium-os:10721
> > TEST=sign up with gmail, try to switch account to another one - there
> > should be
> > no crash. see http://crosbug.com/10721 for more detailed repro steps
> >
> >
> > Please review this at http://codereview.chromium.org/6211005/
> >
> > SVN Base: svn://svn.chromium.org/chrome/trunk/src/
> >
> > Affected files:
> > M chrome/common/resource_dispatcher.cc
> >
> >
> > Index: chrome/common/resource_dispatcher.cc
> > ===================================================================
> > --- chrome/common/resource_dispatcher.cc (revision 71052)
> > +++ chrome/common/resource_dispatcher.cc (working copy)
> > @@ -402,6 +402,9 @@
> > if (request_info->peer->OnReceivedRedirect(new_url, info,
> >
> > &has_new_first_party_for_cookies,
> > &new_first_party_for_cookies))
> > {
> > + request_info = GetPendingRequestInfo(request_id);
> > + if (!request_info)
> > + return;
> > request_info->pending_redirect_message.reset(
> > new ViewHostMsg_FollowRedirect(routing_id, request_id,
> > has_new_first_party_for_cookies,
> >
> >
>