- Revision
- 279518
- Author
- cdu...@apple.com
- Date
- 2021-07-02 15:26:31 -0700 (Fri, 02 Jul 2021)
Log Message
[MacOS wk1] crypto/workers/subtle/hrsa-postMessage-worker.html is a flaky failure
https://bugs.webkit.org/show_bug.cgi?id=227540
<rdar://problem/79977662>
Reviewed by Geoffrey Garen.
Source/WebCore:
The test was sending 2 messages to the worker, the first one with the public key
and the second one for the private key. As the worker would receive those messages,
it would do some checks synchronously and post a message back to the main thread
script with the result. As we could see from the test's expected results, the
main thread script would usually (although flakily) receive the result for the
private key *before* the result for the public key, which was wrong.
The reason for this was that serializing / deserializing the crypto key would
spin the worker's run loop and the worker would thus process incoming messages
while serializing / deserializing a crypto key from a previous message.
To address the issue, we now use a BinarySemaphore to wait for the main thread
to finish the wrapping / unwrapping, instead of spinning the run loop.
No new tests, unskipped existing test.
* workers/WorkerGlobalScope.cpp:
(WebCore::WorkerGlobalScope::wrapCryptoKey):
(WebCore::WorkerGlobalScope::unwrapCryptoKey):
LayoutTests:
Rebaseline test now that the output is correct (and non-flaky). Also unmark the test as flaky.
* crypto/workers/subtle/ec-postMessage-worker-expected.txt:
* crypto/workers/subtle/hrsa-postMessage-worker-expected.txt:
* crypto/workers/subtle/hrsa-postMessage-worker.html:
* platform/mac-wk1/TestExpectations:
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (279517 => 279518)
--- trunk/LayoutTests/ChangeLog 2021-07-02 21:19:29 UTC (rev 279517)
+++ trunk/LayoutTests/ChangeLog 2021-07-02 22:26:31 UTC (rev 279518)
@@ -1,5 +1,20 @@
2021-07-02 Chris Dumez <cdu...@apple.com>
+ [MacOS wk1] crypto/workers/subtle/hrsa-postMessage-worker.html is a flaky failure
+ https://bugs.webkit.org/show_bug.cgi?id=227540
+ <rdar://problem/79977662>
+
+ Reviewed by Geoffrey Garen.
+
+ Rebaseline test now that the output is correct (and non-flaky). Also unmark the test as flaky.
+
+ * crypto/workers/subtle/ec-postMessage-worker-expected.txt:
+ * crypto/workers/subtle/hrsa-postMessage-worker-expected.txt:
+ * crypto/workers/subtle/hrsa-postMessage-worker.html:
+ * platform/mac-wk1/TestExpectations:
+
+2021-07-02 Chris Dumez <cdu...@apple.com>
+
REGRESSION (r279427): [ Mac wk1 ] imported/w3c/web-platform-tests/html/rendering/replaced-elements/embedded-content/tall-cross-domain-iframe-in-scrolled.sub.html is timing out
https://bugs.webkit.org/show_bug.cgi?id=227637
<rdar://problem/80102396>
Modified: trunk/LayoutTests/crypto/workers/subtle/ec-postMessage-worker-expected.txt (279517 => 279518)
--- trunk/LayoutTests/crypto/workers/subtle/ec-postMessage-worker-expected.txt 2021-07-02 21:19:29 UTC (rev 279517)
+++ trunk/LayoutTests/crypto/workers/subtle/ec-postMessage-worker-expected.txt 2021-07-02 22:26:31 UTC (rev 279518)
@@ -3,6 +3,14 @@
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+PASS All checks passed in worker for public key
+PASS publicKey.type is 'public'
+PASS publicKey.extractable is true
+PASS publicKey.algorithm.name is 'ECDH'
+PASS publicKey.algorithm.namedCurve is 'P-256'
+PASS publicKey.usages is [ ]
+PASS exportedKey.x is publicKeyJSON.x
+PASS exportedKey.y is publicKeyJSON.y
PASS All checks passed in worker for private key
PASS privateKey.type is 'private'
PASS privateKey.extractable is true
@@ -12,14 +20,6 @@
PASS exportedKey.x is privateKeyJSON.x
PASS exportedKey.y is privateKeyJSON.y
PASS exportedKey.d is privateKeyJSON.d
-PASS All checks passed in worker for public key
-PASS publicKey.type is 'public'
-PASS publicKey.extractable is true
-PASS publicKey.algorithm.name is 'ECDH'
-PASS publicKey.algorithm.namedCurve is 'P-256'
-PASS publicKey.usages is [ ]
-PASS exportedKey.x is publicKeyJSON.x
-PASS exportedKey.y is publicKeyJSON.y
PASS successfullyParsed is true
TEST COMPLETE
Modified: trunk/LayoutTests/crypto/workers/subtle/hrsa-postMessage-worker-expected.txt (279517 => 279518)
--- trunk/LayoutTests/crypto/workers/subtle/hrsa-postMessage-worker-expected.txt 2021-07-02 21:19:29 UTC (rev 279517)
+++ trunk/LayoutTests/crypto/workers/subtle/hrsa-postMessage-worker-expected.txt 2021-07-02 22:26:31 UTC (rev 279518)
@@ -3,6 +3,14 @@
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+PASS All checks passed in worker for public key
+PASS publicKey.type is 'public'
+PASS publicKey.extractable is true
+PASS publicKey.algorithm.name is 'RSASSA-PKCS1-v1_5'
+PASS publicKey.algorithm.modulusLength is 2048
+PASS bytesToHexString(publicKey.algorithm.publicExponent) is '010001'
+PASS publicKey.algorithm.hash.name is 'SHA-256'
+PASS publicKey.usages is ['verify']
PASS All checks passed in worker for private key
PASS privateKey.type is 'private'
PASS privateKey.extractable is false
@@ -11,14 +19,6 @@
PASS bytesToHexString(privateKey.algorithm.publicExponent) is '010001'
PASS privateKey.algorithm.hash.name is 'SHA-256'
PASS privateKey.usages is ['sign']
-PASS All checks passed in worker for public key
-PASS publicKey.type is 'public'
-PASS publicKey.extractable is true
-PASS publicKey.algorithm.name is 'RSASSA-PKCS1-v1_5'
-PASS publicKey.algorithm.modulusLength is 2048
-PASS bytesToHexString(publicKey.algorithm.publicExponent) is '010001'
-PASS publicKey.algorithm.hash.name is 'SHA-256'
-PASS publicKey.usages is ['verify']
PASS successfullyParsed is true
TEST COMPLETE
Modified: trunk/LayoutTests/crypto/workers/subtle/hrsa-postMessage-worker.html (279517 => 279518)
--- trunk/LayoutTests/crypto/workers/subtle/hrsa-postMessage-worker.html 2021-07-02 21:19:29 UTC (rev 279517)
+++ trunk/LayoutTests/crypto/workers/subtle/hrsa-postMessage-worker.html 2021-07-02 22:26:31 UTC (rev 279518)
@@ -48,6 +48,8 @@
finishJSTest();
} else {
if (publicKey = evt.data.publicKey) {
+ if (count)
+ testFailed("Public key should have been received first");
testPassed("All checks passed in worker for public key");
shouldBe("publicKey.type", "'public'");
shouldBe("publicKey.extractable", "true");
@@ -57,6 +59,8 @@
shouldBe("publicKey.algorithm.hash.name", "'SHA-256'");
shouldBe("publicKey.usages", "['verify']");
} else if (privateKey = evt.data.privateKey) {
+ if (!count)
+ testFailed("Private key should have been received second");
testPassed("All checks passed in worker for private key");
shouldBe("privateKey.type", "'private'");
shouldBe("privateKey.extractable", "false");
Modified: trunk/LayoutTests/crypto/workers/subtle/rsa-postMessage-worker-expected.txt (279517 => 279518)
--- trunk/LayoutTests/crypto/workers/subtle/rsa-postMessage-worker-expected.txt 2021-07-02 21:19:29 UTC (rev 279517)
+++ trunk/LayoutTests/crypto/workers/subtle/rsa-postMessage-worker-expected.txt 2021-07-02 22:26:31 UTC (rev 279518)
@@ -3,6 +3,16 @@
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+PASS All checks passed in worker for public key
+PASS publicKey.type is 'public'
+PASS publicKey.extractable is true
+PASS publicKey.algorithm.name is 'RSAES-PKCS1-v1_5'
+PASS publicKey.algorithm.modulusLength is 2048
+PASS bytesToHexString(publicKey.algorithm.publicExponent) is '010001'
+PASS publicKey.algorithm.hash is undefined.
+PASS publicKey.usages is ['encrypt']
+PASS exportedKey.n is publicKeyJSON.n
+PASS exportedKey.e is publicKeyJSON.e
PASS All checks passed in worker for private key
PASS privateKey.type is 'private'
PASS privateKey.extractable is true
@@ -19,16 +29,6 @@
PASS exportedKey.dp is privateKeyJSON.dp
PASS exportedKey.dq is privateKeyJSON.dq
PASS exportedKey.qi is privateKeyJSON.qi
-PASS All checks passed in worker for public key
-PASS publicKey.type is 'public'
-PASS publicKey.extractable is true
-PASS publicKey.algorithm.name is 'RSAES-PKCS1-v1_5'
-PASS publicKey.algorithm.modulusLength is 2048
-PASS bytesToHexString(publicKey.algorithm.publicExponent) is '010001'
-PASS publicKey.algorithm.hash is undefined.
-PASS publicKey.usages is ['encrypt']
-PASS exportedKey.n is publicKeyJSON.n
-PASS exportedKey.e is publicKeyJSON.e
PASS successfullyParsed is true
TEST COMPLETE
Modified: trunk/LayoutTests/platform/mac-wk1/TestExpectations (279517 => 279518)
--- trunk/LayoutTests/platform/mac-wk1/TestExpectations 2021-07-02 21:19:29 UTC (rev 279517)
+++ trunk/LayoutTests/platform/mac-wk1/TestExpectations 2021-07-02 22:26:31 UTC (rev 279518)
@@ -1311,5 +1311,3 @@
webkit.org/b/227366 [ BigSur ] http/tests/preload/onload_event.html [ Pass Timeout ]
webkit.org/b/227516 fast/shadow-dom/style-resolver-sharing.html [ Pass Failure ]
-
-webkit.org/b/227540 [ Debug ] crypto/workers/subtle/hrsa-postMessage-worker.html [ Pass Failure ]
Modified: trunk/Source/WebCore/ChangeLog (279517 => 279518)
--- trunk/Source/WebCore/ChangeLog 2021-07-02 21:19:29 UTC (rev 279517)
+++ trunk/Source/WebCore/ChangeLog 2021-07-02 22:26:31 UTC (rev 279518)
@@ -1,3 +1,31 @@
+2021-07-02 Chris Dumez <cdu...@apple.com>
+
+ [MacOS wk1] crypto/workers/subtle/hrsa-postMessage-worker.html is a flaky failure
+ https://bugs.webkit.org/show_bug.cgi?id=227540
+ <rdar://problem/79977662>
+
+ Reviewed by Geoffrey Garen.
+
+ The test was sending 2 messages to the worker, the first one with the public key
+ and the second one for the private key. As the worker would receive those messages,
+ it would do some checks synchronously and post a message back to the main thread
+ script with the result. As we could see from the test's expected results, the
+ main thread script would usually (although flakily) receive the result for the
+ private key *before* the result for the public key, which was wrong.
+
+ The reason for this was that serializing / deserializing the crypto key would
+ spin the worker's run loop and the worker would thus process incoming messages
+ while serializing / deserializing a crypto key from a previous message.
+
+ To address the issue, we now use a BinarySemaphore to wait for the main thread
+ to finish the wrapping / unwrapping, instead of spinning the run loop.
+
+ No new tests, unskipped existing test.
+
+ * workers/WorkerGlobalScope.cpp:
+ (WebCore::WorkerGlobalScope::wrapCryptoKey):
+ (WebCore::WorkerGlobalScope::unwrapCryptoKey):
+
2021-07-02 Alan Bujtas <za...@apple.com>
[LFC][TFC] Keep track of both the fixed and percent maximum values for each column
Modified: trunk/Source/WebCore/workers/WorkerGlobalScope.cpp (279517 => 279518)
--- trunk/Source/WebCore/workers/WorkerGlobalScope.cpp 2021-07-02 21:19:29 UTC (rev 279517)
+++ trunk/Source/WebCore/workers/WorkerGlobalScope.cpp 2021-07-02 22:26:31 UTC (rev 279518)
@@ -59,6 +59,7 @@
#include <_javascript_Core/ScriptCallStack.h>
#include <wtf/IsoMallocInlines.h>
#include <wtf/Lock.h>
+#include <wtf/threads/BinarySemaphore.h>
namespace WebCore {
using namespace Inspector;
@@ -390,69 +391,30 @@
#if ENABLE(WEB_CRYPTO)
-class CryptoBufferContainer : public ThreadSafeRefCounted<CryptoBufferContainer> {
-public:
- static Ref<CryptoBufferContainer> create() { return adoptRef(*new CryptoBufferContainer); }
- Vector<uint8_t>& buffer() { return m_buffer; }
-
-private:
- Vector<uint8_t> m_buffer;
-};
-
-class CryptoBooleanContainer : public ThreadSafeRefCounted<CryptoBooleanContainer> {
-public:
- static Ref<CryptoBooleanContainer> create() { return adoptRef(*new CryptoBooleanContainer); }
- bool boolean() const { return m_boolean; }
- void setBoolean(bool boolean) { m_boolean = boolean; }
-
-private:
- std::atomic<bool> m_boolean { false };
-};
-
bool WorkerGlobalScope::wrapCryptoKey(const Vector<uint8_t>& key, Vector<uint8_t>& wrappedKey)
{
- Ref<WorkerGlobalScope> protectedThis(*this);
- auto resultContainer = CryptoBooleanContainer::create();
- auto doneContainer = CryptoBooleanContainer::create();
- auto wrappedKeyContainer = CryptoBufferContainer::create();
- thread().workerLoaderProxy().postTaskToLoader([resultContainer, key, wrappedKeyContainer, doneContainer, workerMessagingProxy = makeRef(downcast<WorkerMessagingProxy>(thread().workerLoaderProxy()))](ScriptExecutionContext& context) {
- resultContainer->setBoolean(context.wrapCryptoKey(key, wrappedKeyContainer->buffer()));
- doneContainer->setBoolean(true);
- workerMessagingProxy->postTaskForModeToWorkerOrWorkletGlobalScope([](ScriptExecutionContext& context) {
- ASSERT_UNUSED(context, context.isWorkerGlobalScope());
- }, WorkerRunLoop::defaultMode());
+ Ref protectedThis { *this };
+ bool success = false;
+ BinarySemaphore semaphore;
+ thread().workerLoaderProxy().postTaskToLoader([&semaphore, &success, &key, &wrappedKey](auto& context) {
+ success = context.wrapCryptoKey(key, wrappedKey);
+ semaphore.signal();
});
-
- auto waitResult = MessageQueueMessageReceived;
- while (!doneContainer->boolean() && waitResult != MessageQueueTerminated)
- waitResult = thread().runLoop().runInMode(this, WorkerRunLoop::defaultMode());
-
- if (doneContainer->boolean())
- wrappedKey.swap(wrappedKeyContainer->buffer());
- return resultContainer->boolean();
+ semaphore.wait();
+ return success;
}
bool WorkerGlobalScope::unwrapCryptoKey(const Vector<uint8_t>& wrappedKey, Vector<uint8_t>& key)
{
- Ref<WorkerGlobalScope> protectedThis(*this);
- auto resultContainer = CryptoBooleanContainer::create();
- auto doneContainer = CryptoBooleanContainer::create();
- auto keyContainer = CryptoBufferContainer::create();
- thread().workerLoaderProxy().postTaskToLoader([resultContainer, wrappedKey, keyContainer, doneContainer, workerMessagingProxy = makeRef(downcast<WorkerMessagingProxy>(thread().workerLoaderProxy()))](ScriptExecutionContext& context) {
- resultContainer->setBoolean(context.unwrapCryptoKey(wrappedKey, keyContainer->buffer()));
- doneContainer->setBoolean(true);
- workerMessagingProxy->postTaskForModeToWorkerOrWorkletGlobalScope([](ScriptExecutionContext& context) {
- ASSERT_UNUSED(context, context.isWorkerGlobalScope());
- }, WorkerRunLoop::defaultMode());
+ Ref protectedThis { *this };
+ bool success = false;
+ BinarySemaphore semaphore;
+ thread().workerLoaderProxy().postTaskToLoader([&semaphore, &success, &key, &wrappedKey](auto& context) {
+ success = context.unwrapCryptoKey(wrappedKey, key);
+ semaphore.signal();
});
-
- auto waitResult = MessageQueueMessageReceived;
- while (!doneContainer->boolean() && waitResult != MessageQueueTerminated)
- waitResult = thread().runLoop().runInMode(this, WorkerRunLoop::defaultMode());
-
- if (doneContainer->boolean())
- key.swap(keyContainer->buffer());
- return resultContainer->boolean();
+ semaphore.wait();
+ return success;
}
#endif // ENABLE(WEB_CRYPTO)