Bug 1014581 - Vibration API: clamp vibration lengths and array length to match platform restrictions. r=smaug
authorMounir Lamouri <mounir@lamouri.fr>
Tue, 27 May 2014 21:54:03 +0100
changeset 185170 989acfceca7bcd1c0d977757b4b11fa328136870
parent 185169 c9062e64d9c046d455d825ed647025f139b7f220
child 185171 efadd83eaf9153ebb62049bd6bad545b0280685f
push id44020
push usermounir@lamouri.fr
push dateTue, 27 May 2014 21:08:23 +0000
treeherdermozilla-inbound@efadd83eaf91 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1014581
milestone32.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1014581 - Vibration API: clamp vibration lengths and array length to match platform restrictions. r=smaug
dom/base/Navigator.cpp
dom/tests/mochitest/general/test_vibrator.html
--- a/dom/base/Navigator.cpp
+++ b/dom/base/Navigator.cpp
@@ -787,29 +787,31 @@ Navigator::Vibrate(const nsTArray<uint32
     return false;
   }
 
   if (doc->Hidden()) {
     // Hidden documents cannot start or stop a vibration.
     return false;
   }
 
-  if (aPattern.Length() > sMaxVibrateListLen) {
-    return false;
+  nsTArray<uint32_t> pattern(aPattern);
+
+  if (pattern.Length() > sMaxVibrateListLen) {
+    pattern.SetLength(sMaxVibrateMS);
   }
 
-  for (size_t i = 0; i < aPattern.Length(); ++i) {
-    if (aPattern[i] > sMaxVibrateMS) {
-      return false;
+  for (size_t i = 0; i < pattern.Length(); ++i) {
+    if (pattern[i] > sMaxVibrateMS) {
+      pattern[i] = sMaxVibrateMS;
     }
   }
 
   // The spec says we check sVibratorEnabled after we've done the sanity
   // checking on the pattern.
-  if (aPattern.IsEmpty() || !sVibratorEnabled) {
+  if (pattern.IsEmpty() || !sVibratorEnabled) {
     return true;
   }
 
   // Add a listener to cancel the vibration if the document becomes hidden,
   // and remove the old visibility listener, if there was one.
 
   if (!gVibrateWindowListener) {
     // If gVibrateWindowListener is null, this is the first time we've vibrated,
@@ -817,17 +819,17 @@ Navigator::Vibrate(const nsTArray<uint32
     // shutdown.
     ClearOnShutdown(&gVibrateWindowListener);
   }
   else {
     gVibrateWindowListener->RemoveListener();
   }
   gVibrateWindowListener = new VibrateWindowListener(mWindow, doc);
 
-  hal::Vibrate(aPattern, mWindow);
+  hal::Vibrate(pattern, mWindow);
   return true;
 }
 
 //*****************************************************************************
 //  Pointer Events interface
 //*****************************************************************************
 
 uint32_t
--- a/dom/tests/mochitest/general/test_vibrator.html
+++ b/dom/tests/mochitest/general/test_vibrator.html
@@ -18,39 +18,46 @@ function expectFailure(param) {
   is(result, false, 'vibrate(' + param + ') should have failed.');
 }
 
 function expectSuccess(param) {
   result = navigator.vibrate(param);
   is(result, true, 'vibrate(' + param + ') must succeed.');
 }
 
-function testFailures() {
+function tests() {
+  // Some edge cases that the bindings should handle for us.
   expectSuccess(null);
   expectSuccess(undefined);
-  expectFailure(-1);
+  // -1 will be converted to the highest unsigned long then clamped.
+  expectSuccess(-1);
   expectSuccess('a');
-  expectFailure([100, -1]);
+  // -1 will be converted to the highest unsigned long then clamped.
+  expectSuccess([100, -1]);
   expectSuccess([100, 'a']);
 
   var maxVibrateMs = SpecialPowers.getIntPref('dom.vibrator.max_vibrate_ms');
   var maxVibrateListLen = SpecialPowers.getIntPref('dom.vibrator.max_vibrate_list_len');
 
-  // Make sure that these preferences are respected.
-  expectFailure(maxVibrateMs + 1);
-  expectFailure([maxVibrateMs + 1]);
+  // If we pass a vibration pattern with a value higher than max_vibrate_ms or a
+  // pattern longer than max_vibrate_list_len, the call should succeed but the
+  // pattern should be modified to match the restrictions.
+
+  // Values will be clamped to dom.vibrator.max_vibrate_ms.
+  expectSuccess(maxVibrateMs + 1);
+  expectSuccess([maxVibrateMs + 1]);
 
   var arr = [];
   for (var i = 0; i < maxVibrateListLen + 1; i++) {
     arr[i] = 0;
   }
-  expectFailure(arr);
-}
+  // The array will be truncated to have a length equal to dom.vibrator.max_vibrate_list_len.
+  expectSuccess(arr);
 
-function testSuccesses() {
+
   expectSuccess(0);
   expectSuccess([]);
   expectSuccess('1000');
   expectSuccess(1000);
   expectSuccess(1000.1);
   expectSuccess([0, 0, 0]);
   expectSuccess(['1000', 1000]);
   expectSuccess([1000, 1000]);
@@ -63,25 +70,23 @@ function testSuccesses() {
   ok(true, "Didn't crash after issuing a lot of vibrate() calls.");
 }
 
 var origVibratorEnabled = SpecialPowers.getBoolPref('dom.vibrator.enabled');
 
 // Test with the vibrator pref enabled.
 try {
   SpecialPowers.setBoolPref('dom.vibrator.enabled', true);
-  testFailures();
-  testSuccesses();
+  tests();
 
   // Everything should be the same when the vibrator is disabled -- in
   // particular, a disabled vibrator shouldn't eat failures we'd otherwise
   // observe.
   SpecialPowers.setBoolPref('dom.vibrator.enabled', false);
-  testFailures();
-  testSuccesses();
+  tests();
 }
 finally {
   SpecialPowers.setBoolPref('dom.vibrator.enabled', origVibratorEnabled);
 }
 
 </script>
 </body>