Skip to content

Commit 71a070e

Browse files
JosephPecorarowebkit-commit-queue
JosephPecoraro
authored and
webkit-commit-queue
committed
Race between creating/deleting a database in test
https://bugs.webkit.org/show_bug.cgi?id=161285 Patch by Joseph Pecoraro <pecoraro@apple.com> on 2016-08-27 Reviewed by Darin Adler. Because IndexedDB creation is asynchronous, we were not waiting for the database to be completely open before sending more commands. These could race and give unexpected results. Wait for the database to be successfully created before interacting with it. * inspector/indexeddb/resources/utilities.js: (createEmptyDatabase): (createDatabaseWithStores): Send a single when the database creation is completed. * inspector/indexeddb/clearObjectStore-expected.txt: * inspector/indexeddb/clearObjectStore.html: * inspector/indexeddb/deleteDatabaseNamesWithSpace.html: * inspector/indexeddb/requestData-expected.txt: * inspector/indexeddb/requestDatabase-expected.txt: * inspector/indexeddb/requestDatabase.html: * inspector/indexeddb/requestDatabaseNames.html: Listen for the database created signal before proceeding with the test. Canonical link: https://commits.webkit.org/179460@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@205086 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent e65d27e commit 71a070e

9 files changed

Lines changed: 120 additions & 81 deletions

‎LayoutTests/ChangeLog‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,30 @@
1+
2016-08-27 Joseph Pecoraro <pecoraro@apple.com>
2+
3+
Race between creating/deleting a database in test
4+
https://bugs.webkit.org/show_bug.cgi?id=161285
5+
6+
Reviewed by Darin Adler.
7+
8+
Because IndexedDB creation is asynchronous, we were not waiting for the
9+
database to be completely open before sending more commands. These
10+
could race and give unexpected results. Wait for the database to be
11+
successfully created before interacting with it.
12+
13+
* inspector/indexeddb/resources/utilities.js:
14+
(createEmptyDatabase):
15+
(createDatabaseWithStores):
16+
Send a single when the database creation is completed.
17+
18+
* inspector/indexeddb/clearObjectStore-expected.txt:
19+
* inspector/indexeddb/clearObjectStore.html:
20+
* inspector/indexeddb/deleteDatabaseNamesWithSpace.html:
21+
* inspector/indexeddb/requestData-expected.txt:
22+
* inspector/indexeddb/requestDatabase-expected.txt:
23+
* inspector/indexeddb/requestDatabase.html:
24+
* inspector/indexeddb/requestDatabaseNames.html:
25+
Listen for the database created signal before proceeding
26+
with the test.
27+
128
2016-08-27 Ryosuke Niwa <rniwa@webkit.org>
229

330
adoptcallback

‎LayoutTests/inspector/indexeddb/clearObjectStore-expected.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
CONSOLE MESSAGE: line 18: Created Database 'CompleteDatabase'
1+
CONSOLE MESSAGE: line 19: Created Database 'CompleteDatabase'
22
CONSOLE MESSAGE: line 9: Created Database 'EmptyDatabase'
33

44
== Running test suite: IndexedDB.clearObjectStore

