Bug 167584

Summary: Allow to use stored credentials also for downloads started by DownloadManager::startDownload
Product: WebKit Reporter: Carlos Garcia Campos <cgarcia>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, bugs-noreply, calvaris, mcatanzaro
Priority: P2 Keywords: Gtk
Version: WebKit Local Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=167583
Attachments:
Description Flags
Patch mcatanzaro: review+

Description Carlos Garcia Campos 2017-01-30 05:27:40 PST
I don't know why this is the default in other ports, but I don't think it's what we want in GTK+.
Comment 1 Carlos Garcia Campos 2017-01-30 05:39:24 PST
Created attachment 300100 [details]
Patch
Comment 2 Michael Catanzaro 2017-01-30 06:24:30 PST
Comment on attachment 300100 [details]
Patch

r=me but please ask Alex Christensen before landing if you can do this cross-platform. Either this must be broken on Mac too, or he might have a different suggestion.
Comment 3 Carlos Garcia Campos 2017-01-30 22:04:42 PST
(In reply to comment #2)
> Comment on attachment 300100 [details]
> Patch
> 
> r=me but please ask Alex Christensen before landing if you can do this
> cross-platform. Either this must be broken on Mac too, or he might have a
> different suggestion.

Alex? :-)
Comment 4 Alex Christensen 2017-02-01 12:49:43 PST
Comment on attachment 300100 [details]
Patch

This should definitely have a test.  If we allow stored credentials in an ephemeral session, won't it just use the credentials in the CredentialStorage for that session?
Comment 5 Carlos Garcia Campos 2017-02-01 22:24:44 PST
(In reply to comment #4)
> Comment on attachment 300100 [details]
> Patch
> 
> This should definitely have a test.  If we allow stored credentials in an
> ephemeral session, won't it just use the credentials in the
> CredentialStorage for that session?

But we don't allow it. The patch allows stored credentials only for the default session, the problem is that PendingDownload is created with the default parameter for allow stored credentials unconditionally, which is to no allow them.
Comment 6 Michael Catanzaro 2017-02-07 08:01:12 PST
So this is good then...?
Comment 7 Alex Christensen 2017-02-13 23:55:29 PST
Comment on attachment 300100 [details]
Patch

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

> Source/WebKit2/NetworkProcess/Downloads/DownloadManager.cpp:64
> +#if PLATFORM(GTK)
> +    // The GTK+ port wants to allow using stored credentials also for pending downloads.
> +    parameters.allowStoredCredentials = sessionID.isEphemeral() ? DoNotAllowStoredCredentials : AllowStoredCredentials;
> +#endif

Let's make this the case on all ports.
Comment 8 Carlos Garcia Campos 2017-02-14 05:10:17 PST
Committed r212291: <http://trac.webkit.org/changeset/212291>