Bug 1281377 - In IDBKeyRange.includes(), do not compare the given value with the unset boundary. r=khuey, a=gchang
authorBevis Tseng <btseng@mozilla.com>
Wed, 22 Jun 2016 13:40:12 +0800
changeset 340068 d1475df3d7295b385f8b7d2c2b8aa458e9982661
parent 340067 90964ed0b53027ef9ee5f07452130f45ea7d7e21
child 340069 40828084d17fecf280f6b1e1caba38be40a369ed
push id6249
push userjlund@mozilla.com
push dateMon, 01 Aug 2016 13:59:36 +0000
treeherdermozilla-beta@bad9d4f5bf7e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskhuey, gchang
bugs1281377
milestone49.0a2
Bug 1281377 - In IDBKeyRange.includes(), do not compare the given value with the unset boundary. r=khuey, a=gchang
dom/indexedDB/IDBKeyRange.cpp
testing/web-platform/tests/IndexedDB/idbkeyrange-includes.htm
--- a/dom/indexedDB/IDBKeyRange.cpp
+++ b/dom/indexedDB/IDBKeyRange.cpp
@@ -17,26 +17,25 @@ namespace dom {
 
 using namespace mozilla::dom::indexedDB;
 
 namespace {
 
 nsresult
 GetKeyFromJSVal(JSContext* aCx,
                 JS::Handle<JS::Value> aVal,
-                Key& aKey,
-                bool aAllowUnset = false)
+                Key& aKey)
 {
   nsresult rv = aKey.SetFromJSVal(aCx, aVal);
   if (NS_FAILED(rv)) {
     MOZ_ASSERT(NS_ERROR_GET_MODULE(rv) == NS_ERROR_MODULE_DOM_INDEXEDDB);
     return rv;
   }
 
-  if (aKey.IsUnset() && !aAllowUnset) {
+  if (aKey.IsUnset()) {
     return NS_ERROR_DOM_INDEXEDDB_DATA_ERR;
   }
 
   return NS_OK;
 }
 
 } // namespace
 
@@ -342,49 +341,48 @@ IDBKeyRange::Includes(JSContext* aCx,
                       ErrorResult& aRv) const
 {
   Key key;
   aRv = GetKeyFromJSVal(aCx, aValue, key);
   if (aRv.Failed()) {
     return false;
   }
 
-  switch (Key::CompareKeys(Lower(), key)) {
-  case 1:
-    return false;
-  case 0:
-    // Identical keys.
-    if (LowerOpen()) {
+  MOZ_ASSERT(!(Lower().IsUnset() && Upper().IsUnset()));
+  MOZ_ASSERT_IF(IsOnly(),
+    !Lower().IsUnset() && !LowerOpen() &&
+    Lower() == Upper() && LowerOpen() == UpperOpen());
+
+  if (!Lower().IsUnset()) {
+    switch (Key::CompareKeys(Lower(), key)) {
+    case 1:
       return false;
+    case 0:
+      // Identical keys.
+      return !LowerOpen();
+    case -1:
+      if (IsOnly()) {
+        return false;
+      }
+      break;
+    default:
+      MOZ_CRASH();
     }
-    break;
-  case -1:
-    if (IsOnly()) {
-      return false;
-    }
-    break;
-  default:
-    MOZ_CRASH();
   }
 
-  if (!IsOnly()) {
+  if (!Upper().IsUnset()) {
     switch (Key::CompareKeys(key, Upper())) {
     case 1:
       return false;
     case 0:
       // Identical keys.
-      if (UpperOpen()) {
-        return false;
-      }
-      break;
+      return !UpperOpen();
     case -1:
       break;
     }
-  } else {
-    MOZ_ASSERT(key == Lower());
   }
 
   return true;
 }
 
 // static
 already_AddRefed<IDBKeyRange>
 IDBKeyRange::Only(const GlobalObject& aGlobal,
--- a/testing/web-platform/tests/IndexedDB/idbkeyrange-includes.htm
+++ b/testing/web-platform/tests/IndexedDB/idbkeyrange-includes.htm
@@ -1,33 +1,61 @@
 <!doctype html>
 <meta charset=utf-8>
 <title></title>
 <script src=/resources/testharness.js></script>
 <script src=/resources/testharnessreport.js></script>
 <script>
 
-  test( function() {
+  test(function() {
     var closedRange = IDBKeyRange.bound(5, 20);
     assert_true(!!closedRange.includes, "IDBKeyRange has a .includes");
     assert_true(closedRange.includes(7), "in range");
     assert_false(closedRange.includes(1), "below range");
     assert_false(closedRange.includes(42), "above range");
     assert_true(closedRange.includes(5) && closedRange.includes(20),
                  "boundary points");
     assert_throws("DataError", function() { closedRange.includes({}) },
                   "invalid key");
   }, "IDBKeyRange.includes() with a closed range");
 
-  test( function() {
+  test(function() {
     var openRange = IDBKeyRange.bound(5, 20, true, true);
     assert_false(openRange.includes(5) || openRange.includes(20),
                  "boundary points");
   }, "IDBKeyRange.includes() with an open range");
 
-  test( function() {
+  test(function() {
     var range = IDBKeyRange.only(42);
     assert_true(range.includes(42), "in range");
     assert_false(range.includes(1), "below range");
     assert_false(range.includes(9000), "above range");
   }, "IDBKeyRange.includes() with an only range");
 
+  test(function() {
+    var range = IDBKeyRange.lowerBound(5);
+    assert_false(range.includes(4), 'value before closed lower bound');
+    assert_true(range.includes(5), 'value at closed lower bound');
+    assert_true(range.includes(6), 'value after closed lower bound');
+  }, "IDBKeyRange.includes() with an closed lower-bounded range");
+
+  test(function() {
+    var range = IDBKeyRange.lowerBound(5, true);
+    assert_false(range.includes(4), 'value before open lower bound');
+    assert_false(range.includes(5), 'value at open lower bound');
+    assert_true(range.includes(6), 'value after open lower bound');
+  }, "IDBKeyRange.includes() with an open lower-bounded range");
+
+  test(function() {
+    var range = IDBKeyRange.upperBound(5);
+    assert_true(range.includes(4), 'value before closed upper bound');
+    assert_true(range.includes(5), 'value at closed upper bound');
+    assert_false(range.includes(6), 'value after closed upper bound');
+  }, "IDBKeyRange.includes() with an closed upper-bounded range");
+
+  test(function() {
+    var range = IDBKeyRange.upperBound(5, true);
+    assert_true(range.includes(4), 'value before open upper bound');
+    assert_false(range.includes(5), 'value at open upper bound');
+    assert_false(range.includes(6), 'value after open upper bound');
+  }, "IDBKeyRange.includes() with an open upper-bounded range");
+
 </script>