Bug 884935 - Align navigator.vibrate to spec. r=smaug

dom/base/Navigator.cpp

@@ -709,49 +709,45 @@ Navigator::RemoveIdleObserver(MozIdleObserver& aIdleObserver, ErrorResult& aRv)
   }
 }
 
-void
-Navigator::Vibrate(uint32_t aDuration, ErrorResult& aRv)
+bool
+Navigator::Vibrate(uint32_t aDuration)
 {
   nsAutoTArray<uint32_t, 1> pattern;
   pattern.AppendElement(aDuration);
-  Vibrate(pattern, aRv);
+  return Vibrate(pattern);
 }
 
-void
-Navigator::Vibrate(const nsTArray<uint32_t>& aPattern, ErrorResult& aRv)
+bool
+Navigator::Vibrate(const nsTArray<uint32_t>& aPattern)
 {
   if (!mWindow) {
-    aRv.Throw(NS_ERROR_UNEXPECTED);
-    return;
+    return false;
   }
+
   nsCOMPtr<nsIDocument> doc = mWindow->GetExtantDoc();
   if (!doc) {
-    aRv.Throw(NS_ERROR_FAILURE);
-    return;
+    return false;
   }
+
   if (doc->Hidden()) {
     // Hidden documents cannot start or stop a vibration.
-    return;
+    return false;
   }
 
   if (aPattern.Length() > sMaxVibrateListLen) {
-    // XXXbz this should be returning false instead
-    aRv.Throw(NS_ERROR_DOM_NOT_SUPPORTED_ERR);
-    return;
+    return false;
   }
 
   for (size_t i = 0; i < aPattern.Length(); ++i) {
     if (aPattern[i] > sMaxVibrateMS) {
-      // XXXbz this should be returning false instead
-      aRv.Throw(NS_ERROR_DOM_NOT_SUPPORTED_ERR);
-      return;
+      return false;
     }
   }
 
   // The spec says we check sVibratorEnabled after we've done the sanity
   // checking on the pattern.
-  if (!sVibratorEnabled) {
-    return;
+  if (aPattern.IsEmpty() || !sVibratorEnabled) {
+    return true;
   }
 
   // Add a listener to cancel the vibration if the document becomes hidden,
@@ -769,6 +765,7 @@ Navigator::Vibrate(const nsTArray<uint32_t>& aPattern, ErrorResult& aRv)
   gVibrateWindowListener = new VibrateWindowListener(mWindow, doc);
 
   hal::Vibrate(aPattern, mWindow);
+  return true;
 }
 
 //*****************************************************************************

dom/base/Navigator.h

@@ -175,8 +175,8 @@ public:
   // The XPCOM GetDoNotTrack is ok
   Geolocation* GetGeolocation(ErrorResult& aRv);
   battery::BatteryManager* GetBattery(ErrorResult& aRv);
-  void Vibrate(uint32_t aDuration, ErrorResult& aRv);
-  void Vibrate(const nsTArray<uint32_t>& aDuration, ErrorResult& aRv);
+  bool Vibrate(uint32_t aDuration);
+  bool Vibrate(const nsTArray<uint32_t>& aDuration);
   void GetAppCodeName(nsString& aAppCodeName, ErrorResult& aRv)
   {
     aRv = GetAppCodeName(aAppCodeName);

dom/tests/mochitest/general/test_vibrator.html

@@ -12,26 +12,15 @@
      navigator.vibrate throws an exception where appropriate. -->
 
 <script class="testbody" type="text/javascript;version=1.7">
+var result;
 function expectFailure(param) {
-  try {
-    navigator.vibrate(param);
-  }
-  catch(e) {
-    ok(true, 'vibrate(' + param + ') threw an expected exception.');
-    return;
-  }
-  ok(false, 'vibrate(' + param + ') should have thrown an exception.');
+  result = navigator.vibrate(param);
+  is(result, false, 'vibrate(' + param + ') should have failed.');
 }
 
 function expectSuccess(param) {
-  try {
-    navigator.vibrate(param);
-  }
-  catch(e) {
-    ok(false, 'vibrate(' + param + ') threw an unexpected exception.');
-    return;
-  }
-  ok(true, 'vibrate(' + param + ') did not throw an exception.');
+  result = navigator.vibrate(param);
+  is(result, true, 'vibrate(' + param + ') must succeed.');
 }
 
 function testFailures() {

dom/webidl/Navigator.webidl

@@ -112,12 +112,8 @@ Navigator implements NavigatorBattery;
 partial interface Navigator {
     // We don't support sequences in unions yet
     //boolean vibrate ((unsigned long or sequence<unsigned long>) pattern);
-    // XXXbz also, per spec we should be returning a boolean, and we just don't.
-    // See bug 884935.
-    [Throws]
-    void vibrate(unsigned long duration);
-    [Throws]
-    void vibrate(sequence<unsigned long> pattern);
+    boolean vibrate(unsigned long duration);
+    boolean vibrate(sequence<unsigned long> pattern);
 };
 
 // Mozilla-specific extensions