It's a very common pattern to define lazy getters for services in component constructors like this: http://mxr.mozilla.org/mozilla-central/source/toolkit/components/places/src/nsPlacesAutoComplete.js#247 sayrer suggested that we make this a method on XPCOMUtils so we share more code. I'm thinking adding two methods looking something like this: defineLazyGetter: function(aObject, aName, aLambda) { aObject.__defineGetter__(aName, function() { delete aObject[aName]; return aObject[aName] = aLambda(); }); }, defineLazyServiceGetter: function(aObject, aName, aContract, aCID) { this.defineLazyGetter(aObject, aName, function() { return Components.classes[aContract].getService(aCID); }); } patch coming in a bit

sounds good to me

Comment on attachment 393034 [details] [diff] [review] v1.0 >diff --git a/js/src/xpconnect/loader/XPCOMUtils.jsm b/js/src/xpconnect/loader/XPCOMUtils.jsm >+/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- pretty sure tab-width should be 8 or some equally huge number so that we notice bad tabs in the file... the coding style says so at least! > var XPCOMUtils = { > /** > * Generate a QueryInterface implementation. The returned function must be > * assigned to the 'QueryInterface' property of a JS object. When invoked on > * that object, it checks if the given iid is listed in the |interfaces| >@@ -223,16 +225,57 @@ var XPCOMUtils = { > > canUnload: function(compMgr) { > return true; > } > }; > }, > > /** >+ * Defines a getter on a specified object that will be created upon first use. >+ * >+ * @param aObject >+ * The object to define the lazy getter on. >+ * @param aName >+ * The name of the getter to define on aObject. >+ * @param aLambda >+ * A function that returns what the getter should return. This will >+ * only ever be called once. >+ */ >+ defineLazyGetter: function XPCU_defineLazyGetter(aObject, aName, aLambda) >+ { >+ aObject.__defineGetter__(aName, function() { >+ delete aObject[aName]; >+ return aObject[aName] = aLambda(); >+ }); >+ }, >+ >+ /** >+ * Defines a getter on a specified object for a service. The service will not >+ * be obtained until first use. >+ * >+ * @param aObject >+ * The object to define the lazy getter on. >+ * @param aName >+ * The name of the getter to define on aObject for the service. >+ * @param aContract >+ * The contract used to obtain the service. >+ * @param aInterfaceName >+ * The name of the interface to query the service to. >+ */ >+ defineLazyServiceGetter: function XPCU_defineLazyServiceGetter(aObject, aName, >+ aContract, >+ aInterfaceName) >+ { >+ this.defineLazyGetter(aObject, aName, function XPCU_serviceLambda() { >+ return Cc[aContract].getService(Ci[aInterfaceName]); >+ }); >+ }, >+ >+ /** > * Convenience access to category manager > */ > get categoryManager() { > return Components.classes["@mozilla.org/categorymanager;1"] > .getService(Ci.nsICategoryManager); > }, > > /** >diff --git a/js/src/xpconnect/tests/unit/test_xpcomutils.js b/js/src/xpconnect/tests/unit/test_xpcomutils.js >--- a/js/src/xpconnect/tests/unit/test_xpcomutils.js >+++ b/js/src/xpconnect/tests/unit/test_xpcomutils.js >@@ -1,23 +1,131 @@ >+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- >+ * vim: sw=4 ts=4 sts=4 et >+ * ***** BEGIN LICENSE BLOCK ***** >+ * Version: MPL 1.1/GPL 2.0/LGPL 2.1 >+ * >+ * The contents of this file are subject to the Mozilla Public License Version >+ * 1.1 (the "License"); you may not use this file except in compliance with >+ * the License. You may obtain a copy of the License at >+ * http://www.mozilla.org/MPL/ >+ * >+ * Software distributed under the License is distributed on an "AS IS" basis, >+ * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License >+ * for the specific language governing rights and limitations under the >+ * License. >+ * >+ * The Original Code is Necko Test Code. >+ * >+ * The Initial Developer of the Original Code is >+ * Mozilla Corporation. >+ * Portions created by the Initial Developer are Copyright (C) 2009 >+ * the Initial Developer. All Rights Reserved. >+ * >+ * Contributor(s): >+ * Boris Zbarsky <bzbarsky@mit.edu> (Original Author) >+ * Shawn Wilsher <me@shawnwilsher.com> >+ * >+ * Alternatively, the contents of this file may be used under the terms of >+ * either the GNU General Public License Version 2 or later (the "GPL"), or >+ * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"), >+ * in which case the provisions of the GPL or the LGPL are applicable instead >+ * of those above. If you wish to allow use of your version of this file only >+ * under the terms of either the GPL or the LGPL, and not to allow others to >+ * use your version of this file under the terms of the MPL, indicate your >+ * decision by deleting the provisions above and replace them with the notice >+ * and other provisions required by the GPL or the LGPL. If you do not delete >+ * the provisions above, a recipient may use your version of this file under >+ * the terms of any one of the MPL, the GPL or the LGPL. >+ * >+ * ***** END LICENSE BLOCK ***** */ >+ >+/** >+ * This file tests the methods on XPCOMUtils.jsm. >+ */ >+ > Components.utils.import("resource://gre/modules/XPCOMUtils.jsm"); > >-var x = { >- QueryInterface: XPCOMUtils.generateQI([Components.interfaces.nsIClassInfo, >- "nsIDOMNode"]) >-}; >+const Cc = Components.classes; >+const Ci = Components.interfaces; > >-function run_test() { >+//////////////////////////////////////////////////////////////////////////////// >+//// Tests >+ >+function test_generateQI_string_names() >+{ >+ var x = { >+ QueryInterface: XPCOMUtils.generateQI([ >+ Components.interfaces.nsIClassInfo, >+ "nsIDOMNode" >+ ]) >+ }; >+ > try { >- x.QueryInterface(Components.interfaces.nsIClassInfo); >+ x.QueryInterface(Components.interfaces.nsIClassInfo); > } catch(e) { >- do_throw("Should QI to nsIClassInfo"); >+ do_throw("Should QI to nsIClassInfo"); > } > try { >- x.QueryInterface(Components.interfaces.nsIDOMNode); >+ x.QueryInterface(Components.interfaces.nsIDOMNode); > } catch(e) { >- do_throw("Should QI to nsIDOMNode"); >+ do_throw("Should QI to nsIDOMNode"); > } > try { >- x.QueryInterface(Components.interfaces.nsIDOMDocument); >- do_throw("QI should not have succeeded!"); >+ x.QueryInterface(Components.interfaces.nsIDOMDocument); >+ do_throw("QI should not have succeeded!"); > } catch(e) {} > } >+ >+function test_defineLazyGetter() >+{ >+ let accessCount = 0; >+ let obj = { }; >+ const TEST_VALUE = "test value"; >+ XPCOMUtils.defineLazyGetter(obj, "foo", function() { >+ accessCount++; >+ return TEST_VALUE; >+ }); >+ do_check_eq(accessCount, 0); >+ >+ // Get the property, making sure the access count has increased. >+ do_check_eq(obj.foo, TEST_VALUE); >+ do_check_eq(accessCount, 1); >+ >+ // Get the property once more, making sure the access count has not >+ // increased. >+ do_check_eq(obj.foo, TEST_VALUE); >+ do_check_eq(accessCount, 1); >+} >+ >+function test_defineLazyServiceGetter() >+{ >+ let obj = { }; >+ XPCOMUtils.defineLazyServiceGetter(obj, "service", >+ "@mozilla.org/consoleservice;1", >+ "nsIConsoleService"); >+ let service = Cc["@mozilla.org/consoleservice;1"]. >+ getService(Ci.nsIConsoleService); >+ >+ // Check that the lazy service getter and the actual service have the same >+ // properties. >+ for (let prop in obj.service) >+ do_check_true(prop in service); >+ for (let prop in service) >+ do_check_true(prop in obj.service); >+} >+ >+//////////////////////////////////////////////////////////////////////////////// >+//// Test Runner >+ >+let tests = [ >+ test_generateQI_string_names, >+ test_defineLazyGetter, >+ test_defineLazyServiceGetter, >+]; >+ >+function run_test() >+{ >+ tests.forEach(function(test) { >+ print("Running next test: " + test.name); >+ test(); >+ }); >+}

Comment on attachment 393034 [details] [diff] [review] v1.0 >+ return aObject[aName] = aLambda(); So, what scope does aLambda get called with? Would aLambda.call(aObject) be more useful?

(In reply to comment #5) > Would aLambda.call(aObject) be more useful? I actually changed it to aLambda.apply in bug 513710 because of the scope issue.

Comment on attachment 393034 [details] [diff] [review] v1.0 since more code is starting using these, i'd like to see them on 1.9.2, otherwise any patch using this should be unbitrotted.

Comment on attachment 393034 [details] [diff] [review] v1.0 If you want this in 1.9.2 Marco, you need to take the change I did in bug 513710 for this to be useful. This would need a new patch up on this bug (and probably a test for that behavior since it looks like I neglected to add one).

There's now documentation for this here; I've also documented the rest of XPCOMUtils.jsm. I'd appreciate it if someone could give it a once-over: https://developer.mozilla.org/en/JavaScript_code_modules/XPCOMUtils.jsm (Just a note, I'm still doing some style cleanup work on this at this time, but the content itself is as done as it's going to be for now). Can someone point me to an example of XPCOMUtils.jsm actually in use that I can look at?

(In reply to comment #9) > Can someone point me to an example of XPCOMUtils.jsm actually in use that I can > look at? What bits are you actually interested in seeing? nsPlacesDBFlush.js is a good example of everything else in this file before this bug landed. If you want examples with the new code, I can provide that as well.

I modified the documentation slightly to indicate how |this| is handled when calling aLambda.

I'd like an example with the new stuff too, yes.

Actually, http://mxr.mozilla.org/mozilla-central/source/toolkit/components/places/src/nsPlacesDBFlush.js has everything.

adds a test for scope change, if r+ i'll attach a rollup patch for 1.9.2 including all needed changes.

Comment on attachment 404343 [details] [diff] [review] scope test v1.0 r=sdwilsh Thanks!

includes all needed changes for 1.9.2

I can see where XPCOMUtils.defineLazyServiceGetter is coming from, but what's the deal with XPCOMUtils.defineLazyGetter? It doesn't seem to have anything to do with XPCOM.

scope test on trunk http://hg.mozilla.org/mozilla-central/rev/1cd24ecc343d

Pushed to mozilla-1.9.2: http://hg.mozilla.org/releases/mozilla-1.9.2/rev/50f79db3c211 The documentation should be changed to reflect the API's availability on 1.9.2.

Doc updated to reflect this being added to 1.9.2.