Title: [235410] trunk
Revision
235410
Author
you...@apple.com
Date
2018-08-27 16:41:11 -0700 (Mon, 27 Aug 2018)

Log Message

Various IndexDB tests abandon documents
https://bugs.webkit.org/show_bug.cgi?id=188728
<rdar://problem/43651095>

Reviewed by Alex Christensen.

Source/WebCore:

Some IDB objects implement hasPendingActivity but there are some possibilities that they continue returning true after being stopped.
This is the case for requests that get stopped while still waiting for some pending activity.
This is also the case for requests that emits upgradeneeded or blocked events.

Enforce that these objects return false to hasPendingActivity once being stopped.
This ensures that they can be garbage collected once their context is preparing for destruction like in Document::prepareForDestruction.

Test: http/tests/IndexedDB/collect-IDB-objects.https.html

* Modules/indexeddb/IDBIndex.cpp:
(WebCore::IDBIndex::hasPendingActivity const):
* Modules/indexeddb/IDBObjectStore.cpp:
(WebCore::IDBObjectStore::hasPendingActivity const):
* Modules/indexeddb/IDBRequest.cpp:
(WebCore::IDBRequest::hasPendingActivity const):
(WebCore::IDBRequest::enqueueEvent):
* Modules/indexeddb/IDBTransaction.cpp:
(WebCore::IDBTransaction::notifyDidAbort):
In case the context is stopped, IDBTransaction should not ask IDBRequest to fire an event.

LayoutTests:

* http/tests/IndexedDB/collect-IDB-objects.https-expected.txt: Added.
* http/tests/IndexedDB/collect-IDB-objects.https.html: Added.
* http/tests/IndexedDB/resources/myidbframe.htm: Added.
* http/tests/IndexedDB/resources/support.js: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (235409 => 235410)


--- trunk/LayoutTests/ChangeLog	2018-08-27 23:31:18 UTC (rev 235409)
+++ trunk/LayoutTests/ChangeLog	2018-08-27 23:41:11 UTC (rev 235410)
@@ -1,3 +1,16 @@
+2018-08-27  Youenn Fablet  <you...@apple.com>
+
+        Various IndexDB tests abandon documents
+        https://bugs.webkit.org/show_bug.cgi?id=188728
+        <rdar://problem/43651095>
+
+        Reviewed by Alex Christensen.
+
+        * http/tests/IndexedDB/collect-IDB-objects.https-expected.txt: Added.
+        * http/tests/IndexedDB/collect-IDB-objects.https.html: Added.
+        * http/tests/IndexedDB/resources/myidbframe.htm: Added.
+        * http/tests/IndexedDB/resources/support.js: Added.
+
 2018-08-27  Simon Fraser  <simon.fra...@apple.com>
 
         [LayoutTests] results.html shows "no expected results" for text diff failures

Added: trunk/LayoutTests/http/tests/IndexedDB/collect-IDB-objects.https-expected.txt (0 => 235410)


--- trunk/LayoutTests/http/tests/IndexedDB/collect-IDB-objects.https-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/http/tests/IndexedDB/collect-IDB-objects.https-expected.txt	2018-08-27 23:41:11 UTC (rev 235410)
@@ -0,0 +1,4 @@
+
+
+PASS Ensuring frame document gets collected after being stopped in the middle of IDB operations 
+

Added: trunk/LayoutTests/http/tests/IndexedDB/collect-IDB-objects.https.html (0 => 235410)


--- trunk/LayoutTests/http/tests/IndexedDB/collect-IDB-objects.https.html	                        (rev 0)
+++ trunk/LayoutTests/http/tests/IndexedDB/collect-IDB-objects.https.html	2018-08-27 23:41:11 UTC (rev 235410)
@@ -0,0 +1,45 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<script src=""
+<script src=""
+<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 = ""
+        let counter = 0;
+        while (++counter < 10) {
+            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;
+}, "Ensuring frame document gets collected after being stopped in the middle of IDB operations");
+
+</script>
+<iframe src="" id="iframe"></iframe>

Added: trunk/LayoutTests/http/tests/IndexedDB/resources/myidbframe.htm (0 => 235410)


