Fixes crash at ResourceDispatcherHost::BeginDownload...

Fixes crash at ResourceDispatcherHost::BeginDownload
 - It is a 'planned' crash in a CHECK(false)
 - Now is a bug because the user can trigger it by alt-click in a  <a href=data:xxxxx >

The codepath in question skips all the checks we do in the downloadmanager and on the UI command handlers.

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

[Nicolas Sylvain, 2008-12-04 23:56:46]

We explicitly added the check because we wanted to know when someone was not
calling this function correctly.

I forgot a lot how this thing work, but I don't think we can just return like
that here. I think we need to create an ErrorRequest or something like that, or
we might leak stuff, or have requests that are never closed. Darin would know.

You should fix the caller instead.

[cpu_(ooo_6.6-7.5), 2008-12-05 01:54:09]

The call stack is just this:

1 ResourceDispatcherHost::BeginDownload
2 ResourceMessageFilter::OnDownloadUrl
3 IPC::MessageWithTuple (message from renderer)

At #3 is impossible, #2 is just pass thru, trivial function.

It simply does not go thru any of the places you expect.

For reference, in the renderer the top part of the stack is this:

#1 RenderView::DownloadUrl
#2 WebFrameLoaderClient::startDownload
#3 WebCore::FrameLoader::continueAfterNavigationPolicy

I just don't see were else.



On 2008/12/04 23:56:46, Nicolas Sylvain wrote:
> We explicitly added the check because we wanted to know when someone was not
> calling this function correctly.
> 
> I forgot a lot how this thing work, but I don't think we can just return like
> that here. I think we need to create an ErrorRequest or something like that,
or
> we might leak stuff, or have requests that are never closed. Darin would know.
> 
> You should fix the caller instead.

[cpu_(ooo_6.6-7.5), 2008-12-12 01:38:50]

Lonely CL seeking some love.

On 2008/12/05 01:54:09, cpu wrote:
> The call stack is just this:
> 
> 1 ResourceDispatcherHost::BeginDownload
> 2 ResourceMessageFilter::OnDownloadUrl
> 3 IPC::MessageWithTuple (message from renderer)
> 
> At #3 is impossible, #2 is just pass thru, trivial function.
> 
> It simply does not go thru any of the places you expect.
> 
> For reference, in the renderer the top part of the stack is this:
> 
> #1 RenderView::DownloadUrl
> #2 WebFrameLoaderClient::startDownload
> #3 WebCore::FrameLoader::continueAfterNavigationPolicy
> 
> I just don't see were else.
> 
> 
> 
> On 2008/12/04 23:56:46, Nicolas Sylvain wrote:
> > We explicitly added the check because we wanted to know when someone was not
> > calling this function correctly.
> > 
> > I forgot a lot how this thing work, but I don't think we can just return
like
> > that here. I think we need to create an ErrorRequest or something like that,
> or
> > we might leak stuff, or have requests that are never closed. Darin would
know.
> > 
> > You should fix the caller instead.

[darin (slow to review), 2008-12-12 15:44:17]

seems OK to me given that we just early return in the RendererSecurityPolicy
case too.

[Nicolas Sylvain, 2008-12-12 21:13:49]

lgtm ;)