[WK2] Regression(r187691): If a page is showing an auth pane in one t…

…ab, any new tabs with same page hang until credentials are entered in first tab https://bugs.webkit.org/show_bug.cgi?id=151960 <rdar://problem/23618112> Reviewed by Alex Christensen. After r187691, if a page is showing an auth pane in one tab, any new tabs with same page hang until credentials are entered in first tab. This is because we coalescing all authentication challenges from the same domain, no matter what tab they are for. This can be confusing so we now only coalesce authentication challenges within each tab, by leveraging the pageID (in addition to the domain). * Shared/Authentication/AuthenticationManager.cpp: (WebKit::AuthenticationManager::shouldCoalesceChallenge): (WebKit::AuthenticationManager::coalesceChallengesMatching): (WebKit::AuthenticationManager::didReceiveAuthenticationChallenge): * Shared/Authentication/AuthenticationManager.h: Canonical link: https://commits.webkit.org/170103@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@193654 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · Dec 7, 2015 · 63296ef · 63296ef
1 parent 48fc953
commit 63296ef
Show file tree

Hide file tree

Showing 3 changed files with 43 additions and 13 deletions.
diff --git a/Source/WebKit2/ChangeLog b/Source/WebKit2/ChangeLog
@@ -1,3 +1,24 @@
+2015-12-07  Chris Dumez  <cdumez@apple.com>
+
+        [WK2] Regression(r187691): If a page is showing an auth pane in one tab, any new tabs with same page hang until credentials are entered in first tab
+        https://bugs.webkit.org/show_bug.cgi?id=151960
+        <rdar://problem/23618112>
+
+        Reviewed by Alex Christensen.
+
+        After r187691, if a page is showing an auth pane in one tab, any new
+        tabs with same page hang until credentials are entered in first tab.
+        This is because we coalescing all authentication challenges from the
+        same domain, no matter what tab they are for. This can be confusing
+        so we now only coalesce authentication challenges within each tab,
+        by leveraging the pageID (in addition to the domain).
+
+        * Shared/Authentication/AuthenticationManager.cpp:
+        (WebKit::AuthenticationManager::shouldCoalesceChallenge):
+        (WebKit::AuthenticationManager::coalesceChallengesMatching):
+        (WebKit::AuthenticationManager::didReceiveAuthenticationChallenge):
+        * Shared/Authentication/AuthenticationManager.h:
+
 2015-12-07  Beth Dakin  <bdakin@apple.com>
 
         Hook up request and show for typing candidates in WK1

diff --git a/Source/WebKit2/Shared/Authentication/AuthenticationManager.cpp b/Source/WebKit2/Shared/Authentication/AuthenticationManager.cpp
@@ -76,13 +76,13 @@ uint64_t AuthenticationManager::addChallengeToChallengeMap(const Challenge& chal
     return challengeID;
 }
 
