Modern IDB: A few cursor tests are flaky because JS wrappers are GC'ed.
authorbeidson@apple.com <beidson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 13 Jan 2016 18:42:00 +0000 (18:42 +0000)
committerbeidson@apple.com <beidson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 13 Jan 2016 18:42:00 +0000 (18:42 +0000)
https://bugs.webkit.org/show_bug.cgi?id=153038

Reviewed by Alex Christensen.

No new tests (Couldn't write a test that was any more reliable than "flaky", so fixing the existing flaky tests will do).

And IDBCursor has an associated IDBRequest that is re-used each time the IDBCursor iterates.

The normal ActiveDOMObject approach to prevent the IDBRequest's wrapper from being garbage collected was not good enough
because, while the IDBRequest may not currently be waiting on any activity, as long as its associated IDBCursor is still
reachable then the request might be reused in the future.

Fortunately there's an IDL allowance for "one object keeping another alive during GC" and that's JSCustomMarkFunction
combined with GenerateIsReachable.

Applying those to IDBCursor and IDBRequest fix this handily.

* CMakeLists.txt:
* WebCore.xcodeproj/project.pbxproj:

* Modules/indexeddb/IDBCursor.h:
(WebCore::IDBCursor::isModernCursor):
* Modules/indexeddb/IDBCursor.idl:

* Modules/indexeddb/IDBRequest.idl:

* Modules/indexeddb/client/IDBCursorImpl.cpp:
(WebCore::IDBClient::IDBCursor::advance):
(WebCore::IDBClient::IDBCursor::continueFunction):
(WebCore::IDBClient::IDBCursor::uncheckedIterateCursor):
(WebCore::IDBClient::IDBCursor::uncheckedIteratorCursor): Deleted. Fixed the typo of this name.
* Modules/indexeddb/client/IDBCursorImpl.h:

* bindings/js/JSIDBCursorCustom.cpp: Added.
(WebCore::JSIDBCursor::visitAdditionalChildren):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@194967 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/CMakeLists.txt
Source/WebCore/ChangeLog
Source/WebCore/Modules/indexeddb/IDBCursor.h
Source/WebCore/Modules/indexeddb/IDBCursor.idl
Source/WebCore/Modules/indexeddb/IDBRequest.idl
Source/WebCore/Modules/indexeddb/client/IDBCursorImpl.cpp
Source/WebCore/Modules/indexeddb/client/IDBCursorImpl.h
Source/WebCore/WebCore.xcodeproj/project.pbxproj
Source/WebCore/bindings/js/JSIDBCursorCustom.cpp [new file with mode: 0644]

index ca84551..26304f6 100644 (file)
@@ -1169,6 +1169,7 @@ set(WebCore_SOURCES
     bindings/js/JSHTMLTemplateElementCustom.cpp
     bindings/js/JSHistoryCustom.cpp
     bindings/js/JSIDBAnyCustom.cpp
+    bindings/js/JSIDBCursorCustom.cpp
     bindings/js/JSIDBDatabaseCustom.cpp
     bindings/js/JSIDBObjectStoreCustom.cpp
     bindings/js/JSImageConstructor.cpp
index e479c27..83e5335 100644 (file)
@@ -1,3 +1,42 @@
+2016-01-13  Brady Eidson  <beidson@apple.com>
+
+        Modern IDB: A few cursor tests are flaky because JS wrappers are GC'ed.
+        https://bugs.webkit.org/show_bug.cgi?id=153038
+
+        Reviewed by Alex Christensen.
+
+        No new tests (Couldn't write a test that was any more reliable than "flaky", so fixing the existing flaky tests will do).
+
+        And IDBCursor has an associated IDBRequest that is re-used each time the IDBCursor iterates.
+        
+        The normal ActiveDOMObject approach to prevent the IDBRequest's wrapper from being garbage collected was not good enough
+        because, while the IDBRequest may not currently be waiting on any activity, as long as its associated IDBCursor is still
+        reachable then the request might be reused in the future.
+        
+        Fortunately there's an IDL allowance for "one object keeping another alive during GC" and that's JSCustomMarkFunction
+        combined with GenerateIsReachable.
+        
+        Applying those to IDBCursor and IDBRequest fix this handily.
+        
+        * CMakeLists.txt:
+        * WebCore.xcodeproj/project.pbxproj:
+
+        * Modules/indexeddb/IDBCursor.h:
+        (WebCore::IDBCursor::isModernCursor):
+        * Modules/indexeddb/IDBCursor.idl:
+        
+        * Modules/indexeddb/IDBRequest.idl:
+        
+        * Modules/indexeddb/client/IDBCursorImpl.cpp:
+        (WebCore::IDBClient::IDBCursor::advance):
+        (WebCore::IDBClient::IDBCursor::continueFunction):
+        (WebCore::IDBClient::IDBCursor::uncheckedIterateCursor):
+        (WebCore::IDBClient::IDBCursor::uncheckedIteratorCursor): Deleted. Fixed the typo of this name.
+        * Modules/indexeddb/client/IDBCursorImpl.h:
+        
+        * bindings/js/JSIDBCursorCustom.cpp: Added.
+        (WebCore::JSIDBCursor::visitAdditionalChildren):
+
 2016-01-13  Zalan Bujtas  <zalan@apple.com>
 
         Get text drawing working with display lists.
index bb24ea3..78d60e9 100644 (file)
@@ -74,6 +74,8 @@ public:
 
     virtual bool isKeyCursor() const = 0;
 
+    virtual bool isModernCursor() const { return false; }
+
 protected:
     IDBCursor();
 };
index 9edd017..95180e8 100644 (file)
@@ -27,6 +27,7 @@
     Conditional=INDEXED_DATABASE,
     EnabledAtRuntime=IndexedDB,
     SkipVTableValidation,
+    JSCustomMarkFunction,
 ] interface IDBCursor {
     readonly attribute IDBAny source;
     readonly attribute DOMString direction;
index fea8ebe..de7cc5a 100644 (file)
@@ -35,6 +35,7 @@
     JSGenerateToJSObject,
     JSGenerateToNativeObject,
     SkipVTableValidation,
+    GenerateIsReachable=Impl,
 ] interface IDBRequest : EventTarget {
     [GetterRaisesExceptionWithMessage] readonly attribute IDBAny result;
     [GetterRaisesExceptionWithMessage] readonly attribute DOMError error;
index e8e216e..0263f92 100644 (file)
@@ -201,7 +201,7 @@ void IDBCursor::advance(unsigned long count, ExceptionCodeWithMessage& ec)
 
     m_gotValue = false;
 
-    uncheckedIteratorCursor(IDBKeyData(), count);
+    uncheckedIterateCursor(IDBKeyData(), count);
 }
 
 void IDBCursor::continueFunction(ScriptExecutionContext* context, ExceptionCodeWithMessage& ec)
@@ -276,10 +276,10 @@ void IDBCursor::continueFunction(const IDBKeyData& key, ExceptionCodeWithMessage
 
     m_gotValue = false;
 
-    uncheckedIteratorCursor(key, 0);
+    uncheckedIterateCursor(key, 0);
 }
 
-void IDBCursor::uncheckedIteratorCursor(const IDBKeyData& key, unsigned long count)
+void IDBCursor::uncheckedIterateCursor(const IDBKeyData& key, unsigned long count)
 {
     m_request->willIterateCursor(*this);
     transaction().iterateCursor(*this, key, count);
index 65c459f..9c72e25 100644 (file)
@@ -72,6 +72,7 @@ public:
     void setGetResult(IDBRequest&, const IDBGetResult&);
 
     virtual bool isKeyCursor() const override { return true; }
+    virtual bool isModernCursor() const override final { return true; }
 
 protected:
     IDBCursor(IDBTransaction&, IDBObjectStore&, const IDBCursorInfo&);
@@ -88,7 +89,7 @@ private:
     IDBObjectStore& effectiveObjectStore() const;
     IDBTransaction& transaction() const;
 
-    void uncheckedIteratorCursor(const IDBKeyData&, unsigned long count);
+    void uncheckedIterateCursor(const IDBKeyData&, unsigned long count);
 
     bool m_gotValue { false };
 
index 66c43e5..7bdf2ca 100644 (file)
                5126E6BC0A2E3B12005C29FA /* IconDatabase.h in Headers */ = {isa = PBXBuildFile; fileRef = 5126E6BA0A2E3B12005C29FA /* IconDatabase.h */; settings = {ATTRIBUTES = (Private, ); }; };
                512BDB4A1C456FF5006494DF /* SQLiteIDBBackingStore.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 512BDB481C456FAB006494DF /* SQLiteIDBBackingStore.cpp */; };
                512BDB4B1C456FFA006494DF /* SQLiteIDBBackingStore.h in Headers */ = {isa = PBXBuildFile; fileRef = 512BDB491C456FAB006494DF /* SQLiteIDBBackingStore.h */; };
+               512BDB4D1C46B153006494DF /* JSIDBCursorCustom.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 512BDB4C1C46B0FF006494DF /* JSIDBCursorCustom.cpp */; };
                512DD8E30D91E2B4000F89EE /* SharedBufferCF.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 512DD8E20D91E2B4000F89EE /* SharedBufferCF.cpp */; };
                512DD8F40D91E6AF000F89EE /* LegacyWebArchive.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 512DD8EA0D91E6AF000F89EE /* LegacyWebArchive.cpp */; };
                512DD8F50D91E6AF000F89EE /* LegacyWebArchive.h in Headers */ = {isa = PBXBuildFile; fileRef = 512DD8EB0D91E6AF000F89EE /* LegacyWebArchive.h */; settings = {ATTRIBUTES = (Private, ); }; };
                5126E6BA0A2E3B12005C29FA /* IconDatabase.h */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.c.h; path = IconDatabase.h; sourceTree = "<group>"; };
                512BDB481C456FAB006494DF /* SQLiteIDBBackingStore.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = SQLiteIDBBackingStore.cpp; sourceTree = "<group>"; };
                512BDB491C456FAB006494DF /* SQLiteIDBBackingStore.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = SQLiteIDBBackingStore.h; sourceTree = "<group>"; };
+               512BDB4C1C46B0FF006494DF /* JSIDBCursorCustom.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = JSIDBCursorCustom.cpp; sourceTree = "<group>"; };
                512DD8E20D91E2B4000F89EE /* SharedBufferCF.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = SharedBufferCF.cpp; sourceTree = "<group>"; };
                512DD8EA0D91E6AF000F89EE /* LegacyWebArchive.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = LegacyWebArchive.cpp; sourceTree = "<group>"; };
                512DD8EB0D91E6AF000F89EE /* LegacyWebArchive.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = LegacyWebArchive.h; sourceTree = "<group>"; };
                                AB4CB4EA0B8BDA3D009F40B0 /* JSHTMLSelectElementCustom.h */,
                                D6F7960C166FFECE0076DD18 /* JSHTMLTemplateElementCustom.cpp */,
                                511EF2CC17F0FDF100E4FA16 /* JSIDBAnyCustom.cpp */,
