Bug 146203 - [WinCairo] WebDownload::initWithRequest is not implemented.
Summary: [WinCairo] WebDownload::initWithRequest is not implemented.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-06-22 05:29 PDT by peavo
Modified: 2015-06-23 01:58 PDT (History)
2 users (show)

See Also:

Attachments
Patch (1.95 KB, patch)
2015-06-22 05:45 PDT, peavo 		no flags Details | Formatted Diff | Diff
Patch (1.42 KB, patch)
2015-06-22 14:21 PDT, peavo 		no flags Details | Formatted Diff | Diff
Patch (1.33 KB, patch)
2015-06-22 14:37 PDT, peavo 		achristensen: review+
Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description peavo 2015-06-22 05:29:13 PDT 
The method WebDownload::initWithRequest is not implemented on WinCairo.
Comment 1 peavo 2015-06-22 05:45:13 PDT 
Created attachment 255344 [details]
Patch
Comment 2 Alex Christensen 2015-06-22 12:29:03 PDT 
Comment on attachment 255344 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=255344&action=review

Does this make it so we can finally download things from within WinLauncher?

> Source/WebKit/win/WebDownloadCurl.cpp:186
> +        // If destination is already set, we don't need to ask delegate.
> +        if (!m_destination.isEmpty())
> +            return;

Is this correct?  What if the destination is already set but we want to change it?
Comment 3 peavo 2015-06-22 14:21:14 PDT 
Created attachment 255367 [details]
Patch
Comment 4 peavo 2015-06-22 14:24:26 PDT 
(In reply to comment #2)
>

Thanks for reviewing :)
 
> 
> Does this make it so we can finally download things from within WinLauncher?
> 

Unfortunately, no. WinLauncher crashes for me when I try to download a link, but I don't think it would take much work to fix this crash and get downloading from WinLauncher to work.

> > Source/WebKit/win/WebDownloadCurl.cpp:186
> > +        // If destination is already set, we don't need to ask delegate.
> > +        if (!m_destination.isEmpty())
> > +            return;
> 
> Is this correct?  What if the destination is already set but we want to
> change it?

Good point, I removed this in the latest patch.
Comment 5 Alex Christensen 2015-06-22 14:34:52 PDT 
Comment on attachment 255367 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=255367&action=review

Let's fix that crash and get downloading working, then!

> Source/WebKit/win/ChangeLog:12
> +        (WebDownload::didReceiveResponse): Do not ask delegate for destination if already set.

Remove this line.
Comment 6 peavo 2015-06-22 14:37:56 PDT 
Created attachment 255369 [details]
Patch
Comment 7 peavo 2015-06-22 14:44:00 PDT 
(In reply to comment #5)
> Comment on attachment 255367 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=255367&action=review
> 
> Let's fix that crash and get downloading working, then!
> 

I will try to debug the crash soon :)
Comment 8 Alex Christensen 2015-06-22 14:45:17 PDT 
Comment on attachment 255369 [details]
Patch

This looks good, but I don't think it should go in until we fix the crash that this makes possible.
Comment 9 peavo 2015-06-22 14:51:51 PDT 
(In reply to comment #8)
> Comment on attachment 255369 [details]
> Patch
> 
> This looks good, but I don't think it should go in until we fix the crash
> that this makes possible.

Thanks :) I believe WinLauncher crashes when downloading without this patch aswell. Is it OK if I commit this patch, and make a separate patch for the crash?
Comment 10 Alex Christensen 2015-06-22 14:52:42 PDT 
Comment on attachment 255369 [details]
Patch

Sure.
Comment 11 peavo 2015-06-23 01:58:59 PDT 
Committed r185868: <http://trac.webkit.org/changeset/185868>