--- trunk/LayoutTests/http/tests/IndexedDB/resources/myidbframe.htm	                        (rev 0)
+++ trunk/LayoutTests/http/tests/IndexedDB/resources/myidbframe.htm	2018-08-27 23:41:11 UTC (rev 235410)
@@ -0,0 +1,26 @@
+<!DOCTYPE html>
+<script src=""
+<script src=""
+
+<script>
+var t = async_test();
+createdb(t)._onupgradeneeded_ = function(e) {
+    e.target.result
+            .createObjectStore("store")
+            .add(new Date(), 1);
+
+    e.target._onsuccess_ = t.step_func(function(e) {
+        e.target.result
+                .transaction("store")
+                .objectStore("store")
+                .get(1)
+                ._onsuccess_ = t.step_func(function(e)
+        {
+            t.done();
+            parent.done();
+        });
+    });
+};
+</script>
+
+<div id="log"></div>

Added: trunk/LayoutTests/http/tests/IndexedDB/resources/support.js (0 => 235410)


--- trunk/LayoutTests/http/tests/IndexedDB/resources/support.js	                        (rev 0)
+++ trunk/LayoutTests/http/tests/IndexedDB/resources/support.js	2018-08-27 23:41:11 UTC (rev 235410)
@@ -0,0 +1,194 @@
+var databaseName = "database";
+var databaseVersion = 1;
+
+/* Delete created databases
+ *
+ * Go through each finished test, see if it has an associated database. Close
+ * that and delete the database. */
+add_completion_callback(function(tests)
+{
+    for (var i in tests)
+    {
+        if(tests[i].db)
+        {
+            tests[i].db.close();
+            self.indexedDB.deleteDatabase(tests[i].db.name);
+        }
+    }
+});
+
+function fail(test, desc) {
+    return test.step_func(function(e) {
+        if (e && e.message && e.target.error)
+            assert_unreached(desc + " (" + e.target.error.name + ": " + e.message + ")");
+        else if (e && e.message)
+            assert_unreached(desc + " (" + e.message + ")");
+        else if (e && e.target.readyState === 'done' && e.target.error)
+            assert_unreached(desc + " (" + e.target.error.name + ")");
+        else
+            assert_unreached(desc);
+    });
+}
+
+function createdb(test, dbname, version)
+{
+    var rq_open = createdb_for_multiple_tests(dbname, version);
+    return rq_open.setTest(test);
+}
+
+function createdb_for_multiple_tests(dbname, version) {
+    var rq_open,
+        fake_open = {},
+        test = null,
+        dbname = (dbname ? dbname : "testdb-" + new Date().getTime() + Math.random() );
+
+    if (version)
+        rq_open = self.indexedDB.open(dbname, version);
+    else
+        rq_open = self.indexedDB.open(dbname);
+
+    function auto_fail(evt, current_test) {
+        /* Fail handlers, if we haven't set on/whatever/, don't
+         * expect to get event whatever. */
+        rq_open.manually_handled = {};
+
+        rq_open.addEventListener(evt, function(e) {
+            if (current_test !== test) {
+                return;
+            }
+
+            test.step(function() {
+                if (!rq_open.manually_handled[evt]) {
+                    assert_unreached("unexpected open." + evt + " event");
+                }
+
+                if (e.target.result + '' == '[object IDBDatabase]' &&
+                    !this.db) {
+                  this.db = e.target.result;
+
+                  this.db._onerror_ = fail(test, 'unexpected db.error');
+                  this.db._onabort_ = fail(test, 'unexpected db.abort');
+                  this.db._onversionchange_ =
+                      fail(test, 'unexpected db.versionchange');
+                }
+            });
+        });
+        rq_open.__defineSetter__("on" + evt, function(h) {
+            rq_open.manually_handled[evt] = true;
+            if (!h)
+                rq_open.addEventListener(evt, function() {});
+            else
+                rq_open.addEventListener(evt, test.step_func(h));
+        });
+    }
+
+    // add a .setTest method to the IDBOpenDBRequest object
+    Object.defineProperty(rq_open, 'setTest', {
+        enumerable: false,
+        value: function(t) {
+            test = t;
+
+            auto_fail("upgradeneeded", test);
+            auto_fail("success", test);
+            auto_fail("blocked", test);
+            auto_fail("error", test);
+
+            return this;
+        }
+    });
+
+    return rq_open;
+}
+
+function assert_key_equals(actual, expected, description) {
+  assert_equals(indexedDB.cmp(actual, expected), 0, description);
+}
+
+function indexeddb_test(upgrade_func, open_func, description, options) {
+  async_test(function(t) {
+    options = Object.assign({upgrade_will_abort: false}, options);
+    var dbname = location + '-' + t.name;
+    var del = indexedDB.deleteDatabase(dbname);
+    del._onerror_ = t.unreached_func('deleteDatabase should succeed');
+    var open = indexedDB.open(dbname, 1);
+    open._onupgradeneeded_ = t.step_func(function() {
+      var db = open.result;
+      t.add_cleanup(function() {
+        // If open didn't succeed already, ignore the error.
+        open._onerror_ = function(e) {
+          e.preventDefault();
+        };
+        db.close();
+        indexedDB.deleteDatabase(db.name);
+      });
+      var tx = open.transaction;
+      upgrade_func(t, db, tx, open);
+    });
+    if (options.upgrade_will_abort) {
+      open._onsuccess_ = t.unreached_func('open should not succeed');
+    } else {
+      open._onerror_ = t.unreached_func('open should succeed');
+      open._onsuccess_ = t.step_func(function() {
+        var db = open.result;
+        if (open_func)
+          open_func(t, db, open);
+      });
+    }
+  }, description);
+}
+
+// Call with a Test and an array of expected results in order. Returns
+// a function; call the function when a result arrives and when the
+// expected number appear the order will be asserted and test
+// completed.
+function expect(t, expected) {
+  var results = [];
+  return result => {
+    results.push(result);
+    if (results.length === expected.length) {
+      assert_array_equals(results, expected);
+      t.done();
+    }
+  };
+}
+
+// Checks to see if the passed transaction is active (by making
+// requests against the named store).
+function is_transaction_active(tx, store_name) {
+  try {
+    const request = tx.objectStore(store_name).get(0);
+    request._onerror_ = e => {
+      e.preventDefault();
+      e.stopPropagation();
+    };
+    return true;
+  } catch (ex) {
+    assert_equals(ex.name, 'TransactionInactiveError',
+                  'Active check should either not throw anything, or throw ' +
+                  'TransactionInactiveError');
+    return false;
+  }
+}
+
+// Keep the passed transaction alive indefinitely (by making requests
+// against the named store). Returns a function to to let the
+// transaction finish, and asserts that the transaction is not yet
+// finished.
+function keep_alive(tx, store_name) {
+  let completed = false;
+  tx.addEventListener('complete', () => { completed = true; });
+
+  let pin = true;
+
+  function spin() {
+    if (!pin)
+      return;
+    tx.objectStore(store_name).get(0)._onsuccess_ = spin;
+  }
+  spin();
+
+  return () => {
+    assert_false(completed, 'Transaction completed while kept alive');
+    pin = false;
+  };
+}

