Attachment 348712 Details for Bug 189201 – Patch

[patch] Patch

bug-189201-20180901080304.patch (text/plain), 26.14 KB, created by youenn fablet on 2018-09-01 08:03:05 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: youenn fablet

Created: 2018-09-01 08:03:05 PDT

Size: 26.14 KB

patch

obsolete

>Subversion Revision: 235582
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 7de7e235a48bda373f93e195782d7c7829e12dbb..a28741aa4f5ef91b404884f616c4c3fb84a9cacf 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,43 @@
>+2018-08-31  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 whenthe DOM object is being visited. 
>+        This requires to mark JSIDBRequest as JSCustomMarkFunction.
>+
>+        Test: http/tests/IndexedDB/collect-IDB-cursor.https.html
>+
>+        * Modules/indexeddb/IDBCursor.cpp:
>+        (WebCore::IDBCursor::update):
>+        (WebCore::IDBCursor::deleteFunction):
>+        (WebCore::IDBCursor::setGetResult):
>+        * Modules/indexeddb/IDBCursor.h:
>+        (WebCore::IDBCursor::currentKey):
>+        (WebCore::IDBCursor::currentPrimaryKey):
>+        (WebCore::IDBCursor::currentValue):
>+        (WebCore::IDBCursor::key const):
>+        (WebCore::IDBCursor::primaryKey const):
>+        (WebCore::IDBCursor::value const):
>+        * Modules/indexeddb/IDBRequest.cpp:
>+        (WebCore::IDBRequest::result const):
>+        (WebCore::IDBRequest::setResult):
>+        (WebCore::IDBRequest::setResultToStructuredClone):
>+        (WebCore::IDBRequest::setResultToUndefined):
>+        (WebCore::IDBRequest::resultValue):
>+        * Modules/indexeddb/IDBRequest.h:
>+        * Modules/indexeddb/IDBRequest.idl:
>+        * Sources.txt:
>+        * WebCore.xcodeproj/project.pbxproj:
>+        * bindings/js/JSIDBCursorCustom.cpp:
>+        (WebCore::JSIDBCursor::visitAdditionalChildren):
>+        * bindings/js/JSIDBRequestCustom.cpp: Added.
>+        (WebCore::JSIDBRequest::visitAdditionalChildren):
>+
> 2018-08-31  Youenn Fablet  <youenn@apple.com>
> 
>         Move stats gathering out of LibWebRTCMediaEndpoint
>diff --git a/Source/WebCore/Modules/indexeddb/IDBCursor.cpp b/Source/WebCore/Modules/indexeddb/IDBCursor.cpp
>index 9b59cffce546669e8fce3f4642fe1a1bb740c93a..2d0f1a7ffbe3dda042d9e1e0705f0788a07344ac 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,18 +334,23 @@ 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;
> }
>diff --git a/Source/WebCore/Modules/indexeddb/IDBCursor.h b/Source/WebCore/Modules/indexeddb/IDBCursor.h
>index 9ba27f370361b1df3502b4cc6c572d5d75015bbc..2f92040ad640ccafb8ea066282dcf46cae223871 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,12 @@ public:
> 
>     virtual bool isKeyCursorWithValue() const { return false; }
> 
>+    JSValueInWrappedObject& currentKey() { return m_currentKey; }
>+    JSValueInWrappedObject& currentPrimaryKey() { return m_currentPrimaryKey; }
>+    JSValueInWrappedObject& currentValue() { return m_currentValue; }
>+
>+    void clearJSCStrongValues();
>+
> protected:
>     IDBCursor(IDBObjectStore&, const IDBCursorInfo&);
>     IDBCursor(IDBIndex&, const IDBCursorInfo&);
>@@ -95,11 +102,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 +126,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..13666a932d4121f793c508d4e3415b1a8847856b 100644
>--- a/Source/WebCore/Modules/indexeddb/IDBRequest.idl
>+++ b/Source/WebCore/Modules/indexeddb/IDBRequest.idl
>@@ -31,6 +31,7 @@
>     ActiveDOMObject,
>     Conditional=INDEXED_DATABASE,
>     GenerateIsReachable=Impl,
>+    JSCustomMarkFunction,
>     SkipVTableValidation,
> ] interface IDBRequest : EventTarget {
>     readonly attribute (IDBCursor or IDBDatabase or any)? result;
>diff --git a/Source/WebCore/Sources.txt b/Source/WebCore/Sources.txt
>index 5915736c5168492c500b8ae5c23f90ab48854560..61c773e65c494e7fa9278e0b93c5ec2d0822c753 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 71cec8cb1140f538e914b05badac66dd754ecc5e..c3335358b2f5a80b4ee3ef1a7a437826f02eab63 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 */; };
>@@ -7337,6 +7338,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>"; };
>@@ -19745,6 +19747,7 @@
> 				5141298D1C5FD7E90059E714 /* JSIDBCursorWithValueCustom.cpp */,
> 				5141299A1C6C166D0059E714 /* JSIDBIndexCustom.cpp */,
> 				511EF2CE17F0FDF100E4FA16 /* JSIDBObjectStoreCustom.cpp */,
>+				41D28D16213A59D200F4206F /* JSIDBRequestCustom.cpp */,
> 				51E269321DD3BC43006B6A58 /* JSIDBTransactionCustom.cpp */,
> 				A7D0318D0E93540300E24ACD /* JSImageDataCustom.cpp */,
> 				AD726FE716D9F204003A4E6D /* JSMediaListCustom.h */,
>@@ -29069,6 +29072,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..08415c591f49e5907dd130cf4c486aaf5c5519ce 100644
>--- a/Source/WebCore/bindings/js/JSIDBCursorCustom.cpp
>+++ b/Source/WebCore/bindings/js/JSIDBCursorCustom.cpp
>@@ -40,6 +40,12 @@ void JSIDBCursor::visitAdditionalChildren(SlotVisitor& visitor)
>     auto& cursor = wrapped();
>     if (auto* request = cursor.request())
>         visitor.addOpaqueRoot(request);
>+
>+    wrapped().currentKey().visit(visitor);
>+    wrapped().currentPrimaryKey().visit(visitor);
>+    wrapped().currentValue().visit(visitor);
>+
>+    wrapped().clearJSCStrongValues();
> }
> 
> JSValue toJSNewlyCreated(JSC::ExecState*, JSDOMGlobalObject* globalObject, Ref<IDBCursor>&& cursor)
>diff --git a/Source/WebCore/bindings/js/JSIDBRequestCustom.cpp b/Source/WebCore/bindings/js/JSIDBRequestCustom.cpp
>new file mode 100644
>index 0000000000000000000000000000000000000000..62eca315bcbfac83cfa4012128a122c6245f39da
>--- /dev/null
>+++ b/Source/WebCore/bindings/js/JSIDBRequestCustom.cpp
>@@ -0,0 +1,42 @@
>+/*
>+ * 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"
>+
>+#if ENABLE(INDEXED_DATABASE)
>+
>+namespace WebCore {
>+using namespace JSC;
>+
>+void JSIDBRequest::visitAdditionalChildren(SlotVisitor& visitor)
>+{
>+    if (auto* value = wrapped().resultValue())
>+        value->visit(visitor);
>+}
>+
>+} // namespace WebCore
>+
>+#endif // ENABLE(INDEXED_DATABASE)
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 4517b1182ce3a155b5e8137bb87f0859bcef04c2..576ef558d5d30bcd71dc8df1370273b080f46a70 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,14 @@
>+2018-08-31  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-08-31  Simon Fraser  <simon.fraser@apple.com>
> 
>         Clean up TestExpectations so that ./Tools/Scripts/lint-test-expectations
>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>

Actions: View | Formatted Diff | Diff

Attachments on bug 189201: 348654 | 348669 | 348684 | 348692 | 348707 | 348712 | 348714 | 348826