Bug 197093 - REGRESSION (r230681): Do not use stored credentials if WKBundlePageResourceLoadClient.shouldUseCredentialStorage returns false
Summary: REGRESSION (r230681): Do not use stored credentials if WKBundlePageResourceLo...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alex Christensen
URL:
Keywords: InRadar, Regression
Depends on: 184240
Blocks:
  Show dependency tree / graph
 
Reported: 2019-04-18 23:12 PDT by Alex Christensen
Modified: 2019-07-06 19:34 PDT (History)
5 users (show)

See Also:

Attachments
Patch (27.82 KB, patch)
2019-04-18 23:20 PDT, Alex Christensen 		no flags Details | Formatted Diff | Diff
Patch (27.83 KB, patch)
2019-04-19 10:03 PDT, Alex Christensen 		no flags Details | Formatted Diff | Diff
Patch (34.99 KB, patch)
2019-07-05 17:46 PDT, Sihui Liu 		no flags Details | Formatted Diff | Diff
Show Obsolete (3) 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 Alex Christensen 2019-04-18 23:12:56 PDT 
REGRESSION(r230681) Do not use stored credentials if WKBundlePageResourceLoadClient.shouldUseCredentialStorage returns false
Comment 1 Alex Christensen 2019-04-18 23:20:50 PDT 
Created attachment 367788 [details]
Patch
Comment 2 Alex Christensen 2019-04-19 10:03:16 PDT 
Created attachment 367805 [details]
Patch
Comment 3 youenn fablet 2019-04-22 14:58:18 PDT 
Comment on attachment 367805 [details]
Patch

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

> Source/WebKit/UIProcess/WebProcessPool.cpp:1724
> +void WebProcessPool::removeCredential(WebCore::Credential&& credential, WebCore::ProtectionSpace&& protectionSpace, CompletionHandler<void()>&& completionHandler)

Do we really need Credential&& and ProtectionSpace&&?
Comment 4 Chris Dumez 2019-04-22 14:59:12 PDT 
(In reply to youenn fablet from comment #3)
> Comment on attachment 367805 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=367805&action=review
> 
> > Source/WebKit/UIProcess/WebProcessPool.cpp:1724
> > +void WebProcessPool::removeCredential(WebCore::Credential&& credential, WebCore::ProtectionSpace&& protectionSpace, CompletionHandler<void()>&& completionHandler)
> 
> Do we really need Credential&& and ProtectionSpace&&?

It is coming from IPC so I think this is common practice.
Comment 5 youenn fablet 2019-04-22 15:07:28 PDT 
> > > Source/WebKit/UIProcess/WebProcessPool.cpp:1724
> > > +void WebProcessPool::removeCredential(WebCore::Credential&& credential, WebCore::ProtectionSpace&& protectionSpace, CompletionHandler<void()>&& completionHandler)
> > 
> > Do we really need Credential&& and ProtectionSpace&&?
> 
> It is coming from IPC so I think this is common practice.

NetworkProcess::removeCredential is coming from IPC and maybe in the future we might need r-values.
WebProcessPool::removeCredential is called directly from SPI and does not seem to need r-values.
Comment 6 Chris Dumez 2019-04-22 15:08:22 PDT 
(In reply to youenn fablet from comment #5)
> > > > Source/WebKit/UIProcess/WebProcessPool.cpp:1724
> > > > +void WebProcessPool::removeCredential(WebCore::Credential&& credential, WebCore::ProtectionSpace&& protectionSpace, CompletionHandler<void()>&& completionHandler)
> > > 
> > > Do we really need Credential&& and ProtectionSpace&&?
> > 
> > It is coming from IPC so I think this is common practice.
> 
> NetworkProcess::removeCredential is coming from IPC and maybe in the future
> we might need r-values.
> WebProcessPool::removeCredential is called directly from SPI and does not
> seem to need r-values.

Oh, right. I misread which one you were commenting on. Sorry.
Comment 7 Alex Christensen 2019-04-22 16:06:22 PDT 
https://trac.webkit.org/changeset/244521/webkit
Comment 8 Radar WebKit Bug Importer 2019-04-22 16:07:22 PDT 
<rdar://problem/50111468>
Comment 9 Sihui Liu 2019-07-05 17:46:42 PDT 
Reopening to attach new patch.
Comment 10 Sihui Liu 2019-07-05 17:46:43 PDT 
Created attachment 373556 [details]
Patch
Comment 11 Sihui Liu 2019-07-06 19:34:22 PDT 
The patch is not for this bug...