Bug 202500 – Only wrapping CryptoKeys for IDB during serialization

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Jiewen Tan]

Only wrapping CryptoKeys for IDB during serialization.

[Jiewen Tan]

<rdar://problem/52445927>

[Jiewen Tan]

Created attachment 380081 [details]
Patch

[Jiewen Tan]

Created attachment 380091 [details]
Patch

[Chris Dumez]

Comment on attachment 380091 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=380091&action=review

> Source/WebCore/workers/WorkerGlobalScope.cpp:417
> +    m_thread.workerLoaderProxy().postTaskToLoader([resultContainer = resultContainer.copyRef(), key, wrappedKeyContainer = wrappedKeyContainer.copyRef(), doneContainer = doneContainer.copyRef(), workerMessagingProxy = Ref<WorkerMessagingProxy>(downcast<WorkerMessagingProxy>(m_thread.workerLoaderProxy()))](ScriptExecutionContext& context) {

Ref'ing the WorkerMessagingProxy and passing it to another thread like this is not correct. If you look at the constructor / destructor of WorkerMessagingProxy, you'll see that this object needs to be constructed and destroyed on the same thread. The fact that it subclasses ThreadSafeRefCounted<> does not change that.

> Source/WebCore/workers/WorkerGlobalScope.cpp:420
> +        ((WorkerLoaderProxy&)workerMessagingProxy.get()).postTaskForModeToWorkerGlobalScope([](ScriptExecutionContext& context) {

No C-casts please.

> Source/WebCore/workers/WorkerGlobalScope.cpp:427
>          waitResult = m_thread.runLoop().runInMode(this, WorkerRunLoop::defaultMode());

What makes sure that |this| stays alive while spinning the run loop? I do not see you holding a Ref<> to the WorkerGlobalScope, which seems wrong.

> Tools/ChangeLog:9
> +        Modifies IndexedDB.StructuredCloneBackwardCompatibility test to include CryptoKeys.

Do these reproduce the crash?

[Jiewen Tan]

Comment on attachment 380091 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=380091&action=review

Thanks Chris for taking a look.

>> Source/WebCore/workers/WorkerGlobalScope.cpp:417
>> +    m_thread.workerLoaderProxy().postTaskToLoader([resultContainer = resultContainer.copyRef(), key, wrappedKeyContainer = wrappedKeyContainer.copyRef(), doneContainer = doneContainer.copyRef(), workerMessagingProxy = Ref<WorkerMessagingProxy>(downcast<WorkerMessagingProxy>(m_thread.workerLoaderProxy()))](ScriptExecutionContext& context) {
> 
> Ref'ing the WorkerMessagingProxy and passing it to another thread like this is not correct. If you look at the constructor / destructor of WorkerMessagingProxy, you'll see that this object needs to be constructed and destroyed on the same thread. The fact that it subclasses ThreadSafeRefCounted<> does not change that.

Thanks for pointing it out. I didn't consider that. However, it looks like that WorkerMessagingProxy expects itself to be destructed in main thread which is where this lambda is going to be executed. It looks fine for me then.

>> Source/WebCore/workers/WorkerGlobalScope.cpp:420
>> +        ((WorkerLoaderProxy&)workerMessagingProxy.get()).postTaskForModeToWorkerGlobalScope([](ScriptExecutionContext& context) {
> 
> No C-casts please.

Fixed.

>> Source/WebCore/workers/WorkerGlobalScope.cpp:427
>>          waitResult = m_thread.runLoop().runInMode(this, WorkerRunLoop::defaultMode());
> 
> What makes sure that |this| stays alive while spinning the run loop? I do not see you holding a Ref<> to the WorkerGlobalScope, which seems wrong.

Added.

>> Tools/ChangeLog:9
>> +        Modifies IndexedDB.StructuredCloneBackwardCompatibility test to include CryptoKeys.
> 
> Do these reproduce the crash?

No, none of the tests help reproduce the crash. The crash is very racy and it is very hard to reproduce. I will give it one more shot.

[Jiewen Tan]

Created attachment 380305 [details]
Patch

[Chris Dumez]

Comment on attachment 380305 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=380305&action=review

> Source/WebCore/ChangeLog:3
> +        Only wrapping CryptoKeys for IDB during serialization

EWS failures look legit.

[Chris Dumez]

(In reply to Chris Dumez from comment #7)
> Comment on attachment 380305 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=380305&action=review
> 
> > Source/WebCore/ChangeLog:3
> > +        Only wrapping CryptoKeys for IDB during serialization
> 
> EWS failures look legit.

For example:
ASSERTION FAILED: isActive()
./Modules/indexeddb/IDBTransaction.cpp(1254) : Ref<WebCore::IDBRequest> WebCore::IDBTransaction::requestPutOrAdd(JSC::ExecState &, WebCore::IDBObjectStore &, RefPtr<WebCore::IDBKey> &&, WebCore::SerializedScriptValue &, IndexedDB::ObjectStoreOverwriteMode)
1   0x102724019 WTFCrash
2   0x1120c834b WTFCrashWithInfo(int, char const*, char const*, int)
3   0x1138996dc WebCore::IDBTransaction::requestPutOrAdd(JSC::ExecState&, WebCore::IDBObjectStore&, WTF::RefPtr<WebCore::IDBKey, WTF::DumbPtrTraits<WebCore::IDBKey> >&&, WebCore::SerializedScriptValue&, WebCore::IndexedDB::ObjectStoreOverwriteMode)
4   0x113899325 WebCore::IDBObjectStore::putOrAdd(JSC::ExecState&, JSC::JSValue, WTF::RefPtr<WebCore::IDBKey, WTF::DumbPtrTraits<WebCore::IDBKey> >, WebCore::IndexedDB::ObjectStoreOverwriteMode, WebCore::IDBObjectStore::InlineKeyCheck)
5   0x113899433 WebCore::IDBObjectStore::put(JSC::ExecState&, JSC::JSValue, JSC::JSValue)
6   0x112d9e364 WebCore::jsIDBObjectStorePrototypeFunctionPutBody(JSC::ExecState*, WebCore::JSIDBObjectStore*, JSC::ThrowScope&)
7   0x112d812e0 long long WebCore::IDLOperation<WebCore::JSIDBObjectStore>::call<&(WebCore::jsIDBObjectStorePrototypeFunctionPutBody(JSC::ExecState*, WebCore::JSIDBObjectStore*, JSC::ThrowScope&)), (WebCore::CastedThisErrorBehavior)0>(JSC::ExecState&, char const*)
8   0x112d80fcc WebCore::jsIDBObjectStorePrototypeFunctionPut(JSC::ExecState*)
9   0x2d4ae99894b
10  0x102c63562 op_call_slow_return_location_narrow
11  0x102c63562 op_call_slow_return_location_narrow
12  0x102c47063 vmEntryToJavaScript
13  0x1039772ce JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*)
14  0x1039778ff JSC::Interpreter::executeCall(JSC::ExecState*, JSC::JSObject*, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&)
15  0x103c5531c JSC::call(JSC::ExecState*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&)
16  0x103c5540a JSC::call(JSC::ExecState*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&)
17  0x103c556fe JSC::profiledCall(JSC::ExecState*, JSC::ProfilingReason, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&)
18  0x113e9283b WebCore::JSExecState::profiledCall(JSC::ExecState*, JSC::ProfilingReason, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&)
19  0x113ee082a WebCore::JSEventListener::handleEvent(WebCore::ScriptExecutionContext&, WebCore::Event&)
20  0x1145332b3 WebCore::EventTarget::innerInvokeEventListeners(WebCore::Event&, WTF::Vector<WTF::RefPtr<WebCore::RegisteredEventListener, WTF::DumbPtrTraits<WebCore::RegisteredEventListener> >, 1ul, WTF::CrashOnOverflow, 16ul>, WebCore::EventTarget::EventInvokePhase)
21  0x11452eac2 WebCore::EventTarget::fireEventListeners(WebCore::Event&, WebCore::EventTarget::EventInvokePhase)
22  0x114508c67 WebCore::EventContext::handleLocalEvents(WebCore::Event&, WebCore::EventTarget::EventInvokePhase) const
23  0x114509781 WebCore::dispatchEventInDOM(WebCore::Event&, WebCore::EventPath const&)
24  0x114509ba2 void WebCore::dispatchEventWithType<WebCore::EventTarget>(WTF::Vector<WebCore::EventTarget*, 0ul, WTF::CrashOnOverflow, 16ul> const&, WebCore::Event&)
25  0x114509a3d WebCore::EventDispatcher::dispatchEvent(WTF::Vector<WebCore::EventTarget*, 0ul, WTF::CrashOnOverflow, 16ul> const&, WebCore::Event&)
26  0x1138a1f2a WebCore::IDBRequest::dispatchEvent(WebCore::Event&)
27  0x1138a1a4e WebCore::IDBOpenDBRequest::dispatchEvent(WebCore::Event&)
28  0x1160464a0 WebCore::WorkerEventQueue::EventDispatcher::dispatch()
29  0x116046414 WebCore::WorkerEventQueue::enqueueEvent(WTF::Ref<WebCore::Event, WTF::DumbPtrTraits<WebCore::Event> >&&)::$_1::operator()(WebCore::ScriptExecutionContext&) const
30  0x116046204 WTF::Detail::CallableWrapper<WebCore::WorkerEventQueue::enqueueEvent(WTF::Ref<WebCore::Event, WTF::DumbPtrTraits<WebCore::Event> >&&)::$_1, void, WebCore::ScriptExecutionContext&>::call(WebCore::ScriptExecutionContext&)
31  0x113d1ae00 WTF::Function<void (WebCore::ScriptExecutionContext&)>::operator()(WebCore::ScriptExecutionContext&) const

[Chris Dumez]

Comment on attachment 380305 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=380305&action=review

r- due to failures.

> Source/WebCore/workers/WorkerGlobalScope.cpp:418
> +    m_thread.workerLoaderProxy().postTaskToLoader([resultContainer = resultContainer.copyRef(), key, wrappedKeyContainer = wrappedKeyContainer.copyRef(), doneContainer = doneContainer.copyRef(), workerMessagingProxy = Ref<WorkerMessagingProxy>(downcast<WorkerMessagingProxy>(m_thread.workerLoaderProxy()))](ScriptExecutionContext& context) {

Why not use makeRef() instead of Ref<WorkerMessagingProxy>() ?

> Source/WebCore/workers/WorkerGlobalScope.cpp:421
> +        static_cast<WorkerLoaderProxy&>(workerMessagingProxy.get()).postTaskForModeToWorkerGlobalScope([](ScriptExecutionContext& context) {

Why is the static_cast() needed?

[Jiewen Tan]

(In reply to Chris Dumez from comment #7)
> Comment on attachment 380305 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=380305&action=review
> 
> > Source/WebCore/ChangeLog:3
> > +        Only wrapping CryptoKeys for IDB during serialization
> 
> EWS failures look legit.

Not sure how. Will talk to Brady or Sihui about this.

[Jiewen Tan]

Comment on attachment 380305 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=380305&action=review

>> Source/WebCore/workers/WorkerGlobalScope.cpp:418
>> +    m_thread.workerLoaderProxy().postTaskToLoader([resultContainer = resultContainer.copyRef(), key, wrappedKeyContainer = wrappedKeyContainer.copyRef(), doneContainer = doneContainer.copyRef(), workerMessagingProxy = Ref<WorkerMessagingProxy>(downcast<WorkerMessagingProxy>(m_thread.workerLoaderProxy()))](ScriptExecutionContext& context) {
> 
> Why not use makeRef() instead of Ref<WorkerMessagingProxy>() ?

Fixed.

>> Source/WebCore/workers/WorkerGlobalScope.cpp:421
>> +        static_cast<WorkerLoaderProxy&>(workerMessagingProxy.get()).postTaskForModeToWorkerGlobalScope([](ScriptExecutionContext& context) {
> 
> Why is the static_cast() needed?

postTaskForModeToWorkerGlobalScope is not exposed in WorkerMessagingProxy, and therefore I need to upcast the object to WorkerLoaderProxy to use it.

[Jiewen Tan]

Comment on attachment 380305 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=380305&action=review

>>>> Source/WebCore/ChangeLog:3
>>>> +        Only wrapping CryptoKeys for IDB during serialization
>>> 
>>> EWS failures look legit.
>> 
>> For example:
>> ASSERTION FAILED: isActive()
>> ./Modules/indexeddb/IDBTransaction.cpp(1254) : Ref<WebCore::IDBRequest> WebCore::IDBTransaction::requestPutOrAdd(JSC::ExecState &, WebCore::IDBObjectStore &, RefPtr<WebCore::IDBKey> &&, WebCore::SerializedScriptValue &, IndexedDB::ObjectStoreOverwriteMode)
>> 1   0x102724019 WTFCrash
>> 2   0x1120c834b WTFCrashWithInfo(int, char const*, char const*, int)
>> 3   0x1138996dc WebCore::IDBTransaction::requestPutOrAdd(JSC::ExecState&, WebCore::IDBObjectStore&, WTF::RefPtr<WebCore::IDBKey, WTF::DumbPtrTraits<WebCore::IDBKey> >&&, WebCore::SerializedScriptValue&, WebCore::IndexedDB::ObjectStoreOverwriteMode)
>> 4   0x113899325 WebCore::IDBObjectStore::putOrAdd(JSC::ExecState&, JSC::JSValue, WTF::RefPtr<WebCore::IDBKey, WTF::DumbPtrTraits<WebCore::IDBKey> >, WebCore::IndexedDB::ObjectStoreOverwriteMode, WebCore::IDBObjectStore::InlineKeyCheck)
>> 5   0x113899433 WebCore::IDBObjectStore::put(JSC::ExecState&, JSC::JSValue, JSC::JSValue)
>> 6   0x112d9e364 WebCore::jsIDBObjectStorePrototypeFunctionPutBody(JSC::ExecState*, WebCore::JSIDBObjectStore*, JSC::ThrowScope&)
>> 7   0x112d812e0 long long WebCore::IDLOperation<WebCore::JSIDBObjectStore>::call<&(WebCore::jsIDBObjectStorePrototypeFunctionPutBody(JSC::ExecState*, WebCore::JSIDBObjectStore*, JSC::ThrowScope&)), (WebCore::CastedThisErrorBehavior)0>(JSC::ExecState&, char const*)
>> 8   0x112d80fcc WebCore::jsIDBObjectStorePrototypeFunctionPut(JSC::ExecState*)
>> 9   0x2d4ae99894b
>> 10  0x102c63562 op_call_slow_return_location_narrow
>> 11  0x102c63562 op_call_slow_return_location_narrow
>> 12  0x102c47063 vmEntryToJavaScript
>> 13  0x1039772ce JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*)
>> 14  0x1039778ff JSC::Interpreter::executeCall(JSC::ExecState*, JSC::JSObject*, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&)
>> 15  0x103c5531c JSC::call(JSC::ExecState*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&)
>> 16  0x103c5540a JSC::call(JSC::ExecState*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&)
>> 17  0x103c556fe JSC::profiledCall(JSC::ExecState*, JSC::ProfilingReason, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&)
>> 18  0x113e9283b WebCore::JSExecState::profiledCall(JSC::ExecState*, JSC::ProfilingReason, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&)
>> 19  0x113ee082a WebCore::JSEventListener::handleEvent(WebCore::ScriptExecutionContext&, WebCore::Event&)
>> 20  0x1145332b3 WebCore::EventTarget::innerInvokeEventListeners(WebCore::Event&, WTF::Vector<WTF::RefPtr<WebCore::RegisteredEventListener, WTF::DumbPtrTraits<WebCore::RegisteredEventListener> >, 1ul, WTF::CrashOnOverflow, 16ul>, WebCore::EventTarget::EventInvokePhase)
>> 21  0x11452eac2 WebCore::EventTarget::fireEventListeners(WebCore::Event&, WebCore::EventTarget::EventInvokePhase)
>> 22  0x114508c67 WebCore::EventContext::handleLocalEvents(WebCore::Event&, WebCore::EventTarget::EventInvokePhase) const
>> 23  0x114509781 WebCore::dispatchEventInDOM(WebCore::Event&, WebCore::EventPath const&)
>> 24  0x114509ba2 void WebCore::dispatchEventWithType<WebCore::EventTarget>(WTF::Vector<WebCore::EventTarget*, 0ul, WTF::CrashOnOverflow, 16ul> const&, WebCore::Event&)
>> 25  0x114509a3d WebCore::EventDispatcher::dispatchEvent(WTF::Vector<WebCore::EventTarget*, 0ul, WTF::CrashOnOverflow, 16ul> const&, WebCore::Event&)
>> 26  0x1138a1f2a WebCore::IDBRequest::dispatchEvent(WebCore::Event&)
>> 27  0x1138a1a4e WebCore::IDBOpenDBRequest::dispatchEvent(WebCore::Event&)
>> 28  0x1160464a0 WebCore::WorkerEventQueue::EventDispatcher::dispatch()
>> 29  0x116046414 WebCore::WorkerEventQueue::enqueueEvent(WTF::Ref<WebCore::Event, WTF::DumbPtrTraits<WebCore::Event> >&&)::$_1::operator()(WebCore::ScriptExecutionContext&) const
>> 30  0x116046204 WTF::Detail::CallableWrapper<WebCore::WorkerEventQueue::enqueueEvent(WTF::Ref<WebCore::Event, WTF::DumbPtrTraits<WebCore::Event> >&&)::$_1, void, WebCore::ScriptExecutionContext&>::call(WebCore::ScriptExecutionContext&)
>> 31  0x113d1ae00 WTF::Function<void (WebCore::ScriptExecutionContext&)>::operator()(WebCore::ScriptExecutionContext&) const
> 
> Not sure how. Will talk to Brady or Sihui about this.

Without my change, indexeddb crashes at the same location with my test. Bug 202648.

[Jiewen Tan]

Created attachment 380347 [details]
Patch

[Chris Dumez]

(In reply to Jiewen Tan from comment #11)
> Comment on attachment 380305 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=380305&action=review
> 
> >> Source/WebCore/workers/WorkerGlobalScope.cpp:418
> >> +    m_thread.workerLoaderProxy().postTaskToLoader([resultContainer = resultContainer.copyRef(), key, wrappedKeyContainer = wrappedKeyContainer.copyRef(), doneContainer = doneContainer.copyRef(), workerMessagingProxy = Ref<WorkerMessagingProxy>(downcast<WorkerMessagingProxy>(m_thread.workerLoaderProxy()))](ScriptExecutionContext& context) {
> > 
> > Why not use makeRef() instead of Ref<WorkerMessagingProxy>() ?
> 
> Fixed.
> 
> >> Source/WebCore/workers/WorkerGlobalScope.cpp:421
> >> +        static_cast<WorkerLoaderProxy&>(workerMessagingProxy.get()).postTaskForModeToWorkerGlobalScope([](ScriptExecutionContext& context) {
> > 
> > Why is the static_cast() needed?
> 
> postTaskForModeToWorkerGlobalScope is not exposed in WorkerMessagingProxy,
> and therefore I need to upcast the object to WorkerLoaderProxy to use it.

WorkerMessagingProxy is a subclass of WorkerLoaderProxy so there should be no reason to cast. If the issue is that an override was marked as private in the subclass, then just make it public.
Or maybe the subclass has a good reason to make this private, in which case you'd have a bug.

[Jiewen Tan]

(In reply to Chris Dumez from comment #14)
> (In reply to Jiewen Tan from comment #11)
> > Comment on attachment 380305 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=380305&action=review
> > 
> > >> Source/WebCore/workers/WorkerGlobalScope.cpp:418
> > >> +    m_thread.workerLoaderProxy().postTaskToLoader([resultContainer = resultContainer.copyRef(), key, wrappedKeyContainer = wrappedKeyContainer.copyRef(), doneContainer = doneContainer.copyRef(), workerMessagingProxy = Ref<WorkerMessagingProxy>(downcast<WorkerMessagingProxy>(m_thread.workerLoaderProxy()))](ScriptExecutionContext& context) {
> > > 
> > > Why not use makeRef() instead of Ref<WorkerMessagingProxy>() ?
> > 
> > Fixed.
> > 
> > >> Source/WebCore/workers/WorkerGlobalScope.cpp:421
> > >> +        static_cast<WorkerLoaderProxy&>(workerMessagingProxy.get()).postTaskForModeToWorkerGlobalScope([](ScriptExecutionContext& context) {
> > > 
> > > Why is the static_cast() needed?
> > 
> > postTaskForModeToWorkerGlobalScope is not exposed in WorkerMessagingProxy,
> > and therefore I need to upcast the object to WorkerLoaderProxy to use it.
> 
> WorkerMessagingProxy is a subclass of WorkerLoaderProxy so there should be
> no reason to cast. If the issue is that an override was marked as private in
> the subclass, then just make it public.
> Or maybe the subclass has a good reason to make this private, in which case
> you'd have a bug.

Made public.

[Chris Dumez]

Still not concerned about those?
Tests that failed text/pixel/audio diff (2): flag all

 test 	results		actual failure	expected failure	history
+imported/w3c/web-platform-tests/2dcontext/imagebitmap/createImageBitmap-serializable.html	expected actual diff pretty diff		text	pass	history
+imported/w3c/web-platform-tests/workers/semantics/structured-clone/dedicated.html	expected actual diff pretty diff		text	pass	history


You did touch the structured cloning implementation which I why I believed they could be related.

[Jiewen Tan]

Comment on attachment 380347 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=380347&action=review

> Source/WebCore/bindings/js/SerializedScriptValue.cpp:-3093
> -        case ImageBitmapTag:

Oops... I'm bad at merging stuffs...

[Jiewen Tan]

Created attachment 380360 [details]
Patch

[Chris Dumez]

Comment on attachment 380360 [details]
Patch

r=me assuming the bots are happy this time.

[Jiewen Tan]

(In reply to Chris Dumez from comment #19)
> Comment on attachment 380360 [details]
> Patch
> 
> r=me assuming the bots are happy this time.

Thanks, Chris. I will only cq+ it once the bots are all green.

[WebKit Commit Bot]

Comment on attachment 380360 [details]
Patch

Clearing flags on attachment: 380360

Committed r250811: <https://trac.webkit.org/changeset/250811>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Truitt Savell]

The new test crypto/workers/subtle/aes-indexeddb.html

added in https://trac.webkit.org/changeset/250811/webkit

is timing out and crashing depending on platform.this is a constant crash on Mojave debug wk2

History:
https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=crypto%2Fworkers%2Fsubtle%2Faes-indexeddb.html

log:
ASSERTION FAILED: isActive()
./Modules/indexeddb/IDBTransaction.cpp(1254) : Ref<WebCore::IDBRequest> WebCore::IDBTransaction::requestPutOrAdd(JSC::ExecState &, WebCore::IDBObjectStore &, RefPtr<WebCore::IDBKey> &&, WebCore::SerializedScriptValue &, IndexedDB::ObjectStoreOverwriteMode)
1   0x552cf0239 WTFCrash

[Jiewen Tan]

(In reply to Truitt Savell from comment #23)
> The new test crypto/workers/subtle/aes-indexeddb.html
> 
> added in https://trac.webkit.org/changeset/250811/webkit
> 
> is timing out and crashing depending on platform.this is a constant crash on
> Mojave debug wk2
> 
> History:
> https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.
> html#showAllRuns=true&tests=crypto%2Fworkers%2Fsubtle%2Faes-indexeddb.html
> 
> log:
> ASSERTION FAILED: isActive()
> ./Modules/indexeddb/IDBTransaction.cpp(1254) : Ref<WebCore::IDBRequest>
> WebCore::IDBTransaction::requestPutOrAdd(JSC::ExecState &,
> WebCore::IDBObjectStore &, RefPtr<WebCore::IDBKey> &&,
> WebCore::SerializedScriptValue &, IndexedDB::ObjectStoreOverwriteMode)
> 1   0x552cf0239 WTFCrash

Could you add some expectations? Here is the bug I have created. It is nothing to do with my code change here. IDB is flaky somehow.

[Truitt Savell]

(In reply to Jiewen Tan from comment #24)
> Could you add some expectations? Here is the bug I have created. It is
> nothing to do with my code change here. IDB is flaky somehow.

Giving expectations like this to your new test would be the same as not having a test at all for your commit, which is against webkit policy. Is there a better way of resolving this?

[Truitt Savell]

I do understand there are other tests in your commit but my concern is still there.

[Jiewen Tan]

(In reply to Truitt Savell from comment #26)
> I do understand there are other tests in your commit but my concern is still
> there.

Here is the test gardening:
Committed r250844: <https://trac.webkit.org/changeset/250844>

We still have release macOS coverage.

[Truitt Savell]

Marked this test as a Timeout on Mojave WK2 Release: https://trac.webkit.org/changeset/250931/webkit