430305 - nsITimerCallback should be declared as [function]

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Ben Bucksch (:BenB)]

nsITimerCallback is just a callback, as the same says, with only one function.
XPConnect has a nice feature to make usage of these from JavaScript much more comfortable: If you add "function" to the interface definition, the JS caller can just pass a normal JS function object instead of an object of this interface, and XPConnect will wrap it and call the JS function when the first method of the interface (in this case "notify") is called.

Actual result:
 Interface:
[scriptable, uuid(a796816d-7d47-4348-9ab8-c7aeb3216a7d)]
interface nsITimerCallback : nsISupports

 Usage:
function ttRunAsyncTimerCallback(func)
{
    assert(typeof func == "function", "please pass a JS function");
    this._func = func;
}
ttRunAsyncTimerCallback.prototype =
{
    _func : null,
    notify : function(timer)
    {
        this._func();
    },
    QueryInterface : function(iid)
    {
        if (iid.equals(CI.nsITimerCallback) ||
            iid.equals(CI.nsISupports))
            return this;
       throw Components.results.NS_NOINTERFACE;
    }
}

...
    var func = function() {...};
    var timer = Cc["@mozilla.org/timer;1"].createInstance(CI.nsITimer);
    timer.initWithCallback(new ttRunAsyncTimerCallback(func), sec * 1000,
        CI.nsITimer.TYPE_ONE_SHOT);

Expected result:
 Interface:
[function, scriptable, uuid(a796816d-7d47-4348-9ab8-c7aeb3216a7d)]
interface nsITimerCallback : nsISupports

 Usage:
    var func = function() {...};
    var timer = Cc["@mozilla.org/timer;1"].createInstance(CI.nsITimer);
    timer.initWithCallback(func, sec * 1000, CI.nsITimer.TYPE_ONE_SHOT);

Comment 1

[Ben Bucksch (:BenB)]

Attachment: Fix

Comment 2

[Ben Bucksch (:BenB)]

Comment on attachment 317052 [details] [diff] [review]
Fix

Tested, works

Comment 3

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 317052 [details] [diff] [review]
Fix

Put the tests in this bug, pls (make-check style preferred).

Comment 4

[Ben Bucksch (:BenB)]

Attachment: Test, for JS function

Comment 5

[Ben Bucksch (:BenB)]

Attachment: Test, to still be able to pass an IDL object

Comment 6

[Ben Bucksch (:BenB)]

Revving IID is not necessary:
[23:05:19] <bz> weirdal: see logs if they exist?  bsmedberg, timeless, and I all agree that this should be fine

Comment 7

[Ben Bucksch (:BenB)]

Attachment: Fix and tests

All in one patch

Comment 8

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 317208 [details] [diff] [review]
Fix and tests

r+sr+a=shaver for this thoroughly-analyzed (additive) API fix, presuming that it passes the test suites.  (I'd have put both those test cases in the same file, but whatever!)

Comment 9

[Ben Bucksch (:BenB)]

Thanks for the review and approval!
Checked in on trunk.
FIXED.