Last Comment Bug 757599 - Settings API: add test for closed lock
: Settings API: add test for closed lock
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM: Device Interfaces (show other bugs)
: unspecified
: x86 Mac OS X
: -- normal (vote)
: mozilla15
Assigned To: Gregor Wagner [:gwagner]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-05-22 13:43 PDT by Gregor Wagner [:gwagner]
Modified: 2012-05-23 04:51 PDT (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (3.19 KB, patch)
2012-05-22 13:43 PDT, Gregor Wagner [:gwagner]
jonas: review+
Details | Diff | Review

Description Gregor Wagner [:gwagner] 2012-05-22 13:43:02 PDT
Created attachment 626163 [details] [diff] [review]
patch
Comment 1 Jonas Sicking (:sicking) PTO Until July 5th 2012-05-22 16:17:34 PDT
Comment on attachment 626163 [details] [diff] [review]
patch

Review of attachment 626163 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/settings/SettingsManager.js
@@ +122,2 @@
>        throw Components.results.NS_ERROR_ABORT;
> +      return;

The return line won't get executed. The 'throw' call will end execution of the function. So just remove the return statement.

Same in all other changes in this file.

::: dom/settings/tests/test_settings_basics.html
@@ +481,5 @@
> +    var lockx = mozSettings.getLock();
> +    var cb = function() {
> +      var reqx = {};
> +      try {
> +        reqx = lockx.set(wifiNetworks0);

Add a ok(false, "should have thrown") test here

@@ +487,5 @@
> +        ok(true, "Caught Exception");
> +        next();
> +      }
> +      reqx.onsuccess = function () {
> +        ok(false, "Shouldn't get here");

instead of doing this, set reqx to null in the beginning, and check that it's still null inside the catch-block before calling next().

@@ +489,5 @@
> +      }
> +      reqx.onsuccess = function () {
> +        ok(false, "Shouldn't get here");
> +      };
> +      reqx.onerror = onFailure;

And remove this line.

@@ +491,5 @@
> +        ok(false, "Shouldn't get here");
> +      };
> +      reqx.onerror = onFailure;
> +    }
> +    window.setTimeout(cb, 1);

Use SimpleTest.executeSoon()
Comment 3 Ed Morley [:emorley] 2012-05-23 04:51:34 PDT
https://hg.mozilla.org/mozilla-central/rev/99878dcae029

Note You need to log in before you can comment on or make changes to this bug.