-bool AuthenticationManager::shouldCoalesceChallenge(uint64_t challengeID, const AuthenticationChallenge& challenge) const
+bool AuthenticationManager::shouldCoalesceChallenge(uint64_t pageID, uint64_t challengeID, const AuthenticationChallenge& challenge) const
 {
     if (!canCoalesceChallenge(challenge))
         return false;
 
     for (auto& item : m_challenges) {
-        if (item.key != challengeID && ProtectionSpace::compare(challenge.protectionSpace(), item.value.challenge.protectionSpace()))
+        if (item.key != challengeID && item.value.pageID == pageID && ProtectionSpace::compare(challenge.protectionSpace(), item.value.challenge.protectionSpace()))
             return true;
     }
     return false;
@@ -100,7 +100,7 @@ Vector<uint64_t> AuthenticationManager::coalesceChallengesMatching(uint64_t chal
         return challengesToCoalesce;
 
     for (auto& item : m_challenges) {
-        if (item.key != challengeID && ProtectionSpace::compare(challenge.challenge.protectionSpace(), item.value.challenge.protectionSpace()))
+        if (item.key != challengeID && item.value.pageID == challenge.pageID && ProtectionSpace::compare(challenge.challenge.protectionSpace(), item.value.challenge.protectionSpace()))
             challengesToCoalesce.append(item.key);
     }
 
@@ -112,14 +112,15 @@ void AuthenticationManager::didReceiveAuthenticationChallenge(WebFrame* frame, c
     ASSERT(frame);
     ASSERT(frame->page());
 
-    uint64_t challengeID = addChallengeToChallengeMap({authenticationChallenge
+    auto pageID = frame->page()->pageID();
+    uint64_t challengeID = addChallengeToChallengeMap({pageID, authenticationChallenge
 #if USE(NETWORK_SESSION)
         , ChallengeCompletionHandler()
 #endif
     });
 
-    // Coalesce challenges in the same protection space.
-    if (shouldCoalesceChallenge(challengeID, authenticationChallenge))
+    // Coalesce challenges in the same protection space and in the same page.
+    if (shouldCoalesceChallenge(pageID, challengeID, authenticationChallenge))
         return;
 
     m_process->send(Messages::WebPageProxy::DidReceiveAuthenticationChallenge(frame->frameID(), authenticationChallenge, challengeID), frame->page()->pageID());
@@ -131,8 +132,10 @@ void AuthenticationManager::didReceiveAuthenticationChallenge(uint64_t pageID, u
     ASSERT(pageID);
     ASSERT(frameID);
 
-    uint64_t challengeID = addChallengeToChallengeMap({authenticationChallenge, completionHandler});
-    if (shouldCoalesceChallenge(challengeID, authenticationChallenge))
+    uint64_t challengeID = addChallengeToChallengeMap({pageID, authenticationChallenge, completionHandler});
+
+    // Coalesce challenges in the same protection space and in the same page.
+    if (shouldCoalesceChallenge(pageID, challengeID, authenticationChallenge))
         return;
 
     m_process->send(Messages::NetworkProcessProxy::DidReceiveAuthenticationChallenge(pageID, frameID, authenticationChallenge, challengeID));
@@ -143,12 +146,14 @@ void AuthenticationManager::didReceiveAuthenticationChallenge(uint64_t pageID, u
     ASSERT(pageID);
     ASSERT(frameID);
 
-    uint64_t challengeID = addChallengeToChallengeMap({authenticationChallenge
+    uint64_t challengeID = addChallengeToChallengeMap({pageID, authenticationChallenge
 #if USE(NETWORK_SESSION)
         , ChallengeCompletionHandler()
 #endif
     });
-    if (shouldCoalesceChallenge(challengeID, authenticationChallenge))
+
+    // Coalesce challenges in the same protection space and in the same page.
+    if (shouldCoalesceChallenge(pageID, challengeID, authenticationChallenge))
         return;
 
     m_process->send(Messages::NetworkProcessProxy::DidReceiveAuthenticationChallenge(pageID, frameID, authenticationChallenge, challengeID));
@@ -157,8 +162,11 @@ void AuthenticationManager::didReceiveAuthenticationChallenge(uint64_t pageID, u
 #if !USE(NETWORK_SESSION)
 void AuthenticationManager::didReceiveAuthenticationChallenge(Download* download, const AuthenticationChallenge& authenticationChallenge)
 {
-    uint64_t challengeID = addChallengeToChallengeMap({authenticationChallenge});
-    if (shouldCoalesceChallenge(challengeID, authenticationChallenge))
+    uint64_t dummyPageID = 0;
+    uint64_t challengeID = addChallengeToChallengeMap({dummyPageID, authenticationChallenge});
+
+    // Coalesce challenges in the same protection space and in the same page.
+    if (shouldCoalesceChallenge(dummyPageID, challengeID, authenticationChallenge))
         return;
 
     download->send(Messages::DownloadProxy::DidReceiveAuthenticationChallenge(authenticationChallenge, challengeID));

diff --git a/Source/WebKit2/Shared/Authentication/AuthenticationManager.h b/Source/WebKit2/Shared/Authentication/AuthenticationManager.h
@@ -74,6 +74,7 @@ class AuthenticationManager : public WebProcessSupplement, public NetworkProcess
 
 private:
     struct Challenge {
+        uint64_t pageID;
         WebCore::AuthenticationChallenge challenge;
 #if USE(NETWORK_SESSION)
         ChallengeCompletionHandler completionHandler;
@@ -86,7 +87,7 @@ class AuthenticationManager : public WebProcessSupplement, public NetworkProcess
     bool tryUseCertificateInfoForChallenge(const WebCore::AuthenticationChallenge&, const WebCore::CertificateInfo&);
 
     uint64_t addChallengeToChallengeMap(const Challenge&);
-    bool shouldCoalesceChallenge(uint64_t challengeID, const WebCore::AuthenticationChallenge&) const;
+    bool shouldCoalesceChallenge(uint64_t pageID, uint64_t challengeID, const WebCore::AuthenticationChallenge&) const;
 
     void useCredentialForSingleChallenge(uint64_t challengeID, const WebCore::Credential&, const WebCore::CertificateInfo&);
     void continueWithoutCredentialForSingleChallenge(uint64_t challengeID);