Title: [194967] trunk/Source/WebCore
Revision
194967
Author
beid...@apple.com
Date
2016-01-13 10:42:00 -0800 (Wed, 13 Jan 2016)

Log Message

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):

Modified Paths

Added Paths

Diff

Modified: trunk/Source/WebCore/CMakeLists.txt (194966 => 194967)


--- trunk/Source/WebCore/CMakeLists.txt	2016-01-13 18:19:45 UTC (rev 194966)
+++ trunk/Source/WebCore/CMakeLists.txt	2016-01-13 18:42:00 UTC (rev 194967)
@@ -1169,6 +1169,7 @@
     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

Modified: trunk/Source/WebCore/ChangeLog (194966 => 194967)


--- trunk/Source/WebCore/ChangeLog	2016-01-13 18:19:45 UTC (rev 194966)
+++ trunk/Source/WebCore/ChangeLog	2016-01-13 18:42:00 UTC (rev 194967)
@@ -1,3 +1,42 @@
+2016-01-13  Brady Eidson  <beid...@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  <za...@apple.com>
 
         Get text drawing working with display lists.

Modified: trunk/Source/WebCore/Modules/indexeddb/IDBCursor.h (194966 => 194967)


--- trunk/Source/WebCore/Modules/indexeddb/IDBCursor.h	2016-01-13 18:19:45 UTC (rev 194966)
+++ trunk/Source/WebCore/Modules/indexeddb/IDBCursor.h	2016-01-13 18:42:00 UTC (rev 194967)
@@ -74,6 +74,8 @@
 
     virtual bool isKeyCursor() const = 0;
 
+    virtual bool isModernCursor() const { return false; }
+
 protected:
     IDBCursor();
 };

Modified: trunk/Source/WebCore/Modules/indexeddb/IDBCursor.idl (194966 => 194967)


--- trunk/Source/WebCore/Modules/indexeddb/IDBCursor.idl	2016-01-13 18:19:45 UTC (rev 194966)
+++ trunk/Source/WebCore/Modules/indexeddb/IDBCursor.idl	2016-01-13 18:42:00 UTC (rev 194967)
@@ -27,6 +27,7 @@
     Conditional=INDEXED_DATABASE,
     EnabledAtRuntime=IndexedDB,
     SkipVTableValidation,
+    JSCustomMarkFunction,
 ] interface IDBCursor {
     readonly attribute IDBAny source;
     readonly attribute DOMString direction;

Modified: trunk/Source/WebCore/Modules/indexeddb/IDBRequest.idl (194966 => 194967)


--- trunk/Source/WebCore/Modules/indexeddb/IDBRequest.idl	2016-01-13 18:19:45 UTC (rev 194966)
+++ trunk/Source/WebCore/Modules/indexeddb/IDBRequest.idl	2016-01-13 18:42:00 UTC (rev 194967)
@@ -35,6 +35,7 @@
     JSGenerateToJSObject,
     JSGenerateToNativeObject,
     SkipVTableValidation,
+    GenerateIsReachable=Impl,
 ] interface IDBRequest : EventTarget {
     [GetterRaisesExceptionWithMessage] readonly attribute IDBAny result;
     [GetterRaisesExceptionWithMessage] readonly attribute DOMError error;

Modified: trunk/Source/WebCore/Modules/indexeddb/client/IDBCursorImpl.cpp (194966 => 194967)


--- trunk/Source/WebCore/Modules/indexeddb/client/IDBCursorImpl.cpp	2016-01-13 18:19:45 UTC (rev 194966)
+++ trunk/Source/WebCore/Modules/indexeddb/client/IDBCursorImpl.cpp	2016-01-13 18:42:00 UTC (rev 194967)
@@ -201,7 +201,7 @@
 
     m_gotValue = false;
 
-    uncheckedIteratorCursor(IDBKeyData(), count);
+    uncheckedIterateCursor(IDBKeyData(), count);
 }
 
 void IDBCursor::continueFunction(ScriptExecutionContext* context, ExceptionCodeWithMessage& ec)
@@ -276,10 +276,10 @@
 
     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);

Modified: trunk/Source/WebCore/Modules/indexeddb/client/IDBCursorImpl.h (194966 => 194967)


--- trunk/Source/WebCore/Modules/indexeddb/client/IDBCursorImpl.h	2016-01-13 18:19:45 UTC (rev 194966)
+++ trunk/Source/WebCore/Modules/indexeddb/client/IDBCursorImpl.h	2016-01-13 18:42:00 UTC (rev 194967)
@@ -72,6 +72,7 @@
     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 @@
     IDBObjectStore& effectiveObjectStore() const;
     IDBTransaction& transaction() const;
 
-    void uncheckedIteratorCursor(const IDBKeyData&, unsigned long count);
+    void uncheckedIterateCursor(const IDBKeyData&, unsigned long count);
 
     bool m_gotValue { false };
 

Modified: trunk/Source/WebCore/WebCore.xcodeproj/project.pbxproj (194966 => 194967)


--- trunk/Source/WebCore/WebCore.xcodeproj/project.pbxproj	2016-01-13 18:19:45 UTC (rev 194966)
+++ trunk/Source/WebCore/WebCore.xcodeproj/project.pbxproj	2016-01-13 18:42:00 UTC (rev 194967)
@@ -1955,6 +1955,7 @@
 		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, ); }; };
@@ -9392,6 +9393,7 @@
 		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>"; };
@@ -22147,6 +22149,7 @@
 				AB4CB4EA0B8BDA3D009F40B0 /* JSHTMLSelectElementCustom.h */,
 				D6F7960C166FFECE0076DD18 /* JSHTMLTemplateElementCustom.cpp */,
 				511EF2CC17F0FDF100E4FA16 /* JSIDBAnyCustom.cpp */,
+				512BDB4C1C46B0FF006494DF /* JSIDBCursorCustom.cpp */,
 				511EF2CD17F0FDF100E4FA16 /* JSIDBDatabaseCustom.cpp */,
 				511EF2CE17F0FDF100E4FA16 /* JSIDBObjectStoreCustom.cpp */,
 				A7D0318D0E93540300E24ACD /* JSImageDataCustom.cpp */,
@@ -30596,6 +30599,7 @@
 				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 */,

Added: trunk/Source/WebCore/bindings/js/JSIDBCursorCustom.cpp (0 => 194967)


--- trunk/Source/WebCore/bindings/js/JSIDBCursorCustom.cpp	                        (rev 0)
+++ trunk/Source/WebCore/bindings/js/JSIDBCursorCustom.cpp	2016-01-13 18:42:00 UTC (rev 194967)
@@ -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)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to