Changeset 246369 in webkit

Timestamp:

Jun 12, 2019 12:06:16 PM (5 years ago)

Author:

jiewen_tan@apple.com

Message:

REGRESSION (r245043) [Mac WK2 Debug] ASSERTION FAILED: m_services.isEmpty() && transports.size() <= maxTransportNumber seen with two http/wpt/webauthn/public-key-credential-* tests
​https://bugs.webkit.org/show_bug.cgi?id=197917
<rdar://problem/51524958>

Reviewed by Brent Fulgham.

Source/WebKit:

This is a race condition that when a new request comes in the middle between the previous one finishes and the clearStateAsync is queued in the main thread.
Therefore, when the new request starts discovery, it will still see previous request's state.

To fix this issue, clearState() will be called unconditionally for every request. And a guard is added to clearState() to prevent double clearance.

• UIProcess/WebAuthentication/AuthenticatorManager.cpp:

(WebKit::AuthenticatorManager::makeCredential):
(WebKit::AuthenticatorManager::getAssertion):
(WebKit::AuthenticatorManager::clearState):

LayoutTests:

• platform/mac-wk2/TestExpectations:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/platform/mac-wk2/TestExpectations (modified) (1 diff)
• Source/WebKit/ChangeLog (modified) (1 diff)
• Source/WebKit/UIProcess/WebAuthentication/AuthenticatorManager.cpp (modified) (3 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2019-06-12  Jiewen Tan  <jiewen_tan@apple.com>
+
+        REGRESSION (r245043) [Mac WK2 Debug] ASSERTION FAILED: m_services.isEmpty() && transports.size() <= maxTransportNumber seen with two http/wpt/webauthn/public-key-credential-* tests
+        https://bugs.webkit.org/show_bug.cgi?id=197917
+        <rdar://problem/51524958>
+
+        Reviewed by Brent Fulgham.
+
+        * platform/mac-wk2/TestExpectations:
+
 2019-06-12  Antti Koivisto  <antti@apple.com>
 

trunk/LayoutTests/platform/mac-wk2/TestExpectations

@@ -921 +921 @@
 webkit.org/b/196376 storage/domstorage/localstorage/private-browsing-affects-storage.html [ Pass Failure ]
 
-webkit.org/b/197917 [ Debug ] http/wpt/webauthn/public-key-credential-create-success-hid.https.html [ Skip ]
-webkit.org/b/197917 [ Debug ] http/wpt/webauthn/public-key-credential-get-success-hid.https.html [ Skip ]
-
-webkit.org/b/194780 http/wpt/webauthn/public-key-credential-create-success-hid.https.html [ Pass Failure ]
-webkit.org/b/196377 http/wpt/webauthn/public-key-credential-get-success-hid.https.html [ Pass Failure ]
-
 webkit.org/b/196400 fast/mediastream/MediaStreamTrack-getSettings.html [ Pass Failure ]
 

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2019-06-12  Jiewen Tan  <jiewen_tan@apple.com>
+
+        REGRESSION (r245043) [Mac WK2 Debug] ASSERTION FAILED: m_services.isEmpty() && transports.size() <= maxTransportNumber seen with two http/wpt/webauthn/public-key-credential-* tests
+        https://bugs.webkit.org/show_bug.cgi?id=197917
+        <rdar://problem/51524958>
+
+        Reviewed by Brent Fulgham.
+
+        This is a race condition that when a new request comes in the middle between the previous one finishes and the clearStateAsync is queued in the main thread.
+        Therefore, when the new request starts discovery, it will still see previous request's state.
+
+        To fix this issue, clearState() will be called unconditionally for every request. And a guard is added to clearState() to prevent double clearance.
+
+        * UIProcess/WebAuthentication/AuthenticatorManager.cpp:
+        (WebKit::AuthenticatorManager::makeCredential):
+        (WebKit::AuthenticatorManager::getAssertion):
+        (WebKit::AuthenticatorManager::clearState):
+
 2019-06-12  Brent Fulgham  <bfulgham@apple.com>
 

trunk/Source/WebKit/UIProcess/WebAuthentication/AuthenticatorManager.cpp

@@ -131 +131 @@
     if (m_pendingCompletionHandler) {
         m_pendingCompletionHandler(ExceptionData { NotAllowedError, "This request has been cancelled by a new request."_s });
-        clearState();
         m_requestTimeOutTimer.stop();
     }
+    clearState();
 
     // 1. Save request for async operations.
@@ -150 +150 @@
     if (m_pendingCompletionHandler) {
         m_pendingCompletionHandler(ExceptionData { NotAllowedError, "This request has been cancelled by a new request."_s });
-        clearState();
         m_requestTimeOutTimer.stop();
     }
+    clearState();
 
     // 1. Save request for async operations.
@@ -175 +175 @@
 void AuthenticatorManager::clearState()
 {
+    if (m_pendingCompletionHandler)
+        return;
     m_pendingRequestData = { };
-    ASSERT(!m_pendingCompletionHandler);
     m_services.clear();
     m_authenticators.clear();