Modified: trunk/Source/WebCore/ChangeLog (235409 => 235410)


--- trunk/Source/WebCore/ChangeLog	2018-08-27 23:31:18 UTC (rev 235409)
+++ trunk/Source/WebCore/ChangeLog	2018-08-27 23:41:11 UTC (rev 235410)
@@ -1,3 +1,31 @@
+2018-08-27  Youenn Fablet  <you...@apple.com>
+
+        Various IndexDB tests abandon documents
+        https://bugs.webkit.org/show_bug.cgi?id=188728
+        <rdar://problem/43651095>
+
+        Reviewed by Alex Christensen.
+
+        Some IDB objects implement hasPendingActivity but there are some possibilities that they continue returning true after being stopped.
+        This is the case for requests that get stopped while still waiting for some pending activity.
+        This is also the case for requests that emits upgradeneeded or blocked events.
+
+        Enforce that these objects return false to hasPendingActivity once being stopped.
+        This ensures that they can be garbage collected once their context is preparing for destruction like in Document::prepareForDestruction.
+
+        Test: http/tests/IndexedDB/collect-IDB-objects.https.html
+
+        * Modules/indexeddb/IDBIndex.cpp:
+        (WebCore::IDBIndex::hasPendingActivity const):
+        * Modules/indexeddb/IDBObjectStore.cpp:
+        (WebCore::IDBObjectStore::hasPendingActivity const):
+        * Modules/indexeddb/IDBRequest.cpp:
+        (WebCore::IDBRequest::hasPendingActivity const):
+        (WebCore::IDBRequest::enqueueEvent):
+        * Modules/indexeddb/IDBTransaction.cpp:
+        (WebCore::IDBTransaction::notifyDidAbort):
+        In case the context is stopped, IDBTransaction should not ask IDBRequest to fire an event.
+
 2018-08-27  Simon Fraser  <simon.fra...@apple.com>
 
         Teach WebKitTestRunner and DumpRenderTree about detecting world leaks

