WebKit Bugzilla
Attachment 348826 Details for
Bug 189201
: IDBRequest and IDBCursor should not hold strong references to JSValue
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch
bug-189201-20180904105241.patch (text/plain), 28.37 KB, created by
youenn fablet
on 2018-09-04 10:52:42 PDT
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
youenn fablet
Created:
2018-09-04 10:52:42 PDT
Size:
28.37 KB
patch
obsolete
>Subversion Revision: 235612 >diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog >index d63bca0e358f679ecc711030661f56a4aae59940..5aff202712ad3108c26b11768df6abbfb4de01cc 100644 >--- a/Source/WebCore/ChangeLog >+++ b/Source/WebCore/ChangeLog >@@ -1,3 +1,51 @@ >+2018-09-04 Youenn Fablet <youenn@apple.com> >+ >+ IDBRequest and IDBCursor should not hold strong references to JSValue >+ https://bugs.webkit.org/show_bug.cgi?id=189201 >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ IDBRequest and IDBCursor previously were keeping strong references to JS objects. >+ This made it possible to create reference cycles that GC cannot break. >+ Instead, replace JS Strong with JSValueInWrappedObject and mark every JS object when the DOM object is being visited. >+ This requires to mark JSIDBRequest as JSCustomMarkFunction. >+ >+ We keep strong references in IDBCursor until a JSIDBCursor is created. >+ At that point, JSDBCursor is responsible to mark its values. >+ We need to make sure JSIDBCursor is not resurrected from an existing IDBCursor. >+ For that purpose, IDBRequest is marking its IDBCursor for result and source. This is implemented by using CachedAttribute. >+ >+ Test: http/tests/IndexedDB/collect-IDB-cursor.https.html >+ >+ * Modules/indexeddb/IDBCursor.cpp: >+ (WebCore::IDBCursor::update): >+ (WebCore::IDBCursor::deleteFunction): >+ (WebCore::IDBCursor::setGetResult): >+ (WebCore::IDBCursor::markValues): >+ * Modules/indexeddb/IDBCursor.h: >+ (WebCore::IDBCursor::key const): >+ (WebCore::IDBCursor::primaryKey const): >+ (WebCore::IDBCursor::value const): >+ (WebCore::IDBCursor::clearJSCStrongValues): >+ * Modules/indexeddb/IDBRequest.cpp: >+ (WebCore::IDBRequest::result const): >+ (WebCore::IDBRequest::setResult): >+ (WebCore::IDBRequest::setResultToStructuredClone): >+ (WebCore::IDBRequest::setResultToUndefined): >+ (WebCore::IDBRequest::sourceCursor): >+ (WebCore::IDBRequest::resultValue): >+ * Modules/indexeddb/IDBRequest.h: >+ * Modules/indexeddb/IDBRequest.idl: >+ * Sources.txt: >+ * WebCore.xcodeproj/project.pbxproj: >+ * bindings/js/JSIDBCursorCustom.cpp: >+ (WebCore::JSIDBCursor::visitAdditionalChildren): >+ (WebCore::toJS): >+ * bindings/js/JSIDBRequestCustom.cpp: Added. >+ (WebCore::JSIDBRequest::visitAdditionalChildren): >+ (WebCore::JSIDBRequest::result const): >+ (WebCore::JSIDBRequest::source const): >+ > 2018-09-04 Zan Dobersek <zdobersek@igalia.com> and Ms2ger <Ms2ger@igalia.com> > > Implement support for passing ImageBitmap to texImage2D/texSubImage2D >diff --git a/Source/WebCore/Modules/indexeddb/IDBCursor.cpp b/Source/WebCore/Modules/indexeddb/IDBCursor.cpp >index 9b59cffce546669e8fce3f4642fe1a1bb740c93a..06ae0dc5c6eb749cd98339d0be3e7c3d12f23fea 100644 >--- a/Source/WebCore/Modules/indexeddb/IDBCursor.cpp >+++ b/Source/WebCore/Modules/indexeddb/IDBCursor.cpp >@@ -128,7 +128,7 @@ ExceptionOr<Ref<IDBRequest>> IDBCursor::update(ExecState& state, JSValue value) > return Exception { DataError, "Failed to execute 'update' on 'IDBCursor': The effective object store of this cursor uses in-line keys and evaluating the key path of the value parameter results in a different value than the cursor's effective key."_s }; > } > >- auto putResult = effectiveObjectStore().putForCursorUpdate(state, value, m_currentPrimaryKey.get()); >+ auto putResult = effectiveObjectStore().putForCursorUpdate(state, value, m_currentPrimaryKey); > if (putResult.hasException()) > return putResult.releaseException(); > >@@ -299,7 +299,7 @@ ExceptionOr<Ref<WebCore::IDBRequest>> IDBCursor::deleteFunction(ExecState& state > if (!isKeyCursorWithValue()) > return Exception { InvalidStateError, "Failed to execute 'delete' on 'IDBCursor': The cursor is a key cursor."_s }; > >- auto result = effectiveObjectStore().deleteFunction(state, m_currentPrimaryKey.get()); >+ auto result = effectiveObjectStore().deleteFunction(state, m_currentPrimaryKey); > if (result.hasException()) > return result.releaseException(); > >@@ -334,22 +334,34 @@ void IDBCursor::setGetResult(IDBRequest& request, const IDBGetResult& getResult) > } > > auto& vm = context->vm(); >+ JSLockHolder lock(vm); > > // FIXME: This conversion should be done lazily, when script needs the JSValues, so that global object > // of the IDBCursor wrapper can be used, rather than the lexicalGlobalObject. >- m_currentKey = { vm, toJS(*exec, *exec->lexicalGlobalObject(), getResult.keyData().maybeCreateIDBKey().get()) }; >+ m_currentKey = toJS(*exec, *exec->lexicalGlobalObject(), getResult.keyData().maybeCreateIDBKey().get()); >+ m_protectedCurrentKey = { vm, m_currentKey }; >+ > m_currentKeyData = getResult.keyData(); >- m_currentPrimaryKey = { vm, toJS(*exec, *exec->lexicalGlobalObject(), getResult.primaryKeyData().maybeCreateIDBKey().get()) }; >+ m_currentPrimaryKey = toJS(*exec, *exec->lexicalGlobalObject(), getResult.primaryKeyData().maybeCreateIDBKey().get()); >+ m_protectedCurrentPrimaryKey = { vm, m_currentPrimaryKey }; > m_currentPrimaryKeyData = getResult.primaryKeyData(); > > if (isKeyCursorWithValue()) >- m_currentValue = { vm, deserializeIDBValueToJSValue(*exec, getResult.value()) }; >+ m_currentValue = deserializeIDBValueToJSValue(*exec, getResult.value()); > else > m_currentValue = { }; >+ m_protectedCurrentValue = { vm, m_currentValue }; > > m_gotValue = true; > } > >+void IDBCursor::markValues(JSC::SlotVisitor& visitor) >+{ >+ m_currentKey.visit(visitor); >+ m_currentPrimaryKey.visit(visitor); >+ m_currentValue.visit(visitor); >+} >+ > } // namespace WebCore > > #endif // ENABLE(INDEXED_DATABASE) >diff --git a/Source/WebCore/Modules/indexeddb/IDBCursor.h b/Source/WebCore/Modules/indexeddb/IDBCursor.h >index 9ba27f370361b1df3502b4cc6c572d5d75015bbc..0c84a42e05f900f284cc11305112c7e55d265cd0 100644 >--- a/Source/WebCore/Modules/indexeddb/IDBCursor.h >+++ b/Source/WebCore/Modules/indexeddb/IDBCursor.h >@@ -30,6 +30,7 @@ > #include "ExceptionOr.h" > #include "IDBCursorDirection.h" > #include "IDBCursorInfo.h" >+#include "JSValueInWrappedObject.h" > #include <JavaScriptCore/Strong.h> > #include <wtf/Variant.h> > #include <wtf/WeakPtr.h> >@@ -74,6 +75,9 @@ public: > > virtual bool isKeyCursorWithValue() const { return false; } > >+ void markValues(JSC::SlotVisitor&); >+ void clearJSCStrongValues(); >+ > protected: > IDBCursor(IDBObjectStore&, const IDBCursorInfo&); > IDBCursor(IDBIndex&, const IDBCursorInfo&); >@@ -95,11 +99,15 @@ private: > IDBKeyData m_currentKeyData; > IDBKeyData m_currentPrimaryKeyData; > >- // FIXME: The following uses of JSC::Strong are incorrect and can lead to storage leaks >- // due to reference cycles; we should use JSValueInWrappedObject instead. >- JSC::Strong<JSC::Unknown> m_currentKey; >- JSC::Strong<JSC::Unknown> m_currentPrimaryKey; >- JSC::Strong<JSC::Unknown> m_currentValue; >+ // We protect these JS values until IDBCursor is exposed to JavaScript. >+ // Once exposed to JavaScript, JSIDBCursor will mark these values. >+ mutable JSC::Strong<JSC::Unknown> m_protectedCurrentKey; >+ mutable JSC::Strong<JSC::Unknown> m_protectedCurrentPrimaryKey; >+ mutable JSC::Strong<JSC::Unknown> m_protectedCurrentValue; >+ >+ JSValueInWrappedObject m_currentKey; >+ JSValueInWrappedObject m_currentPrimaryKey; >+ JSValueInWrappedObject m_currentValue; > }; > > >@@ -115,17 +123,24 @@ inline IDBCursorDirection IDBCursor::direction() const > > inline JSC::JSValue IDBCursor::key() const > { >- return m_currentKey.get(); >+ return m_currentKey; > } > > inline JSC::JSValue IDBCursor::primaryKey() const > { >- return m_currentPrimaryKey.get(); >+ return m_currentPrimaryKey; > } > > inline JSC::JSValue IDBCursor::value() const > { >- return m_currentValue.get(); >+ return m_currentValue; >+} >+ >+inline void IDBCursor::clearJSCStrongValues() >+{ >+ m_protectedCurrentKey.clear(); >+ m_protectedCurrentPrimaryKey.clear(); >+ m_protectedCurrentValue.clear(); > } > > } // namespace WebCore >diff --git a/Source/WebCore/Modules/indexeddb/IDBRequest.cpp b/Source/WebCore/Modules/indexeddb/IDBRequest.cpp >index 6f5ce0e972653f6573ad5db76a4fccd49f22da9b..aae9023fdbcce991f10dc6bd49814bfc4ff24c9e 100644 >--- a/Source/WebCore/Modules/indexeddb/IDBRequest.cpp >+++ b/Source/WebCore/Modules/indexeddb/IDBRequest.cpp >@@ -153,12 +153,26 @@ IDBRequest::~IDBRequest() > } > } > >-ExceptionOr<std::optional<IDBRequest::Result>> IDBRequest::result() const >+ExceptionOr<std::optional<IDBRequest::JSResult>> IDBRequest::result() const > { > if (!isDone()) > return Exception { InvalidStateError, "Failed to read the 'result' property from 'IDBRequest': The request has not finished."_s }; > >- return std::optional<IDBRequest::Result> { m_result }; >+ if (!m_result) >+ return std::optional<IDBRequest::JSResult> { }; >+ >+ // FIXME: Binding generator should be able to take a Result instead of JSResult. >+ auto result = switchOn(m_result.value(), [](const RefPtr<IDBCursor>& cursor) { >+ return JSResult { cursor }; >+ }, [](const RefPtr<IDBDatabase>& database) { >+ return JSResult { database }; >+ }, [&](const JSValueInWrappedObject& object) { >+ VM& vm = scriptExecutionContext()->vm(); >+ JSLockHolder lock(vm); >+ return JSResult { JSC::Strong<JSC::Unknown> { vm, JSValue(object) } }; >+ }); >+ >+ return std::optional<IDBRequest::JSResult> { WTFMove(result) }; > } > > ExceptionOr<DOMException*> IDBRequest::error() const >@@ -362,7 +376,7 @@ void IDBRequest::setResult(const IDBKeyData& keyData) > // of the IDBRequest wrapper can be used, rather than the lexicalGlobalObject. > VM& vm = context->vm(); > JSLockHolder lock(vm); >- m_result = Result { JSC::Strong<JSC::Unknown> { vm, toJS<IDLIDBKeyData>(*state, *jsCast<JSDOMGlobalObject*>(state->lexicalGlobalObject()), keyData) } }; >+ m_result = Result { JSValueInWrappedObject { toJS<IDLIDBKeyData>(*state, *jsCast<JSDOMGlobalObject*>(state->lexicalGlobalObject()), keyData) } }; > } > > void IDBRequest::setResult(const Vector<IDBKeyData>& keyDatas) >@@ -381,7 +395,7 @@ void IDBRequest::setResult(const Vector<IDBKeyData>& keyDatas) > // of the IDBRequest wrapper can be used, rather than the lexicalGlobalObject. > VM& vm = context->vm(); > JSLockHolder lock(vm); >- m_result = Result { JSC::Strong<JSC::Unknown> { vm, toJS<IDLSequence<IDLIDBKeyData>>(*state, *jsCast<JSDOMGlobalObject*>(state->lexicalGlobalObject()), keyDatas) } }; >+ m_result = Result { JSValueInWrappedObject { toJS<IDLSequence<IDLIDBKeyData>>(*state, *jsCast<JSDOMGlobalObject*>(state->lexicalGlobalObject()), keyDatas) } }; > } > > void IDBRequest::setResult(const Vector<IDBValue>& values) >@@ -400,7 +414,7 @@ void IDBRequest::setResult(const Vector<IDBValue>& values) > // of the IDBRequest wrapper can be used, rather than the lexicalGlobalObject. > VM& vm = context->vm(); > JSLockHolder lock(vm); >- m_result = Result { JSC::Strong<JSC::Unknown> { vm, toJS<IDLSequence<IDLIDBValue>>(*state, *jsCast<JSDOMGlobalObject*>(state->lexicalGlobalObject()), values) } }; >+ m_result = Result { JSValueInWrappedObject { toJS<IDLSequence<IDLIDBValue>>(*state, *jsCast<JSDOMGlobalObject*>(state->lexicalGlobalObject()), values) } }; > } > > void IDBRequest::setResult(uint64_t number) >@@ -411,7 +425,7 @@ void IDBRequest::setResult(uint64_t number) > if (!context) > return; > >- m_result = Result { JSC::Strong<JSC::Unknown> { context->vm(), toJS<IDLUnrestrictedDouble>(number) } }; >+ m_result = Result { JSValueInWrappedObject { toJS<IDLUnrestrictedDouble>(number) } }; > } > > void IDBRequest::setResultToStructuredClone(const IDBValue& value) >@@ -432,18 +446,14 @@ void IDBRequest::setResultToStructuredClone(const IDBValue& value) > // of the IDBRequest wrapper can be used, rather than the lexicalGlobalObject. > VM& vm = context->vm(); > JSLockHolder lock(vm); >- m_result = Result { JSC::Strong<JSC::Unknown> { vm, toJS<IDLIDBValue>(*state, *jsCast<JSDOMGlobalObject*>(state->lexicalGlobalObject()), value) } }; >+ m_result = Result { JSValueInWrappedObject { toJS<IDLIDBValue>(*state, *jsCast<JSDOMGlobalObject*>(state->lexicalGlobalObject()), value) } }; > } > > void IDBRequest::setResultToUndefined() > { > ASSERT(&originThread() == &Thread::current()); > >- auto* context = scriptExecutionContext(); >- if (!context) >- return; >- >- m_result = Result { JSC::Strong<JSC::Unknown> { context->vm(), JSC::jsUndefined() } }; >+ m_result = Result { JSValueInWrappedObject { JSC::jsUndefined() } }; > } > > IDBCursor* IDBRequest::resultCursor() >@@ -459,6 +469,18 @@ IDBCursor* IDBRequest::resultCursor() > ); > } > >+JSValueInWrappedObject* IDBRequest::resultValue() >+{ >+ if (!m_result) >+ return nullptr; >+ >+ return switchOn(m_result.value(), [](JSValueInWrappedObject& object) { >+ return &object; >+ }, [](auto&) -> JSValueInWrappedObject* { >+ return nullptr; >+ }); >+} >+ > void IDBRequest::willIterateCursor(IDBCursor& cursor) > { > ASSERT(&originThread() == &Thread::current()); >diff --git a/Source/WebCore/Modules/indexeddb/IDBRequest.h b/Source/WebCore/Modules/indexeddb/IDBRequest.h >index e625cd3c7fbdc8913d6d49aa340ed9bb7239b5e8..c0df443e186973eb41e6324ba9edc94f00d1ff28 100644 >--- a/Source/WebCore/Modules/indexeddb/IDBRequest.h >+++ b/Source/WebCore/Modules/indexeddb/IDBRequest.h >@@ -33,6 +33,7 @@ > #include "IDBError.h" > #include "IDBResourceIdentifier.h" > #include "IndexedDB.h" >+#include "JSValueInWrappedObject.h" > #include <JavaScriptCore/Strong.h> > #include <wtf/Function.h> > #include <wtf/Scope.h> >@@ -69,10 +70,9 @@ public: > > virtual ~IDBRequest(); > >- // FIXME: The following use of JSC::Strong is incorrect and can lead to storage leaks >- // due to reference cycles; we should use JSValueInWrappedObject instead. >- using Result = Variant<RefPtr<IDBCursor>, RefPtr<IDBDatabase>, JSC::Strong<JSC::Unknown>>; >- ExceptionOr<std::optional<Result>> result() const; >+ using Result = Variant<RefPtr<IDBCursor>, RefPtr<IDBDatabase>, JSValueInWrappedObject>; >+ using JSResult = Variant<RefPtr<IDBCursor>, RefPtr<IDBDatabase>, JSC::Strong<JSC::Unknown>>; >+ ExceptionOr<std::optional<JSResult>> result() const; > > using Source = Variant<RefPtr<IDBObjectStore>, RefPtr<IDBIndex>, RefPtr<IDBCursor>>; > const std::optional<Source>& source() const { return m_source; } >@@ -117,6 +117,8 @@ public: > > bool hasPendingActivity() const final; > >+ JSValueInWrappedObject* resultValue(); >+ > protected: > IDBRequest(ScriptExecutionContext&, IDBClient::IDBConnectionProxy&); > >diff --git a/Source/WebCore/Modules/indexeddb/IDBRequest.idl b/Source/WebCore/Modules/indexeddb/IDBRequest.idl >index 7751e205282b8e8a58895155f4b54e0aaabb04ad..54807d88fe309ef210d1cb9ba414b1b57fe2dec2 100644 >--- a/Source/WebCore/Modules/indexeddb/IDBRequest.idl >+++ b/Source/WebCore/Modules/indexeddb/IDBRequest.idl >@@ -31,11 +31,12 @@ > ActiveDOMObject, > Conditional=INDEXED_DATABASE, > GenerateIsReachable=Impl, >+ JSCustomMarkFunction, > SkipVTableValidation, > ] interface IDBRequest : EventTarget { >- readonly attribute (IDBCursor or IDBDatabase or any)? result; >+ [CachedAttribute, CustomGetter] readonly attribute (IDBCursor or IDBDatabase or any)? result; > readonly attribute DOMException? error; >- readonly attribute (IDBObjectStore or IDBIndex or IDBCursor)? source; >+ [CachedAttribute, CustomGetter] readonly attribute (IDBObjectStore or IDBIndex or IDBCursor)? source; > readonly attribute IDBTransaction transaction; > readonly attribute IDBRequestReadyState readyState; > >diff --git a/Source/WebCore/Sources.txt b/Source/WebCore/Sources.txt >index 26fbe219d91d7d8706cc8bb5126eceb0afbbc84b..6da4a107ccf9f0d9881e4af6aea7c273e3d53871 100644 >--- a/Source/WebCore/Sources.txt >+++ b/Source/WebCore/Sources.txt >@@ -415,6 +415,7 @@ bindings/js/JSIDBCursorCustom.cpp > bindings/js/JSIDBCursorWithValueCustom.cpp > bindings/js/JSIDBIndexCustom.cpp > bindings/js/JSIDBObjectStoreCustom.cpp >+bindings/js/JSIDBRequestCustom.cpp > bindings/js/JSIDBTransactionCustom.cpp > bindings/js/JSImageDataCustom.cpp > bindings/js/JSLazyEventListener.cpp >diff --git a/Source/WebCore/WebCore.xcodeproj/project.pbxproj b/Source/WebCore/WebCore.xcodeproj/project.pbxproj >index 5fc769034437b27d7bd54ee630615cfd801d3c3d..b1da17a4b7d3618860c72774184987cb56bff646 100644 >--- a/Source/WebCore/WebCore.xcodeproj/project.pbxproj >+++ b/Source/WebCore/WebCore.xcodeproj/project.pbxproj >@@ -1086,6 +1086,7 @@ > 416E6FE91BBD12E5000A6043 /* ReadableStreamBuiltins.h in Headers */ = {isa = PBXBuildFile; fileRef = 9B03D8061BB3110D00B764D8 /* ReadableStreamBuiltins.h */; settings = {ATTRIBUTES = (Private, ); }; }; > 416E6FE91BBD12E5000A6053 /* WritableStreamBuiltins.h in Headers */ = {isa = PBXBuildFile; fileRef = 9B03D8061BB3110D00B764E8 /* WritableStreamBuiltins.h */; settings = {ATTRIBUTES = (Private, ); }; }; > 417253AB1354BBBC00360F2A /* MediaControlElements.h in Headers */ = {isa = PBXBuildFile; fileRef = 417253A91354BBBC00360F2A /* MediaControlElements.h */; }; >+ 417433152139DE5E007C32E8 /* JSValueInWrappedObject.h in Headers */ = {isa = PBXBuildFile; fileRef = 931AE3B81FB80EAE00F5EFB2 /* JSValueInWrappedObject.h */; settings = {ATTRIBUTES = (Private, ); }; }; > 417612AF1E3A994000C3D81D /* LibWebRTCMediaEndpoint.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 417612AB1E3A993B00C3D81D /* LibWebRTCMediaEndpoint.cpp */; }; > 417612B01E3A994000C3D81D /* LibWebRTCMediaEndpoint.h in Headers */ = {isa = PBXBuildFile; fileRef = 417612AC1E3A993B00C3D81D /* LibWebRTCMediaEndpoint.h */; }; > 417612B11E3A994000C3D81D /* LibWebRTCPeerConnectionBackend.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 417612AD1E3A993B00C3D81D /* LibWebRTCPeerConnectionBackend.cpp */; }; >@@ -7347,6 +7348,7 @@ > 41D129D41F3D0F6600D15E47 /* CacheStorageProvider.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = CacheStorageProvider.h; sourceTree = "<group>"; }; > 41D28D0B2139E01D00F4206F /* LibWebRTCStatsCollector.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = LibWebRTCStatsCollector.cpp; path = libwebrtc/LibWebRTCStatsCollector.cpp; sourceTree = "<group>"; }; > 41D28D0C2139E01E00F4206F /* LibWebRTCStatsCollector.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = LibWebRTCStatsCollector.h; path = libwebrtc/LibWebRTCStatsCollector.h; sourceTree = "<group>"; }; >+ 41D28D16213A59D200F4206F /* JSIDBRequestCustom.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = JSIDBRequestCustom.cpp; sourceTree = "<group>"; }; > 41D51BB21E4E2E8100131A5B /* LibWebRTCAudioFormat.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = LibWebRTCAudioFormat.h; path = libwebrtc/LibWebRTCAudioFormat.h; sourceTree = "<group>"; }; > 41DEFCB21E56C1B9000D9E5F /* JSDOMBindingInternals.js */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.javascript; path = JSDOMBindingInternals.js; sourceTree = "<group>"; }; > 41DEFCB31E56C1B9000D9E5F /* JSDOMMapLike.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = JSDOMMapLike.cpp; sourceTree = "<group>"; }; >@@ -19771,6 +19773,7 @@ > 5141298D1C5FD7E90059E714 /* JSIDBCursorWithValueCustom.cpp */, > 5141299A1C6C166D0059E714 /* JSIDBIndexCustom.cpp */, > 511EF2CE17F0FDF100E4FA16 /* JSIDBObjectStoreCustom.cpp */, >+ 41D28D16213A59D200F4206F /* JSIDBRequestCustom.cpp */, > 51E269321DD3BC43006B6A58 /* JSIDBTransactionCustom.cpp */, > A7D0318D0E93540300E24ACD /* JSImageDataCustom.cpp */, > AD726FE716D9F204003A4E6D /* JSMediaListCustom.h */, >@@ -29118,6 +29121,7 @@ > 7C73FB12191EF6F4007DE061 /* JSUserMessageHandler.h in Headers */, > 7C73FB0D191EF5A8007DE061 /* JSUserMessageHandlersNamespace.h in Headers */, > 15C77093100D3CA8005BA267 /* JSValidityState.h in Headers */, >+ 417433152139DE5E007C32E8 /* JSValueInWrappedObject.h in Headers */, > BE8EF04B171C9014009B48C3 /* JSVideoTrack.h in Headers */, > BE8EF04D171C9014009B48C3 /* JSVideoTrackList.h in Headers */, > 46E791491F97E01A00199739 /* JSVisibilityState.h in Headers */, >diff --git a/Source/WebCore/bindings/js/JSIDBCursorCustom.cpp b/Source/WebCore/bindings/js/JSIDBCursorCustom.cpp >index bafe2fe0bf427af9c14a217945b6b23bd06c9b93..282824f7a68ff400f86e1b9efbd2748e91365b29 100644 >--- a/Source/WebCore/bindings/js/JSIDBCursorCustom.cpp >+++ b/Source/WebCore/bindings/js/JSIDBCursorCustom.cpp >@@ -40,6 +40,8 @@ void JSIDBCursor::visitAdditionalChildren(SlotVisitor& visitor) > auto& cursor = wrapped(); > if (auto* request = cursor.request()) > visitor.addOpaqueRoot(request); >+ >+ wrapped().markValues(visitor); > } > > JSValue toJSNewlyCreated(JSC::ExecState*, JSDOMGlobalObject* globalObject, Ref<IDBCursor>&& cursor) >@@ -51,6 +53,7 @@ JSValue toJSNewlyCreated(JSC::ExecState*, JSDOMGlobalObject* globalObject, Ref<I > > JSValue toJS(JSC::ExecState* state, JSDOMGlobalObject* globalObject, IDBCursor& cursor) > { >+ cursor.clearJSCStrongValues(); > return wrap(state, globalObject, cursor); > } > >diff --git a/Source/WebCore/bindings/js/JSIDBRequestCustom.cpp b/Source/WebCore/bindings/js/JSIDBRequestCustom.cpp >new file mode 100644 >index 0000000000000000000000000000000000000000..3418c3308aba3417d76ee71b66cc979ad7850e32 >--- /dev/null >+++ b/Source/WebCore/bindings/js/JSIDBRequestCustom.cpp >@@ -0,0 +1,65 @@ >+/* >+ * Copyright (C) 2018 Apple Inc. All rights reserved. >+ * >+ * Redistribution and use in source and binary forms, with or without >+ * modification, are permitted provided that the following conditions >+ * are met: >+ * 1. Redistributions of source code must retain the above copyright >+ * notice, this list of conditions and the following disclaimer. >+ * 2. Redistributions in binary form must reproduce the above copyright >+ * notice, this list of conditions and the following disclaimer in the >+ * documentation and/or other materials provided with the distribution. >+ * >+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS'' >+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, >+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR >+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS >+ * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR >+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF >+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS >+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN >+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) >+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF >+ * THE POSSIBILITY OF SUCH DAMAGE. >+ */ >+ >+#include "config.h" >+#include "JSIDBRequest.h" >+ >+#include "JSIDBCursor.h" >+#include "JSIDBDatabase.h" >+#include "JSIDBIndex.h" >+#include "JSIDBObjectStore.h" >+ >+#if ENABLE(INDEXED_DATABASE) >+ >+namespace WebCore { >+using namespace JSC; >+ >+void JSIDBRequest::visitAdditionalChildren(SlotVisitor& visitor) >+{ >+ if (auto* value = wrapped().resultValue()) >+ value->visit(visitor); >+} >+ >+JSValue JSIDBRequest::result(ExecState& state) const >+{ >+ auto throwScope = DECLARE_THROW_SCOPE(state.vm()); >+ JSValue jsResult = toJS<IDLNullable<IDLUnion<IDLInterface<IDBCursor>, IDLInterface<IDBDatabase>, IDLAny>>>(state, *globalObject(), throwScope, wrapped().result()); >+ // We keep m_result so that it is visited through JSIDBRequest. This ensures IDBCursor JS values to be marked. >+ m_result.set(state.vm(), this, jsResult); >+ return jsResult; >+} >+ >+JSValue JSIDBRequest::source(ExecState& state) const >+{ >+ auto throwScope = DECLARE_THROW_SCOPE(state.vm()); >+ JSValue source = toJS<IDLNullable<IDLUnion<IDLInterface<IDBObjectStore>, IDLInterface<IDBIndex>, IDLInterface<IDBCursor>>>>(state, *globalObject(), throwScope, wrapped().source()); >+ // We keep m_source so that it is visited through JSIDBRequest. This ensures IDBCursor JS values to be marked. >+ m_source.set(state.vm(), this, source); >+ return source; >+} >+ >+} // namespace WebCore >+ >+#endif // ENABLE(INDEXED_DATABASE) >diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog >index 0d7fe7235b4458ce6a3e32e6ef4c55ea9295b7c1..795add21141f9f9c7d9da1efa9ced6ca6bf0a2a1 100644 >--- a/LayoutTests/ChangeLog >+++ b/LayoutTests/ChangeLog >@@ -1,3 +1,14 @@ >+2018-09-04 Youenn Fablet <youenn@apple.com> >+ >+ IDBRequest and IDBCursor should not hold strong references to JSValue >+ https://bugs.webkit.org/show_bug.cgi?id=189201 >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ * http/tests/IndexedDB/collect-IDB-objects-reference-cycles.https-expected.txt: Added. >+ * http/tests/IndexedDB/collect-IDB-objects-reference-cycles.https.html: Added. >+ * http/tests/IndexedDB/resources/myidbframe-cursor.htm: Added. >+ > 2018-09-04 Zan Dobersek <zdobersek@igalia.com> and Ms2ger <Ms2ger@igalia.com> > > Implement support for passing ImageBitmap to texImage2D/texSubImage2D >diff --git a/LayoutTests/http/tests/IndexedDB/collect-IDB-objects-reference-cycles.https-expected.txt b/LayoutTests/http/tests/IndexedDB/collect-IDB-objects-reference-cycles.https-expected.txt >new file mode 100644 >index 0000000000000000000000000000000000000000..1097660dff1e8e07c7250245838d6ddbf2a684f8 >--- /dev/null >+++ b/LayoutTests/http/tests/IndexedDB/collect-IDB-objects-reference-cycles.https-expected.txt >@@ -0,0 +1,4 @@ >+ >+ >+PASS Ensure that IDBCursor/IDBRequest do not create reference cycles that GC cannot break >+ >diff --git a/LayoutTests/http/tests/IndexedDB/collect-IDB-objects-reference-cycles.https.html b/LayoutTests/http/tests/IndexedDB/collect-IDB-objects-reference-cycles.https.html >new file mode 100644 >index 0000000000000000000000000000000000000000..d8b633319d66cbea65d58035c242efc12b9b24d8 >--- /dev/null >+++ b/LayoutTests/http/tests/IndexedDB/collect-IDB-objects-reference-cycles.https.html >@@ -0,0 +1,45 @@ >+<!DOCTYPE html> >+<meta charset="utf-8"> >+<script src="/resources/testharness.js"></script> >+<script src="/resources/testharnessreport.js"></script> >+<script> >+function waitFor(duration) >+{ >+ return new Promise((resolve) => setTimeout(resolve, duration)); >+} >+ >+var resolveCallback, rejectCallback; >+var promise = new Promise((resolve, reject) => { >+ resolveCallback = resolve; >+ rejectCallback = reject; >+}); >+ >+async function done() >+{ >+ try { >+ const frameIdentifier = internals.documentIdentifier(iframe.contentDocument); >+ iframe.src = "non-existent-frame"; >+ let counter = 0; >+ while (++counter < 50) { >+ if (!internals.isDocumentAlive(frameIdentifier)) { >+ resolveCallback(); >+ return; >+ } >+ if (window.GCController) >+ GCController.collect(); >+ >+ await waitFor(50); >+ } >+ } finally { >+ rejectCallback("Test failed"); >+ } >+} >+ >+promise_test((test) => { >+ if (!window.internals) >+ rejectCallback("Test require internals API"); >+ return promise; >+}, "Ensure that IDBCursor/IDBRequest do not create reference cycles that GC cannot break"); >+ >+</script> >+<iframe src="resources/myidbframe-cursor.htm" id="iframe"></iframe> >diff --git a/LayoutTests/http/tests/IndexedDB/resources/myidbframe-cursor.htm b/LayoutTests/http/tests/IndexedDB/resources/myidbframe-cursor.htm >new file mode 100644 >index 0000000000000000000000000000000000000000..4f877ed6a827135acfe0f9a590e0ef987e16535e >--- /dev/null >+++ b/LayoutTests/http/tests/IndexedDB/resources/myidbframe-cursor.htm >@@ -0,0 +1,37 @@ >+<!DOCTYPE html> >+<script src="../../resources/testharness.js"></script> >+<script src="support.js"></script> >+ >+<script> >+var db, >+t = async_test(), >+records = [ { pKey: "primaryKey", iKey: "indexKey" }]; >+ >+var open_rq = createdb(t); >+open_rq.onupgradeneeded = function(e) { >+ db = e.target.result; >+ var objStore = db.createObjectStore("test", { keyPath: "pKey" }); >+ var index = objStore.createIndex("index", "iKey"); >+ >+ objStore.add(records[0]); >+ >+ let request1 = objStore.get(records[0].pKey); >+ request1.onsuccess = (e) => { >+ request1.result.request = request1; >+ }; >+ >+ let request2 = index.openCursor(); >+ request2.onsuccess = t.step_func(function(e) { >+ var cursor = e.target.result; >+ cursor.value.cursor = cursor; >+ cursor.primaryKey.cursor = cursor; >+ cursor.key.cursor = cursor; >+ }); >+ >+ e.target.transaction.oncomplete = t.step_func(function(e) { >+ t.done(); >+ parent.done(); >+ }); >+} >+ >+</script>
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=348826">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 189201
:
348654
|
348669
|
348684
|
348692
|
348707
|
348712
|
348714
| 348826