Modified: trunk/Source/WebCore/ChangeLog (289521 => 289522)
--- trunk/Source/WebCore/ChangeLog 2022-02-10 08:10:04 UTC (rev 289521)
+++ trunk/Source/WebCore/ChangeLog 2022-02-10 08:17:01 UTC (rev 289522)
@@ -1,3 +1,33 @@
+2022-02-10 Sihui Liu <[email protected]>
+
+ IDBRequest should emit write barrier on JSValueInWrappedObject modification
+ https://bugs.webkit.org/show_bug.cgi?id=236278
+
+ Reviewed by Geoffrey Garen.
+
+ In IDBRequest, m_resultWrapper is used to cache JSValue for m_result. When request is performing operation (e.g.
+ iterating cursor), we clear both m_result and m_resultWrapper. When request is done, we set m_result and
+ m_resultWrapper. The spec requires the IDBRequest.result() to return the same _javascript_ object after operation,
+ so we introduced m_cursorWrapper to temporarily store the JSValue during operation. When operation starts, we
+ set m_cursorWrapper with m_resultWrapper and when opreation finishes successfully, we set m_resultWrapper back
+ using m_cursorWrapper.
+
+ This has a semantic error as the assignment process does not emit write barrier (webkit.org/b/236277). To fix
+ it, we may just remove m_cursorWrapper and not clear m_resultWrapper during operation. Since we clear m_result
+ in operation, we can check if m_result is empty to decide whether we should return m_resultWrapper for
+ IDBRequest.result().
+
+ * Modules/indexeddb/IDBRequest.cpp:
+ (WebCore::IDBRequest::willIterateCursor):
+ (WebCore::IDBRequest::didOpenOrIterateCursor):
+ (WebCore::IDBRequest::clearWrappers):
+ * Modules/indexeddb/IDBRequest.h:
+ (WebCore::IDBRequest::resultWrapper):
+ (WebCore::IDBRequest::cursorWrapper): Deleted.
+ * bindings/js/JSIDBRequestCustom.cpp:
+ (WebCore::JSIDBRequest::result const):
+ (WebCore::JSIDBRequest::visitAdditionalChildren):
+
2022-02-10 Manuel Rego Casasnovas <[email protected]>
:focus-visible with click on radio/checkbox labels is broken
Modified: trunk/Source/WebCore/Modules/indexeddb/IDBRequest.cpp (289521 => 289522)
--- trunk/Source/WebCore/Modules/indexeddb/IDBRequest.cpp 2022-02-10 08:10:04 UTC (rev 289521)
+++ trunk/Source/WebCore/Modules/indexeddb/IDBRequest.cpp 2022-02-10 08:17:01 UTC (rev 289522)
@@ -470,17 +470,6 @@
if (!context)
return;
- VM& vm = context->vm();
- JSLockHolder lock(vm);
-
- // FIXME: This code is wrong: let's consider that these fields' access are reordered in the concurrent GC thread.
- // And we just scanned cleared m_resultWrapper and then, we missed scanning m_cursorWrapper with a new value.
- // Then we could make both collected. Whenever changing JSValueInWrappedObject fields, we should emit a write barrier
- // if we would like to keep them alive.
- // https://bugs.webkit.org/show_bug.cgi?id=236278
- if (m_resultWrapper)
- m_cursorWrapper.setWithoutBarrier(m_resultWrapper);
- m_resultWrapper.clear();
m_readyState = ReadyState::Pending;
m_domError = nullptr;
m_idbError = IDBError { };
@@ -499,20 +488,15 @@
JSLockHolder lock(vm);
m_result = NullResultType::Empty;
- m_resultWrapper.clear();
-
- // FIXME: This code is wrong: let's consider that these fields' access are reordered in the concurrent GC thread.
- // And we just scanned cleared m_resultWrapper and then, we missed scanning m_cursorWrapper with a new value.
- // Then we could make both collected. Whenever changing JSValueInWrappedObject fields, we should emit a write barrier
- // if we would like to keep them alive.
- // https://bugs.webkit.org/show_bug.cgi?id=236278
if (resultData.type() == IDBResultType::IterateCursorSuccess || resultData.type() == IDBResultType::OpenCursorSuccess) {
- if (m_pendingCursor->setGetResult(*this, resultData.getResult(), m_currentTransactionOperationID) && m_cursorWrapper)
- m_resultWrapper.setWithoutBarrier(m_cursorWrapper);
+ m_pendingCursor->setGetResult(*this, resultData.getResult(), m_currentTransactionOperationID);
if (resultData.getResult().isDefined())
m_result = m_pendingCursor;
}
+ if (std::get_if<NullResultType>(&m_result))
+ m_resultWrapper.clear();
+
m_pendingCursor = nullptr;
completeRequestAndDispatchEvent(resultData);
@@ -573,7 +557,6 @@
JSLockHolder lock(vm);
m_resultWrapper.clear();
- m_cursorWrapper.clear();
WTF::switchOn(m_result,
[] (RefPtr<IDBCursor>& cursor) { cursor->clearWrappers(); },
Modified: trunk/Source/WebCore/Modules/indexeddb/IDBRequest.h (289521 => 289522)
--- trunk/Source/WebCore/Modules/indexeddb/IDBRequest.h 2022-02-10 08:10:04 UTC (rev 289521)
+++ trunk/Source/WebCore/Modules/indexeddb/IDBRequest.h 2022-02-10 08:17:01 UTC (rev 289522)
@@ -81,7 +81,6 @@
using Result = std::variant<RefPtr<IDBCursor>, RefPtr<IDBDatabase>, IDBKeyData, Vector<IDBKeyData>, IDBGetResult, IDBGetAllResult, uint64_t, NullResultType>;
ExceptionOr<Result> result() const;
JSValueInWrappedObject& resultWrapper() { return m_resultWrapper; }
- JSValueInWrappedObject& cursorWrapper() { return m_cursorWrapper; }
using Source = std::variant<RefPtr<IDBObjectStore>, RefPtr<IDBIndex>, RefPtr<IDBCursor>>;
const std::optional<Source>& source() const { return m_source; }
@@ -181,7 +180,6 @@
IDBResourceIdentifier m_resourceIdentifier;
JSValueInWrappedObject m_resultWrapper;
- JSValueInWrappedObject m_cursorWrapper;
uint64_t m_currentTransactionOperationID { 0 };
Modified: trunk/Source/WebCore/bindings/js/JSIDBRequestCustom.cpp (289521 => 289522)
--- trunk/Source/WebCore/bindings/js/JSIDBRequestCustom.cpp 2022-02-10 08:10:04 UTC (rev 289521)
+++ trunk/Source/WebCore/bindings/js/JSIDBRequestCustom.cpp 2022-02-10 08:17:01 UTC (rev 289522)
@@ -38,29 +38,46 @@
JSC::JSValue JSIDBRequest::result(JSC::JSGlobalObject& lexicalGlobalObject) const
{
- return cachedPropertyValue(lexicalGlobalObject, *this, wrapped().resultWrapper(), [&] {
- auto result = wrapped().result();
- if (UNLIKELY(result.hasException())) {
- auto throwScope = DECLARE_THROW_SCOPE(lexicalGlobalObject.vm());
- propagateException(lexicalGlobalObject, throwScope, result.releaseException());
- return jsNull();
- }
+ auto result = wrapped().result();
+ if (UNLIKELY(result.hasException())) {
+ auto throwScope = DECLARE_THROW_SCOPE(lexicalGlobalObject.vm());
+ propagateException(lexicalGlobalObject, throwScope, result.releaseException());
+ return jsNull();
+ }
- IDBRequest::Result resultValue = result.releaseReturnValue();
- return WTF::switchOn(resultValue, [&lexicalGlobalObject] (RefPtr<IDBCursor>& cursor) {
+ auto resultValue = result.releaseReturnValue();
+ auto& resultWrapper = wrapped().resultWrapper();
+ return WTF::switchOn(resultValue, [] (IDBRequest::NullResultType result) {
+ if (result == IDBRequest::NullResultType::Empty)
+ return JSC::jsNull();
+ return JSC::jsUndefined();
+ }, [] (uint64_t number) {
+ return toJS<IDLUnsignedLongLong>(number);
+ }, [&] (RefPtr<IDBCursor>& cursor) {
+ return cachedPropertyValue(lexicalGlobalObject, *this, resultWrapper, [&] {
auto throwScope = DECLARE_THROW_SCOPE(lexicalGlobalObject.vm());
return toJS<IDLInterface<IDBCursor>>(lexicalGlobalObject, *jsCast<JSDOMGlobalObject*>(&lexicalGlobalObject), throwScope, cursor.get());
- }, [&lexicalGlobalObject] (RefPtr<IDBDatabase>& database) {
+ });
+ }, [&] (RefPtr<IDBDatabase>& database) {
+ return cachedPropertyValue(lexicalGlobalObject, *this, resultWrapper, [&] {
auto throwScope = DECLARE_THROW_SCOPE(lexicalGlobalObject.vm());
return toJS<IDLInterface<IDBDatabase>>(lexicalGlobalObject, *jsCast<JSDOMGlobalObject*>(&lexicalGlobalObject), throwScope, database.get());
- }, [&lexicalGlobalObject] (IDBKeyData keyData) {
+ });
+ }, [&] (IDBKeyData keyData) {
+ return cachedPropertyValue(lexicalGlobalObject, *this, resultWrapper, [&] {
return toJS<IDLIDBKeyData>(lexicalGlobalObject, *jsCast<JSDOMGlobalObject*>(&lexicalGlobalObject), keyData);
- }, [&lexicalGlobalObject] (Vector<IDBKeyData> keyDatas) {
+ });
+ }, [&] (Vector<IDBKeyData> keyDatas) {
+ return cachedPropertyValue(lexicalGlobalObject, *this, resultWrapper, [&] {
return toJS<IDLSequence<IDLIDBKeyData>>(lexicalGlobalObject, *jsCast<JSDOMGlobalObject*>(&lexicalGlobalObject), keyDatas);
- }, [&lexicalGlobalObject] (IDBGetResult getResult) {
+ });
+ }, [&] (IDBGetResult getResult) {
+ return cachedPropertyValue(lexicalGlobalObject, *this, resultWrapper, [&] {
auto result = deserializeIDBValueWithKeyInjection(lexicalGlobalObject, getResult.value(), getResult.keyData(), getResult.keyPath());
return result ? result.value() : jsNull();
- }, [&lexicalGlobalObject] (IDBGetAllResult getAllResult) {
+ });
+ }, [&] (IDBGetAllResult getAllResult) {
+ return cachedPropertyValue(lexicalGlobalObject, *this, resultWrapper, [&] {
auto& keys = getAllResult.keys();
auto& values = getAllResult.values();
auto& keyPath = getAllResult.keyPath();
@@ -77,12 +94,6 @@
}
}
return JSValue(JSC::constructArray(&lexicalGlobalObject, static_cast<JSC::ArrayAllocationProfile*>(nullptr), list));
- }, [] (uint64_t number) {
- return toJS<IDLUnsignedLongLong>(number);
- }, [] (IDBRequest::NullResultType other) {
- if (other == IDBRequest::NullResultType::Empty)
- return JSC::jsNull();
- return JSC::jsUndefined();
});
});
}
@@ -92,7 +103,6 @@
{
auto& request = wrapped();
request.resultWrapper().visit(visitor);
- request.cursorWrapper().visit(visitor);
}
DEFINE_VISIT_ADDITIONAL_CHILDREN(JSIDBRequest);
