|
|
Created:
8 years, 7 months ago by jsbell Modified:
8 years, 6 months ago Reviewers:
tony CC:
chromium-reviews, pam+watch_chromium.org, darin-cc_chromium.org, dgrogan, alecflett Base URL:
svn://svn.chromium.org/chrome/trunk/src Visibility:
Public. |
DescriptionUse a temp data dir for IDB tests so persistence can be verified.
WebCore::IDBFactory always generates specifies an empty string (indicating in-memory storage), and relies on intermediate methods before WebCore::IDBFactoryBackendImpl is reached to rewrite with a non-empty string to use on-disk storage.
This patch inserts a test-specific implementation of WebIDBFactory into the middle of the call path to specify a data dir if one was not specified, mirroring what is done in multiprocess Chromium during IPC.
Prep work for http://webkit.org/b/83074
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=140892
Patch Set 1 #Patch Set 2 : Don't require a new WebKit API #Patch Set 3 : Only override if empty #Patch Set 4 : Move to IDBFactory-specific temp dir #Messages
Total messages: 13 (0 generated)
In multi-process Chromium, the full call path is: user script calls indexedDB.open(...) WebCore::IDBFactory::open - calls m_backend->open(..., String()) // always empty WebKit::IDBFactoryBackendProxy::open - calls m_webIDBFactory->open(..., dataDir) content::RendererWebIDBFactoryImpl::open - calls dispatcher->RequestIDBFactoryOpen (w/ no dir) content::IndexedDBDispatcher::RequestIDBFactoryOpen - sends IndexedDBHostMsg_FactoryOpen over IPC (w/ no dir) content::IndexedDBDispatcherHost::OnIDBFactoryOpen - receives IPC, calls idb_factory_->open() with indexed_db_context_->data_path() WebKit::WebIDBFactoryImpl::open - calls m_idbFactoryBackend->open(... dataDir) WebCore::IDBFactoryBackendImpl::open - and then into implementation details like leveldb DumpRenderTree doesn't have the IPC steps, so the path is: user script calls indexedDB.open(...) WebCore::IDBFactory::open - calls m_backend->open(..., String()) // always empty WebKit::IDBFactoryBackendProxy::open - calls m_webIDBFactory->open(..., dataDir) WebKit::WebIDBFactoryImpl::open - calls m_idbFactoryBackend->open(..., dataDir) WebCore::IDBFactoryBackendImpl::open - and then into implementation details like leveldb ...which leaves the path empty, which uses an in-memory leveldb instance, which prevents testing persistence. This patch inserts a test-specific implementation of WebIDBFactory into the middle of that path to specify a data dir if one was not specified: user script calls indexedDB.open(...) WebCore::IDBFactory::open - calls m_backend->open(..., String()) // always empty WebKit::IDBFactoryBackendProxy::open - calls m_webIDBFactory->open(..., dataDir) TestWebIDBFactory::open - calls factory_(..., dataDir.isEmpty() ? data_dir_ : dataDir) WebKit::WebIDBFactoryImpl::open - calls m_idbFactoryBackend->open(... dataDir) WebCore::IDBFactoryBackendImpl::open - and then into implementation details like leveldb Arguably, the data directory could be specified by an explicit WebKit API during IDBFactory construction rather than being overridden by during IPC. The IDBFactory::deleteDatabase() and IDBFactory::getDatabaseNames() methods - but NOT IDBFactory::open() - make a call to groupSettings->indexedDBDatabasePath() but GroupSettings is not available in workers (those 2 methods need to be exposed to workers - see crbug.com/112535) and not exposed in the WebKit API; that GroupSettings API should be completely removed or rethought on the WebKit side.)
... and FWIW, the motivating factor for this is crbug.com/76641 via http://webkit.org/83074 - once you can actually release back end resources, layout tests which do so need the data persisted for the duration of the test.
tony@ - could you take a look?
LGTM
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jsbell@chromium.org/10382180/4001
Change committed as 137733
Whoops, causing WebKit layout tests to fail on Windows: storage/indexeddb/pending-version-change-on-exit.html storage/indexeddb/index-basics-workers.html storage/indexeddb/objectstore-basics-workers.html storage/indexeddb/transaction-abort-workers.html Weird that it's only this combination of tests. Possibly related to grouping non-worker/worker versions of the tests into the same DRT batch? Pathname encoding? Sample diffs: --- e:\b\build\slave\Webkit_Win\build\layout-test-results\storage/indexeddb/index-basics-workers-expected.txt +++ e:\b\build\slave\Webkit_Win\build\layout-test-results\storage/indexeddb/index-basics-workers-actual.txt @@ -7,101 +7,7 @@ [Worker] indexedDB = self.indexedDB || self.webkitIndexedDB || self.mozIndexedDB || self.msIndexedDB || self.OIndexedDB; [Worker] [Worker] indexedDB.open('index-basics') -[Worker] db = event.target.result -[Worker] db.setVersion('new version') -[Worker] setVersionSuccess(): -[Worker] trans = event.target.result -PASS [Worker] trans !== null is true -[Worker] Deleted all object stores. -[Worker] db.createObjectStore('storeName', null) -[Worker] store.createIndex('indexName', 'x') -[Worker] store.createIndex('indexName2', 'y', {unique: false}) -[Worker] store.createIndex('zIndex', 'z', {unique: true}) -PASS [Worker] indexObject2.unique is false -PASS [Worker] indexObject3.unique is true -[Worker] Expecting TypeError exception from store.createIndex('failureIndex', 'zzz', true) -PASS [Worker] Exception was thrown. -PASS [Worker] store.createIndex('failureIndex', 'zzz', true) threw TypeError: Not an object. -[Worker] Expecting TypeError exception from store.createIndex('failureIndex', 'zzz', 'string') -PASS [Worker] Exception was thrown. -PASS [Worker] store.createIndex('failureIndex', 'zzz', 'string') threw TypeError: Not an object. -PASS [Worker] 'name' in indexObject is true -PASS [Worker] indexObject.name is "indexName" -PASS [Worker] 'objectStore' in indexObject is true -PASS [Worker] indexObject.objectStore.name is "storeName" -PASS [Worker] 'keyPath' in indexObject is true -PASS [Worker] indexObject.keyPath is "x" -PASS [Worker] 'unique' in indexObject is true -PASS [Worker] 'multiEntry' in indexObject is true -PASS [Worker] indexObject.unique is false -PASS [Worker] indexObject.multiEntry is false -PASS [Worker] 'openKeyCursor' in indexObject is true -PASS [Worker] typeof indexObject.openKeyCursor is "function" -PASS [Worker] 'openCursor' in indexObject is true -PASS [Worker] typeof indexObject.openCursor is "function" -PASS [Worker] 'getKey' in indexObject is true -PASS [Worker] typeof indexObject.getKey is "function" -PASS [Worker] 'get' in indexObject is true -PASS [Worker] typeof indexObject.get is "function" -PASS [Worker] 'count' in indexObject is true -PASS [Worker] typeof indexObject.count is "function" -[Worker] store.add({x: 'value', y: 'zzz', z: 2.72}, 'key') -[Worker] event.target.source.add({x: 'value2', y: 'zzz2', z: 2.71, foobar: 12}, 'key2') -[Worker] store.createIndex('indexWhileAddIsInFlight', 'x') -[Worker] store.createIndex('indexWithWeirdKeyPath', 'foobar') -[Worker] event.target.source.add({x: 'value3', y: '456'}, 'key3') -[Worker] indexObject.getKey('value') -PASS [Worker] event.target.result is "key" -[Worker] indexObject2.getKey('zzz') -PASS [Worker] event.target.result is "key" -[Worker] indexObject3.get(2.71) -PASS [Worker] event.target.result.x is "value2" -[Worker] indexObject.get('value') -PASS [Worker] event.target.result.x is "value" -PASS [Worker] event.target.result.y is "zzz" -[Worker] indexObject.getKey('does not exist') -PASS [Worker] event.target.result is undefined -[Worker] indexObject.get('does not exist') -PASS [Worker] event.target.result is undefined -[Worker] indexObject4.getKey('value2') -PASS [Worker] event.target.result is "key2" -[Worker] indexObject.openKeyCursor() -PASS [Worker] event.target.source is indexObject -PASS [Worker] event.target.result === null is false -PASS [Worker] event.target.result.key is "value" -PASS [Worker] event.target.result.primaryKey is "key" -[Worker] event.target.result.continue() -PASS [Worker] event.target.result === null is false -PASS [Worker] event.target.result.key is "value2" -PASS [Worker] event.target.result.primaryKey is "key2" -[Worker] event.target.result.continue() -PASS [Worker] event.target.result === null is false -PASS [Worker] event.target.result.key is "value3" -PASS [Worker] event.target.result.primaryKey is "key3" -[Worker] event.target.result.continue() -PASS [Worker] event.target.result === null is true -[Worker] indexObject.openCursor() -PASS [Worker] event.target.source is indexObject -PASS [Worker] event.target.result === null is false -PASS [Worker] event.target.result.key is "value" -PASS [Worker] event.target.result.value.x is "value" -PASS [Worker] event.target.result.value.y is "zzz" -[Worker] event.target.result.continue() -PASS [Worker] event.target.result === null is false -PASS [Worker] event.target.result.key is "value2" -PASS [Worker] event.target.result.value.x is "value2" -PASS [Worker] event.target.result.value.y is "zzz2" -[Worker] event.target.result.continue() -PASS [Worker] event.target.result === null is false -PASS [Worker] event.target.result.key is "value3" -PASS [Worker] event.target.result.value.x is "value3" -PASS [Worker] event.target.result.value.y is "456" -[Worker] event.target.result.continue() -PASS [Worker] event.target.result === null is true -[Worker] Passing an invalid key into indexObject.get({}). -PASS [Worker] Caught exception: Error: DATA_ERR: DOM IDBDatabase Exception 5 -[Worker] Passing an invalid key into indexObject.getKey({}). -PASS [Worker] Caught exception: Error: DATA_ERR: DOM IDBDatabase Exception 5 +FAIL [Worker] Error function called unexpectedly: (1) Internal error. PASS successfullyParsed is true TEST COMPLETE --- e:\b\build\slave\Webkit_Win\build\layout-test-results\storage/indexeddb/pending-version-change-on-exit-expected.txt +++ e:\b\build\slave\Webkit_Win\build\layout-test-results\storage/indexeddb/pending-version-change-on-exit-actual.txt @@ -5,7 +5,8 @@ indexedDB = self.indexedDB || self.webkitIndexedDB || self.mozIndexedDB || self.msIndexedDB || self.OIndexedDB; -PASS Didn't crash! +request = indexedDB.open("pending-version-change-on-exit") +FAIL Error function called unexpectedly: (1) Internal error. PASS successfullyParsed is true TEST COMPLETE Reverted via https://chromiumcodereview.appspot.com/10408015
I can repro the failures locally on Windows. pending-version-change-on-exit.html fails (timeout) consistently. I believe it's failing due to https://bugs.webkit.org/show_bug.cgi?id=82776 and only passes "by accident" in DRT without this patch. The other tests don't fail if run individually or if the layout test runner is invoked with --run-singly
Debug spew from running the other tests in a batch: [10444:5796:1386275584:ERROR:test_webkit_platform_support.cc(375)] Temp dir: C:\Users\jsbell\AppData\Local\Temp\scoped_dir10444_18969 ERROR: Failed to open LevelDB database from C:\Users\jsbell\AppData\Local\Temp\scoped_dir10444_18969\file__0.indexeddb.leveldb: IO error: C:\Users\jsbell\AppData\Local\Temp\scoped_dir10444_18969\file__0.indexeddb.leveldb/LOCK: File currently in use. ERROR: IndexedDB backing store open failed, attempting cleanup ERROR: IndexedDB backing store cleanup failed
I believe the failures were due to workers holding on to leveldb locks longer than the lifetime of the specific test, at least on Windows. Moved the temp dir to be scoped to the IDBFactory instance. This has its own problems (see https://bugs.webkit.org/show_bug.cgi?id=82776) but is more consistent with previous DRT behavior.
Seems OK. LGTM
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jsbell@chromium.org/10382180/17001
Change committed as 140892 |