‎LayoutTests/inspector/indexeddb/clearObjectStore.html‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -71,10 +71,12 @@
7171
name: "NoSuchObjectStore",
7272
test: (resolve, reject) => {
7373
InspectorTest.evaluateInPage("createEmptyDatabase('EmptyDatabase', 1)");
74-
IndexedDBAgent.requestDatabase(WebInspector.frameResourceManager.mainFrame.securityOrigin, "EmptyDatabase", (error, database) => {
75-
IndexedDBAgent.clearObjectStore(WebInspector.frameResourceManager.mainFrame.securityOrigin, database.name, "NoSuchObjectStore", (error) => {
76-
InspectorTest.expectThat(error, "Should be an error attempting to clear an object store that does not exist.");
77-
resolve();
74+
InspectorTest.singleFireEventListener("DatabaseCreated", (event) => {
75+
IndexedDBAgent.requestDatabase(WebInspector.frameResourceManager.mainFrame.securityOrigin, "EmptyDatabase", (error, database) => {
76+
IndexedDBAgent.clearObjectStore(WebInspector.frameResourceManager.mainFrame.securityOrigin, database.name, "NoSuchObjectStore", (error) => {
77+
InspectorTest.expectThat(error, "Should be an error attempting to clear an object store that does not exist.");
78+
resolve();
79+
});
7880
});
7981
});
8082
}

‎LayoutTests/inspector/indexeddb/deleteDatabaseNamesWithSpace.html‎

Lines changed: 18 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -24,14 +24,11 @@
2424
name: "EnsureNoDatabases",
2525
description: "Ensure no databases exist at the start.",
2626
test: (resolve, reject) => {
27-
// FIXME: <https://webkit.org/b/161285> Race between deleting a database and requesting database names seen in test
28-
setTimeout(() => {
29-
IndexedDBAgent.requestDatabaseNames(WebInspector.frameResourceManager.mainFrame.securityOrigin, (error, names) => {
30-
InspectorTest.expectNoError(error);
31-
InspectorTest.expectThat(names.length === 0, "No IndexedDB databases should exist initially.");
32-
resolve();
33-
});
34-
}, 50);
27+
IndexedDBAgent.requestDatabaseNames(WebInspector.frameResourceManager.mainFrame.securityOrigin, (error, names) => {
28+
InspectorTest.expectNoError(error);
29+
InspectorTest.expectThat(names.length === 0, "No IndexedDB databases should exist initially.");
30+
resolve();
31+
});
3532
}
3633
});
3734

@@ -40,11 +37,13 @@
4037
description: "Create a database with spaces in the name.",
4138
test: (resolve, reject) => {
4239
InspectorTest.evaluateInPage("createEmptyDatabase('Database With Space')");
43-
IndexedDBAgent.requestDatabaseNames(WebInspector.frameResourceManager.mainFrame.securityOrigin, (error, names) => {
44-
InspectorTest.expectNoError(error);
45-
InspectorTest.expectThat(names.length === 1, "A single IndexedDB database should exist.");
46-
InspectorTest.log(JSON.stringify(names));
47-
resolve();
40+
InspectorTest.singleFireEventListener("DatabaseCreated", (event) => {
41+
IndexedDBAgent.requestDatabaseNames(WebInspector.frameResourceManager.mainFrame.securityOrigin, (error, names) => {
42+
InspectorTest.expectNoError(error);
43+
InspectorTest.expectThat(names.length === 1, "A single IndexedDB database should exist.");
44+
InspectorTest.log(JSON.stringify(names));
45+
resolve();
46+
});
4847
});
4948
}
5049
});
@@ -54,15 +53,12 @@
5453
description: "Delete the database.",
5554
test: (resolve, reject) => {
5655
InspectorTest.evaluateInPage("deleteDatabaseNames(['Database With Space'])");
57-
// FIXME: <https://webkit.org/b/161285> Race between deleting a database and requesting database names seen in test
58-
setTimeout(() => {
59-
IndexedDBAgent.requestDatabaseNames(WebInspector.frameResourceManager.mainFrame.securityOrigin, (error, names) => {
60-
InspectorTest.expectNoError(error);
61-
InspectorTest.expectThat(names.length === 0, "No IndexedDB databases should exist because we just deleted them.");
62-
InspectorTest.log(JSON.stringify(names));
63-
resolve();
64-
});
65-
}, 50);
56+
IndexedDBAgent.requestDatabaseNames(WebInspector.frameResourceManager.mainFrame.securityOrigin, (error, names) => {
57+
InspectorTest.expectNoError(error);
58+
InspectorTest.expectThat(names.length === 0, "No IndexedDB databases should exist because we just deleted them.");
59+
InspectorTest.log(JSON.stringify(names));
60+
resolve();
61+
});
6662
}
6763
});
6864

‎LayoutTests/inspector/indexeddb/requestData-expected.txt‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
CONSOLE MESSAGE: line 18: Created Database 'Database1'
2-
CONSOLE MESSAGE: line 18: Created Database 'Database2'
3-
CONSOLE MESSAGE: line 18: Created Database 'Database3'
1+
CONSOLE MESSAGE: line 19: Created Database 'Database1'
2+
CONSOLE MESSAGE: line 19: Created Database 'Database2'
3+
CONSOLE MESSAGE: line 19: Created Database 'Database3'
44

55
== Running test suite: IndexedDB.requestData
66
-- Running test case: ClearDatabases

