Bug 197093

Summary: REGRESSION (r230681): Do not use stored credentials if WKBundlePageResourceLoadClient.shouldUseCredentialStorage returns false
Product: WebKit Reporter: Alex Christensen <achristensen>
Component: New BugsAssignee: Alex Christensen <achristensen>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, dbates, sihui_liu, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar, Regression
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 184240    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Alex Christensen
Reported 2019-04-18 23:12:56 PDT
REGRESSION(r230681) Do not use stored credentials if WKBundlePageResourceLoadClient.shouldUseCredentialStorage returns false
Attachments
Patch (27.82 KB, patch)
2019-04-18 23:20 PDT, Alex Christensen 		no flags
DetailsFormatted DiffDiff
Patch (27.83 KB, patch)
2019-04-19 10:03 PDT, Alex Christensen 		no flags
DetailsFormatted DiffDiff
Patch (34.99 KB, patch)
2019-07-05 17:46 PDT, Sihui Liu 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Alex Christensen
Comment 1 2019-04-18 23:20:50 PDT
Created attachment 367788 [details] Patch
Alex Christensen
Comment 2 2019-04-19 10:03:16 PDT
Created attachment 367805 [details] Patch
youenn fablet
Comment 3 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&&?
Chris Dumez
Comment 4 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.
youenn fablet
Comment 5 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.
Chris Dumez
Comment 6 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.
Alex Christensen
Comment 7 2019-04-22 16:06:22 PDT
https://trac.webkit.org/changeset/244521/webkit
Radar WebKit Bug Importer
Comment 8 2019-04-22 16:07:22 PDT
<rdar://problem/50111468>
Sihui Liu
Comment 9 2019-07-05 17:46:42 PDT
Reopening to attach new patch.
Sihui Liu
Comment 10 2019-07-05 17:46:43 PDT
Created attachment 373556 [details] Patch
Sihui Liu
Comment 11 2019-07-06 19:34:22 PDT
The patch is not for this bug...
Note You need to log in before you can comment on or make changes to this bug.