+                               512BDB4C1C46B0FF006494DF /* JSIDBCursorCustom.cpp */,
                                511EF2CD17F0FDF100E4FA16 /* JSIDBDatabaseCustom.cpp */,
                                511EF2CE17F0FDF100E4FA16 /* JSIDBObjectStoreCustom.cpp */,
                                A7D0318D0E93540300E24ACD /* JSImageDataCustom.cpp */,
                                CD5E5B611A15F156000C609E /* PageConfiguration.cpp in Sources */,
                                F3820892147D35F90010BC06 /* PageConsoleAgent.cpp in Sources */,
                                DAED203016F2442B0070EC0F /* PageConsoleClient.cpp in Sources */,
+                               512BDB4D1C46B153006494DF /* JSIDBCursorCustom.cpp in Sources */,
                                A5A2AF0B1829734300DE1729 /* PageDebuggable.cpp in Sources */,
                                F34742DC134362F000531BC2 /* PageDebuggerAgent.cpp in Sources */,
                                9302B0BD0D79F82900C7EE83 /* PageGroup.cpp in Sources */,
diff --git a/Source/WebCore/bindings/js/JSIDBCursorCustom.cpp b/Source/WebCore/bindings/js/JSIDBCursorCustom.cpp
new file mode 100644 (file)
index 0000000..63f355a
--- /dev/null
@@ -0,0 +1,49 @@
+/*
+ * Copyright (C) 2016 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 "JSIDBCursor.h"
+
+#if ENABLE(INDEXED_DATABASE)
+
+#include "IDBCursorImpl.h"
+
+using namespace JSC;
+
+namespace WebCore {
+
+void JSIDBCursor::visitAdditionalChildren(SlotVisitor& visitor)
+{
+    if (!wrapped().isModernCursor())
+        return;
+
+    auto& modernCursor = static_cast<IDBClient::IDBCursor&>(wrapped());
+    if (auto* request = modernCursor.request())
+        visitor.addOpaqueRoot(request);
+}
+
+} // namespace WebCore
+
+#endif // ENABLE(INDEXED_DATABASE)