‎LayoutTests/inspector/indexeddb/requestDatabase-expected.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
CONSOLE MESSAGE: line 9: Created Database 'EmptyDatabase'
2-
CONSOLE MESSAGE: line 18: Created Database 'CompleteDatabase'
2+
CONSOLE MESSAGE: line 19: Created Database 'CompleteDatabase'
33

44
== Running test suite: IndexedDB.requestDatabase
55
-- Running test case: ClearDatabases

‎LayoutTests/inspector/indexeddb/requestDatabase.html‎

Lines changed: 34 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,14 @@
2424
name: "CreateAndRequestEmptyDatabase",
2525
test: (resolve, reject) => {
2626
InspectorTest.evaluateInPage("createEmptyDatabase('EmptyDatabase', 123)");
27-
IndexedDBAgent.requestDatabase(WebInspector.frameResourceManager.mainFrame.securityOrigin, "EmptyDatabase", (error, databaseWithObjectStores) => {
28-
InspectorTest.expectNoError(error);
29-
InspectorTest.expectThat(databaseWithObjectStores.name === "EmptyDatabase", "Database name should be 'EmptyDatabase'.");
30-
InspectorTest.expectThat(databaseWithObjectStores.version === 123, "Database version should be 123.");
31-
InspectorTest.expectThat(databaseWithObjectStores.objectStores.length === 0, "Database should not have any object stores.");
32-
resolve();
27+
InspectorTest.singleFireEventListener("DatabaseCreated", (event) => {
28+
IndexedDBAgent.requestDatabase(WebInspector.frameResourceManager.mainFrame.securityOrigin, "EmptyDatabase", (error, databaseWithObjectStores) => {
29+
InspectorTest.expectNoError(error);
30+
InspectorTest.expectThat(databaseWithObjectStores.name === "EmptyDatabase", "Database name should be 'EmptyDatabase'.");
31+
InspectorTest.expectThat(databaseWithObjectStores.version === 123, "Database version should be 123.");
32+
InspectorTest.expectThat(databaseWithObjectStores.objectStores.length === 0, "Database should not have any object stores.");
33+
resolve();
34+
});
3335
});
3436
}
3537
});
@@ -38,34 +40,36 @@
3840
name: "CreateAndRequestDatabaseWithStores",
3941
test: (resolve, reject) => {
4042
InspectorTest.evaluateInPage("createDatabaseWithStores('CompleteDatabase', 456)");
41-
IndexedDBAgent.requestDatabase(WebInspector.frameResourceManager.mainFrame.securityOrigin, "CompleteDatabase", (error, databaseWithObjectStores) => {
42-
InspectorTest.expectNoError(error);
43-
let objectStores = databaseWithObjectStores.objectStores;
44-
InspectorTest.expectThat(databaseWithObjectStores.name === "CompleteDatabase", "Database name should be 'EmptyDatabase'.");
45-
InspectorTest.expectThat(databaseWithObjectStores.version === 456, "Database version should be 456.");
46-
InspectorTest.expectThat(databaseWithObjectStores.objectStores.length === 3, "Database should have 3 object stores.");
43+
InspectorTest.singleFireEventListener("DatabaseCreated", (event) => {
44+
IndexedDBAgent.requestDatabase(WebInspector.frameResourceManager.mainFrame.securityOrigin, "CompleteDatabase", (error, databaseWithObjectStores) => {
45+
InspectorTest.expectNoError(error);
46+
let objectStores = databaseWithObjectStores.objectStores;
47+
InspectorTest.expectThat(databaseWithObjectStores.name === "CompleteDatabase", "Database name should be 'EmptyDatabase'.");
48+
InspectorTest.expectThat(databaseWithObjectStores.version === 456, "Database version should be 456.");
49+
InspectorTest.expectThat(databaseWithObjectStores.objectStores.length === 3, "Database should have 3 object stores.");
4750

48-
InspectorTest.expectThat(objectStores[0].name === "Empty", "Object store should have name 'Empty'.");
49-
InspectorTest.expectThat(objectStores[0].keyPath.type === "null", "Object store keypath is null.");
50-
InspectorTest.expectThat(!objectStores[0].autoIncrement, "Object store should not autoIncrement.");
51-
InspectorTest.expectThat(!objectStores[0].indexes.length, "Object store should have no indexes.");
51+
InspectorTest.expectThat(objectStores[0].name === "Empty", "Object store should have name 'Empty'.");
52+
InspectorTest.expectThat(objectStores[0].keyPath.type === "null", "Object store keypath is null.");
53+
InspectorTest.expectThat(!objectStores[0].autoIncrement, "Object store should not autoIncrement.");
54+
InspectorTest.expectThat(!objectStores[0].indexes.length, "Object store should have no indexes.");
5255

53-
InspectorTest.expectThat(objectStores[1].name === "Reviewers", "Object store should have name 'Reviewers'.");
54-
InspectorTest.expectThat(objectStores[1].keyPath.type === "null", "Object store keypath is null.");
55-
InspectorTest.expectThat(objectStores[1].autoIncrement, "Object store should autoIncrement.");
56-
InspectorTest.expectThat(objectStores[1].indexes.length === 2, "Object store should have 2 indexes.");
57-
InspectorTest.log("INDEX: " + JSON.stringify(objectStores[1].indexes[0]));
58-
InspectorTest.log("INDEX: " + JSON.stringify(objectStores[1].indexes[1]));
56+
InspectorTest.expectThat(objectStores[1].name === "Reviewers", "Object store should have name 'Reviewers'.");
57+
InspectorTest.expectThat(objectStores[1].keyPath.type === "null", "Object store keypath is null.");
58+
InspectorTest.expectThat(objectStores[1].autoIncrement, "Object store should autoIncrement.");
59+
InspectorTest.expectThat(objectStores[1].indexes.length === 2, "Object store should have 2 indexes.");
60+
InspectorTest.log("INDEX: " + JSON.stringify(objectStores[1].indexes[0]));
61+
InspectorTest.log("INDEX: " + JSON.stringify(objectStores[1].indexes[1]));
5962

60-
InspectorTest.expectThat(objectStores[2].name === "Stats", "Object store should have name 'Stats'.");
61-
InspectorTest.expectThat(objectStores[2].keyPath.type === "string", "Object store keypath is string type.");
62-
InspectorTest.expectThat(objectStores[2].keyPath.string === "name", "Object store keypath is 'name''.");
63-
InspectorTest.expectThat(!objectStores[2].autoIncrement, "Object store should not autoIncrement.");
64-
InspectorTest.expectThat(objectStores[2].indexes.length === 2, "Object store should have 2 indexes.");
65-
InspectorTest.log("INDEX: " + JSON.stringify(objectStores[2].indexes[0]));
66-
InspectorTest.log("INDEX: " + JSON.stringify(objectStores[2].indexes[1]));
63+
InspectorTest.expectThat(objectStores[2].name === "Stats", "Object store should have name 'Stats'.");
64+
InspectorTest.expectThat(objectStores[2].keyPath.type === "string", "Object store keypath is string type.");
65+
InspectorTest.expectThat(objectStores[2].keyPath.string === "name", "Object store keypath is 'name''.");
66+
InspectorTest.expectThat(!objectStores[2].autoIncrement, "Object store should not autoIncrement.");
67+
InspectorTest.expectThat(objectStores[2].indexes.length === 2, "Object store should have 2 indexes.");
68+
InspectorTest.log("INDEX: " + JSON.stringify(objectStores[2].indexes[0]));
69+
InspectorTest.log("INDEX: " + JSON.stringify(objectStores[2].indexes[1]));
6770

68-
resolve();
71+
resolve();
72+
});
6973
});
7074
}
7175
});

