This patch uses the new dom bindings to make a live array of nsIDOMTelephonyCall objects (a "platform array object" according to WebIDL). Modifications are not allowed yet it behaves like an array. Win?

Shouldn't item() return null when GetCallAt() returns null? That's what nodelists do...

Comment on attachment 587831 [details] [diff] [review] Patch, v1 Review of attachment 587831 [details] [diff] [review]: ----------------------------------------------------------------- r=me with those things fixed. ::: dom/telephony/Telephony.cpp @@ +117,5 @@ > } > + if (tmp->mCallsArray) { > + nsISupports* callsArray = tmp->mCallsArray->ToISupports(); > + NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NATIVE_PTR(callsArray, TelephonyCallArray, > + "mCallsArray") Please have peterv, or someone that knows cycle collector better than me, look at this. Or simply use NoteXPCOMChild. ::: dom/telephony/TelephonyCallArray.cpp @@ +59,5 @@ > +already_AddRefed<TelephonyCallArray> > +TelephonyCallArray::Create(Telephony* aTelephony) > +{ > + nsRefPtr<TelephonyCallArray> array = new TelephonyCallArray(); > + array->mTelephony = aTelephony; Make the Telephony* a constructor argument? @@ +67,5 @@ > +NS_IMPL_CYCLE_COLLECTION_CLASS(TelephonyCallArray) > + > +NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(TelephonyCallArray) > + NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NATIVE_PTR(tmp->mTelephony->ToISupports(), > + Telephony, "mTelephony") Same thing here for the traverse goop. @@ +93,5 @@ > + NS_ASSERTION(aItem, "Null pointer!"); > + > + nsCOMPtr<nsIDOMTelephonyCall> item = GetCallAt(aIndex); > + if (!item) { > + return NS_ERROR_NOT_AVAILABLE; Just return null for out-of-bounds checks. No exception.

(In reply to Jonas Sicking (:sicking) from comment #3) > > + NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NATIVE_PTR(callsArray, TelephonyCallArray, > > + "mCallsArray") This looks like the wrong traverse given that mCallsArray is an XPCOM object. Can't you just do: NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NSCOMPTR(mCallsArray) ? I don't understand why you need ToISupports.

Comment on attachment 587831 [details] [diff] [review] Patch, v1 The TRAVERSE_NATIVE_PTRs definitely look wrong. TelephonyCallArray needs to traverse its script objects. This should probably depend on a bug to have bindings that ignore the DOM bindings pref disabled. You don't really want to disable TelephonyCallArray completely just because we find a serious bug in the NodeList bindings.

Backout of https://hg.mozilla.org/mozilla-central/rev/1f3643b7de2d merged to m-c from m-i: https://hg.mozilla.org/mozilla-central/rev/324ad5b90d7d https://hg.mozilla.org/mozilla-central/rev/459b3ccb4e47

(In reply to Peter Van der Beken [:peterv] from comment #5) > This should probably depend on a bug to have bindings that ignore the DOM > bindings pref disabled. You don't really want to disable TelephonyCallArray > completely just because we find a serious bug in the NodeList bindings. That was added in bug 732175.

I don't think this is blocking basecamp or Kilimanjaro. This only affects apps which hold on to the calls array returned from telephony.calls. It's easy to work around in apps by simply using "telephony.calls" rather than holding on to the array. Please feel free to renominate if you disagree. For example if it turns out that telephony.calls is really slow (though that would be surprising).

Closing as WONTFIX since FxOS is no more.