Skip to content

Commit 684c543

Browse files
stelar7trflynn89
stelar7
authored and
trflynn89
committed
LibWeb/IDB: Handle cursor iteration more correctly
1 parent a8514a2 commit 684c543

File tree

9 files changed

+171
-110
lines changed

9 files changed

+171
-110
lines changed

‎Libraries/LibWeb/IndexedDB/Internal/Algorithms.cpp‎

Lines changed: 74 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -1599,27 +1599,28 @@ GC::Ptr<IDBCursor> iterate_a_cursor(JS::Realm& realm, GC::Ref<IDBCursor> cursor,
15991599
// * If key is defined:
16001600
if (key) {
16011601
// * The record’s key is greater than or equal to key.
1602-
auto is_greater_than_or_equal = record.visit(
1603-
[](Empty) { VERIFY_NOT_REACHED(); },
1604-
[key](auto const& inner_record) {
1605-
return Key::greater_than(inner_record.key, *key) || Key::equals(inner_record.key, *key);
1606-
});
1607-
1608-
if (!is_greater_than_or_equal)
1602+
if (!record.visit([&](auto const& inner_record) {
1603+
return Key::greater_than_or_equal(inner_record.key, *key);
1604+
}))
16091605
return false;
16101606
}
16111607

16121608
// * If primaryKey is defined:
16131609
if (primary_key) {
16141610
auto const& inner_record = record.get<IndexRecord>();
16151611

1616-
// * The record’s key is equal to key and the record’s value is greater than or equal to primaryKey
1617-
if (!(Key::equals(inner_record.key, *key) && (Key::greater_than(inner_record.value, *primary_key) || Key::equals(inner_record.value, *primary_key))))
1618-
return false;
1619-
1620-
// * The record’s key is greater than key.
1621-
if (!Key::greater_than(inner_record.key, *key))
1622-
return false;
1612+
// * If the record’s key is equal to key:
1613+
if (Key::equals(inner_record.key, *key)) {
1614+
// * The record’s value is greater than or equal to primaryKey
1615+
if (!Key::greater_than_or_equal(inner_record.value, *primary_key))
1616+
return false;
1617+
}
1618+
// * Else:
1619+
else {
1620+
// * The record’s key is greater than key.
1621+
if (!Key::greater_than(inner_record.key, *key))
1622+
return false;
1623+
}
16231624
}
16241625

16251626
// * If position is defined and source is an object store:
@@ -1635,87 +1636,79 @@ GC::Ptr<IDBCursor> iterate_a_cursor(JS::Realm& realm, GC::Ref<IDBCursor> cursor,
16351636
if (position && source.has<GC::Ref<Index>>()) {
16361637
auto const& inner_record = record.get<IndexRecord>();
16371638

1638-
// * The record’s key is equal to position and the record’s value is greater than object store position
1639-
if (!(Key::equals(inner_record.key, *position) && (Key::greater_than(inner_record.value, *object_store_position))))
1640-
return false;
1641-
1642-
// * The record’s key is greater than position.
1643-
if (!Key::greater_than(inner_record.key, *position))
1644-
return false;
1639+
// * If the record’s key is equal to position:
1640+
if (Key::equals(inner_record.key, *position)) {
1641+
// * The record’s value is greater than object store position
1642+
if (!Key::greater_than(inner_record.value, *object_store_position))
1643+
return false;
1644+
}
1645+
// * Else:
1646+
else {
1647+
// * The record’s key is greater than position.
1648+
if (!Key::greater_than(inner_record.key, *position))
1649+
return false;
1650+
}
16451651
}
16461652

16471653
// * The record’s key is in range.
1648-
auto is_in_range = record.visit(
1649-
[](Empty) { VERIFY_NOT_REACHED(); },
1650-
[range](auto const& inner_record) {
1654+
return record.visit(
1655+
[&](auto const& inner_record) {
16511656
return range->is_in_range(inner_record.key);
16521657
});
1653-
1654-
return is_in_range;
16551658
};
16561659

16571660
auto next_unique_requirements = [&](Variant<ObjectStoreRecord, IndexRecord> const& record) -> bool {
16581661
// * If key is defined:
16591662
if (key) {
16601663
// * The record’s key is greater than or equal to key.
1661-
auto is_greater_than_or_equal = record.visit(
1662-
[](Empty) { VERIFY_NOT_REACHED(); },
1663-
[key](auto const& inner_record) {
1664-
return Key::greater_than(inner_record.key, *key) || Key::equals(inner_record.key, *key);
1665-
});
1666-
1667-
if (!is_greater_than_or_equal)
1664+
if (!record.visit([&](auto const& inner_record) {
1665+
return Key::greater_than_or_equal(inner_record.key, *key);
1666+
}))
16681667
return false;
16691668
}
16701669

16711670
// * If position is defined:
16721671
if (position) {
16731672
// * The record’s key is greater than position.
1674-
auto is_greater_than_position = record.visit(
1675-
[](Empty) { VERIFY_NOT_REACHED(); },
1676-
[position](auto const& inner_record) {
1677-
return Key::greater_than(inner_record.key, *position) || Key::equals(inner_record.key, *position);
1678-
});
1679-
1680-
if (!is_greater_than_position)
1673+
if (!record.visit([&](auto const& inner_record) {
1674+
return Key::greater_than(inner_record.key, *position);
1675+
}))
16811676
return false;
16821677
}
16831678

16841679
// * The record’s key is in range.
1685-
auto is_in_range = record.visit(
1686-
[](Empty) { VERIFY_NOT_REACHED(); },
1687-
[range](auto const& inner_record) {
1680+
return record.visit(
1681+
[&](auto const& inner_record) {
16881682
return range->is_in_range(inner_record.key);
16891683
});
1690-
1691-
return is_in_range;
16921684
};
16931685

16941686
auto prev_requirements = [&](Variant<ObjectStoreRecord, IndexRecord> const& record) -> bool {
16951687
// * If key is defined:
16961688
if (key) {
16971689
// * The record’s key is less than or equal to key.
1698-
auto is_less_than_or_equal = record.visit(
1699-
[](Empty) { VERIFY_NOT_REACHED(); },
1700-
[key](auto const& inner_record) {
1701-
return Key::less_than(inner_record.key, *key) || Key::equals(inner_record.key, *key);
1702-
});
1703-
1704-
if (!is_less_than_or_equal)
1690+
if (!record.visit([&](auto const& inner_record) {
1691+
return Key::less_than_or_equal(inner_record.key, *key);
1692+
}))
17051693
return false;
17061694
}
17071695

17081696
// * If primaryKey is defined:
17091697
if (primary_key) {
17101698
auto const& inner_record = record.get<IndexRecord>();
17111699

1712-
// * The record’s key is equal to key and the record’s value is less than or equal to primaryKey
1713-
if (!(Key::equals(inner_record.key, *key) && (Key::less_than(inner_record.value, *primary_key) || Key::equals(inner_record.value, *primary_key))))
1714-
return false;
1715-
1716-
// * The record’s key is less than key.
1717-
if (!Key::less_than(inner_record.key, *key))
1718-
return false;
1700+
// * If the record’s key is equal to key:
1701+
if (Key::equals(inner_record.key, *key)) {
1702+
// * The record’s value is less than or equal to primaryKey
1703+
if (!Key::less_than_or_equal(inner_record.value, *primary_key))
1704+
return false;
1705+
}
1706+
// * Else:
1707+
else {
1708+
// * The record’s key is less than key.
1709+
if (!Key::less_than(inner_record.key, *key))
1710+
return false;
1711+
}
17191712
}
17201713

17211714
// * If position is defined and source is an object store:
@@ -1731,60 +1724,51 @@ GC::Ptr<IDBCursor> iterate_a_cursor(JS::Realm& realm, GC::Ref<IDBCursor> cursor,
17311724
if (position && source.has<GC::Ref<Index>>()) {
17321725
auto const& inner_record = record.get<IndexRecord>();
17331726

1734-
// * The record’s key is equal to position and the record’s value is less than object store position
1735-
if (!(Key::equals(inner_record.key, *position) && Key::less_than(inner_record.value, *object_store_position)))
1736-
return false;
1737-
1738-
// * The record’s key is less than position.
1739-
if (!Key::less_than(inner_record.key, *position))
1740-
return false;
1727+
// * If the record’s key is equal to position:
1728+
if (Key::equals(inner_record.key, *position)) {
1729+
// * The record’s value is less than object store position
1730+
if (!Key::less_than(inner_record.value, *object_store_position))
1731+
return false;
1732+
}
1733+
// Else:
1734+
else {
1735+
// * The record’s key is less than position.
1736+
if (!Key::less_than(inner_record.key, *position))
1737+
return false;
1738+
}
17411739
}
17421740

17431741
// * The record’s key is in range.
1744-
auto is_in_range = record.visit(
1745-
[](Empty) { VERIFY_NOT_REACHED(); },
1746-
[range](auto const& inner_record) {
1742+
return record.visit(
1743+
[&](auto const& inner_record) {
17471744
return range->is_in_range(inner_record.key);
17481745
});
1749-
1750-
return is_in_range;
17511746
};
17521747

17531748
auto prev_unique_requirements = [&](Variant<ObjectStoreRecord, IndexRecord> const& record) -> bool {
17541749
// * If key is defined:
17551750
if (key) {
17561751
// * The record’s key is less than or equal to key.
1757-
auto is_less_than_or_equal = record.visit(
1758-
[](Empty) { VERIFY_NOT_REACHED(); },
1759-
[key](auto const& inner_record) {
1760-
return Key::less_than(inner_record.key, *key) || Key::equals(inner_record.key, *key);
1761-
});
1762-
1763-
if (!is_less_than_or_equal)
1752+
if (!record.visit([&](auto const& inner_record) {
1753+
return Key::less_than_or_equal(inner_record.key, *key);
1754+
}))
17641755
return false;
17651756
}
17661757

17671758
//* If position is defined:
17681759
if (position) {
17691760
// * The record’s key is less than position.
1770-
auto is_less_than_position = record.visit(
1771-
[](Empty) { VERIFY_NOT_REACHED(); },
1772-
[position](auto const& inner_record) {
1773-
return Key::less_than(inner_record.key, *position) || Key::equals(inner_record.key, *position);
1774-
});
1775-
1776-
if (!is_less_than_position)
1761+
if (!record.visit([&](auto const& inner_record) {
1762+
return Key::less_than(inner_record.key, *position);
1763+
}))
17771764
return false;
17781765
}
17791766

17801767
// * The record’s key is in range.
1781-
auto is_in_range = record.visit(
1782-
[](Empty) { VERIFY_NOT_REACHED(); },
1783-
[range](auto const& inner_record) {
1768+
return record.visit(
1769+
[&](auto const& inner_record) {
17841770
return range->is_in_range(inner_record.key);
17851771
});
1786-
1787-
return is_in_range;
17881772
};
17891773

17901774
// 9. While count is greater than 0:

‎Libraries/LibWeb/IndexedDB/Internal/Key.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,8 @@ class Key : public JS::Cell {
6969
[[nodiscard]] static bool equals(GC::Ref<Key> a, GC::Ref<Key> b) { return compare_two_keys(a, b) == 0; }
7070
[[nodiscard]] static bool less_than(GC::Ref<Key> a, GC::Ref<Key> b) { return compare_two_keys(a, b) < 0; }
7171
[[nodiscard]] static bool greater_than(GC::Ref<Key> a, GC::Ref<Key> b) { return compare_two_keys(a, b) > 0; }
72+
[[nodiscard]] static bool less_than_or_equal(GC::Ref<Key> a, GC::Ref<Key> b) { return compare_two_keys(a, b) <= 0; }
73+
[[nodiscard]] static bool greater_than_or_equal(GC::Ref<Key> a, GC::Ref<Key> b) { return compare_two_keys(a, b) >= 0; }
7274

7375
AK::String dump() const;
7476

‎Tests/LibWeb/Text/expected/wpt-import/IndexedDB/idbcursor-advance-invalid.any.txt‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,10 @@ Harness status: OK
22

33
Found 6 tests
44

5-
4 Pass
6-
2 Fail
7-
Fail IDBCursor.advance() - invalid - attempt to call advance twice
5+
6 Pass
6+
Pass IDBCursor.advance() - invalid - attempt to call advance twice
87
Pass IDBCursor.advance() - invalid - pass something other than number
98
Pass IDBCursor.advance() - invalid - pass null/undefined
109
Pass IDBCursor.advance() - invalid - missing argument
1110
Pass IDBCursor.advance() - invalid - pass negative numbers
12-
Fail IDBCursor.advance() - invalid - got value not set on exception
11+
Pass IDBCursor.advance() - invalid - got value not set on exception

‎Tests/LibWeb/Text/expected/wpt-import/IndexedDB/idbcursor-advance.any.txt‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,10 @@ Harness status: OK
22

33
Found 6 tests
44

5-
2 Pass
6-
4 Fail
7-
Fail IDBCursor.advance() - advances
8-
Fail IDBCursor.advance() - advances backwards
5+
6 Pass
6+
Pass IDBCursor.advance() - advances
7+
Pass IDBCursor.advance() - advances backwards
98
Pass IDBCursor.advance() - skip far forward
10-
Fail IDBCursor.advance() - within range
9+
Pass IDBCursor.advance() - within range
1110
Pass IDBCursor.advance() - within single key range
12-
Fail IDBCursor.advance() - within single key range, with several results
11+
Pass IDBCursor.advance() - within single key range, with several results

‎Tests/LibWeb/Text/expected/wpt-import/IndexedDB/idbcursor-continue.any.txt‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,10 @@ Harness status: OK
22

33
Found 6 tests
44

5-
2 Pass
6-
4 Fail
7-
Fail IDBCursor.continue() - continues
8-
Fail IDBCursor.continue() - with given key
5+
6 Pass
6+
Pass IDBCursor.continue() - continues
7+
Pass IDBCursor.continue() - with given key
98
Pass IDBCursor.continue() - skip far forward
10-
Fail IDBCursor.continue() - within range
9+
Pass IDBCursor.continue() - within range
1110
Pass IDBCursor.continue() - within single key range
12-
Fail IDBCursor.continue() - within single key range, with several results
11+
Pass IDBCursor.continue() - within single key range, with several results

‎Tests/LibWeb/Text/expected/wpt-import/IndexedDB/idbcursor-direction-index-keyrange.any.txt‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
Harness status: OK
2+
3+
Found 4 tests
4+
5+
4 Pass
6+
Pass IDBCursor direction - index with keyrange - next
7+
Pass IDBCursor direction - index with keyrange - prev
8+
Pass IDBCursor direction - index with keyrange - nextunique
9+
Pass IDBCursor direction - index with keyrange - prevunique

‎Tests/LibWeb/Text/expected/wpt-import/IndexedDB/idbcursor-request-source.any.txt‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,11 @@ Harness status: OK
22

33
Found 8 tests
44

5-
6 Pass
6-
2 Fail
5+
8 Pass
76
Pass IDBObjectStore::openCursor's request source must be the IDBObjectStore instance that opened the cursor
87
Pass IDBObjectStore::openKeyCursor's request source must be the IDBObjectStore instance that opened the cursor
9-
Fail IDBIndex::openCursor's request source must be the IDBIndex instance that opened the cursor
10-
Fail IDBIndex::openKeyCursor's request source must be the IDBIndex instance that opened the cursor
8+
Pass IDBIndex::openCursor's request source must be the IDBIndex instance that opened the cursor
9+
Pass IDBIndex::openKeyCursor's request source must be the IDBIndex instance that opened the cursor
1110
Pass The source of the request from IDBObjectStore::update() is the cursor itself
1211
Pass The source of the request from IDBObjectStore::delete() is the cursor itself
1312
Pass The source of the request from IDBIndex::update() is the cursor itself

‎Tests/LibWeb/Text/input/wpt-import/IndexedDB/idbcursor-direction-index-keyrange.any.html‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
<!doctype html>
2+
<meta charset=utf-8>
3+
<title>IDBCursor direction - index with keyrange</title>
4+
<script>
5+
self.GLOBAL = {
6+
isWindow: function() { return true; },
7+
isWorker: function() { return false; },
8+
isShadowRealm: function() { return false; },
9+
};
10+
</script>
11+
<script src="../resources/testharness.js"></script>
12+
<script src="../resources/testharnessreport.js"></script>
13+
<script src="resources/support.js"></script>
14+
<div id=log></div>
15+
<script src="../IndexedDB/idbcursor-direction-index-keyrange.any.js"></script>

0 commit comments

Comments
 (0)