This will eventually allow us to stop blocking the UI thread for IO caused by the page zoom check on every onLocationChange.

This works, but it would be better for getPref to pass through the callback to _selectPref/_selectGlobalPref, which would then construct and execute the statement either synchronously or asynchronously depending on whether or not they received a callback. That way there would be less code repetition and better encapsulation of the statement initialization and execution code. You'll need to factor out the nsIContentPrefCallback literal into a constructor function with a prototype so you don't repeat it in both functions, but that should be pretty simple. And the try/finally statements in those functions could complicate things slightly, but they probably don't, since calling reset on an asynchronously-executing statement appears safe.

Comment on attachment 421802 [details] [diff] [review] Patch Oops, forgot to set review status. This is a very welcome improvement that'll get r+ with the changes specified in comment 1!

And I forgot to pop this off as a patch before adding something on top of it! :) What I wanted to do doesn't seem to be valid XPIDLese and blew up once I tried to compile native code depending on it. This new version isn't quite as elegant, but I guess it's a decent compromise.

Comment on attachment 423214 [details] [diff] [review] Patch v2 This looks good, r=myk. One API question, however: now that XPCOM/XPIDL supports optional parameters, i.e. those with the [optional] prefix prepended to their names, like nsIHandlerInfo::launchWithURI <http://mxr.mozilla.org/mozilla-central/source/netwerk/mime/public/nsIMIMEInfo.idl#123>, wouldn't it be better to make |getPref| a polymorphic function that works either synchronously or asynchronously depending on the presence of an optional aCallback parameter?

Erm, d'oh! That's exactly what you did the first time. Bummer that it doesn't work. What's the issue?

Went back and messed with the first version again and found that the compiler error I was getting confusingly made it seem like there were too many arguments when there were really too few. Null to the rescue!

Comment on attachment 423465 [details] [diff] [review] Alternate patch >+ * @param aCallback an optional nsIContentPrefCallback to receive the >+ * results instead of returning It might be worth noting in a comment here that JS callers can pass a JS function in addition to an nsIContentPrefCallback as the callback. >+ let astmt = new AsyncStatement(this._stmtSelectPref); >+ astmt.execute(aCallback); You could simplify these to: new AsyncStatement(this._stmtSelectPref).execute(aCallback); But overall it looks great. I'm psyched you can implement it via polymorphism! r=myk