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

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>