REGRESSION (r245043) [Mac WK2 Debug] ASSERTION FAILED: m_services.isEmpty() && transp...
authorjiewen_tan@apple.com <jiewen_tan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 12 Jun 2019 19:06:16 +0000 (19:06 +0000)
committerjiewen_tan@apple.com <jiewen_tan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 12 Jun 2019 19:06:16 +0000 (19:06 +0000)
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:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@246369 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/platform/mac-wk2/TestExpectations
Source/WebKit/ChangeLog
Source/WebKit/UIProcess/WebAuthentication/AuthenticatorManager.cpp

index df1ae1d..da86597 100644 (file)
@@ -1,3 +1,13 @@
+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>
 
         (Async scrolling) Handle 'position:fixed' inside 'position:sticky' correctly.
index 15be084..1ba4af1 100644 (file)
@@ -920,12 +920,6 @@ webkit.org/b/194916 fast/mediastream/MediaStream-video-element.html [ Pass Failu
 
 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 ]
 
 webkit.org/b/196403 imported/w3c/web-platform-tests/mediacapture-record/MediaRecorder-stop.html [ Pass Failure ]
index 2990317..3893362 100644 (file)
@@ -1,3 +1,21 @@
+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>
 
         Add mechanism and test case to check if ITP is active
index 913bc8a..2c08445 100644 (file)
@@ -130,9 +130,9 @@ void AuthenticatorManager::makeCredential(const Vector<uint8_t>& hash, const Pub
 
     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.
     m_pendingRequestData = { hash, true, options, { } };
@@ -149,9 +149,9 @@ void AuthenticatorManager::getAssertion(const Vector<uint8_t>& hash, const Publi
 
     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.
     m_pendingRequestData = { hash, false, { }, options };
@@ -174,8 +174,9 @@ void AuthenticatorManager::clearStateAsync()
 
 void AuthenticatorManager::clearState()
 {
+    if (m_pendingCompletionHandler)
+        return;
     m_pendingRequestData = { };
-    ASSERT(!m_pendingCompletionHandler);
     m_services.clear();
     m_authenticators.clear();
 }