Modified: trunk/Source/WebCore/Modules/indexeddb/IDBIndex.cpp (235409 => 235410)


--- trunk/Source/WebCore/Modules/indexeddb/IDBIndex.cpp	2018-08-27 23:31:18 UTC (rev 235409)
+++ trunk/Source/WebCore/Modules/indexeddb/IDBIndex.cpp	2018-08-27 23:41:11 UTC (rev 235410)
@@ -69,7 +69,7 @@
 
 bool IDBIndex::hasPendingActivity() const
 {
-    return !m_objectStore.transaction().isFinished();
+    return m_objectStore.transaction().hasPendingActivity();
 }
 
 const String& IDBIndex::name() const

Modified: trunk/Source/WebCore/Modules/indexeddb/IDBObjectStore.cpp (235409 => 235410)


--- trunk/Source/WebCore/Modules/indexeddb/IDBObjectStore.cpp	2018-08-27 23:31:18 UTC (rev 235409)
+++ trunk/Source/WebCore/Modules/indexeddb/IDBObjectStore.cpp	2018-08-27 23:41:11 UTC (rev 235410)
@@ -82,7 +82,7 @@
 
 bool IDBObjectStore::hasPendingActivity() const
 {
-    return !m_transaction.isFinished();
+    return m_transaction.hasPendingActivity();
 }
 
 const String& IDBObjectStore::name() const

Modified: trunk/Source/WebCore/Modules/indexeddb/IDBRequest.cpp (235409 => 235410)


--- trunk/Source/WebCore/Modules/indexeddb/IDBRequest.cpp	2018-08-27 23:31:18 UTC (rev 235409)
+++ trunk/Source/WebCore/Modules/indexeddb/IDBRequest.cpp	2018-08-27 23:41:11 UTC (rev 235410)
@@ -261,7 +261,7 @@
 bool IDBRequest::hasPendingActivity() const
 {
     ASSERT(&originThread() == &Thread::current() || mayBeGCThread());
-    return m_hasPendingActivity;
+    return !m_contextStopped && m_hasPendingActivity;
 }
 
 void IDBRequest::stop()
@@ -284,7 +284,7 @@
 void IDBRequest::enqueueEvent(Ref<Event>&& event)
 {
     ASSERT(&originThread() == &Thread::current());
-    if (!scriptExecutionContext() || m_contextStopped)
+    if (m_contextStopped)
         return;
 
     event->setTarget(this);

Modified: trunk/Source/WebCore/Modules/indexeddb/IDBTransaction.cpp (235409 => 235410)


--- trunk/Source/WebCore/Modules/indexeddb/IDBTransaction.cpp	2018-08-27 23:31:18 UTC (rev 235409)
+++ trunk/Source/WebCore/Modules/indexeddb/IDBTransaction.cpp	2018-08-27 23:41:11 UTC (rev 235410)
@@ -551,7 +551,7 @@
     m_idbError = error;
     fireOnAbort();
 
-    if (isVersionChange()) {
+    if (isVersionChange() && !m_contextStopped) {
         ASSERT(m_openDBRequest);
         m_openDBRequest->fireErrorAfterVersionChangeCompletion();
     }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to