‎LayoutTests/inspector/indexeddb/requestDatabaseNames.html‎

Lines changed: 28 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,13 @@
3737
description: "Create a first database.",
3838
test: (resolve, reject) => {
3939
InspectorTest.evaluateInPage("createEmptyDatabase('Database1')");
40-
IndexedDBAgent.requestDatabaseNames(WebInspector.frameResourceManager.mainFrame.securityOrigin, (error, names) => {
41-
InspectorTest.expectNoError(error);
42-
InspectorTest.expectThat(names.length === 1, "A single IndexedDB database should exist.");
43-
InspectorTest.log(JSON.stringify(names));
44-
resolve();
40+
InspectorTest.singleFireEventListener("DatabaseCreated", (event) => {
41+
IndexedDBAgent.requestDatabaseNames(WebInspector.frameResourceManager.mainFrame.securityOrigin, (error, names) => {
42+
InspectorTest.expectNoError(error);
43+
InspectorTest.expectThat(names.length === 1, "A single IndexedDB database should exist.");
44+
InspectorTest.log(JSON.stringify(names));
45+
resolve();
46+
});
4547
});
4648
}
4749
});
@@ -51,11 +53,13 @@
5153
description: "Create a second database.",
5254
test: (resolve, reject) => {
5355
InspectorTest.evaluateInPage("createEmptyDatabase('Database2')");
54-
IndexedDBAgent.requestDatabaseNames(WebInspector.frameResourceManager.mainFrame.securityOrigin, (error, names) => {
55-
InspectorTest.expectNoError(error);
56-
InspectorTest.expectThat(names.length === 2, "Two IndexedDB databases should exist.");
57-
InspectorTest.log(JSON.stringify(names));
58-
resolve();
56+
InspectorTest.singleFireEventListener("DatabaseCreated", (event) => {
57+
IndexedDBAgent.requestDatabaseNames(WebInspector.frameResourceManager.mainFrame.securityOrigin, (error, names) => {
58+
InspectorTest.expectNoError(error);
59+
InspectorTest.expectThat(names.length === 2, "Two IndexedDB databases should exist.");
60+
InspectorTest.log(JSON.stringify(names));
61+
resolve();
62+
});
5963
});
6064
}
6165
});
@@ -65,11 +69,13 @@
6569
description: "Create a third database with a unicode name.",
6670
test: (resolve, reject) => {
6771
InspectorTest.evaluateInPage("createEmptyDatabase('\u124d')");
68-
IndexedDBAgent.requestDatabaseNames(WebInspector.frameResourceManager.mainFrame.securityOrigin, (error, names) => {
69-
InspectorTest.expectNoError(error);
70-
InspectorTest.expectThat(names.length === 3, "Three IndexedDB databases should exist.");
71-
InspectorTest.log(JSON.stringify(names));
72-
resolve();
72+
InspectorTest.singleFireEventListener("DatabaseCreated", (event) => {
73+
IndexedDBAgent.requestDatabaseNames(WebInspector.frameResourceManager.mainFrame.securityOrigin, (error, names) => {
74+
InspectorTest.expectNoError(error);
75+
InspectorTest.expectThat(names.length === 3, "Three IndexedDB databases should exist.");
76+
InspectorTest.log(JSON.stringify(names));
77+
resolve();
78+
});
7379
});
7480
}
7581
});
@@ -79,11 +85,13 @@
7985
description: "Create a fourth database with a unicode name.",
8086
test: (resolve, reject) => {
8187
InspectorTest.evaluateInPage("createEmptyDatabase('\ud800\udf46')");
82-
IndexedDBAgent.requestDatabaseNames(WebInspector.frameResourceManager.mainFrame.securityOrigin, (error, names) => {
83-
InspectorTest.expectNoError(error);
84-
InspectorTest.expectThat(names.length === 4, "Four IndexedDB databases should exist.");
85-
InspectorTest.log(JSON.stringify(names));
86-
resolve();
88+
InspectorTest.singleFireEventListener("DatabaseCreated", (event) => {
89+
IndexedDBAgent.requestDatabaseNames(WebInspector.frameResourceManager.mainFrame.securityOrigin, (error, names) => {
90+
InspectorTest.expectNoError(error);
91+
InspectorTest.expectThat(names.length === 4, "Four IndexedDB databases should exist.");
92+
InspectorTest.log(JSON.stringify(names));
93+
resolve();
94+
});
8795
});
8896
}
8997
});

‎LayoutTests/inspector/indexeddb/resources/utilities.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,15 @@ function createEmptyDatabase(name, version=1) {
99
console.log(`Created Database '${name}'`);
1010
let db = event.target.result;
1111
db.close();
12+
TestPage.dispatchEventToFrontend("DatabaseCreated");
1213
});
1314
}
1415

1516
function createDatabaseWithStores(name, version) {
1617
let request = window.indexedDB.open(name, version);
1718
request.addEventListener("success", function(event) {
1819
console.log(`Created Database '${name}'`);
20+
TestPage.dispatchEventToFrontend("DatabaseCreated");
1921
});
2022

2123
request.addEventListener("upgradeneeded", function(event) {

0 commit comments

Comments
 (0)