This is a followup from bug 539095, where bz noted that window.performance.timing and window.performance.navigation (and maybe window.performance?) should use the new DOM bindings.

Could use typedef support here so we don't have to do the manual expansion thing.

This seems to work correctly!

Comment on attachment 640506 [details] [diff] [review] Patch (v1) >+++ b/dom/base/nsPerformance.cpp >+NS_IMPL_CYCLE_COLLECTION_CLASS(nsPerformanceTiming) ... >+NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN(nsPerformanceTiming) >+ NS_IMPL_CYCLE_COLLECTION_TRACE_PRESERVED_WRAPPER >+NS_IMPL_CYCLE_COLLECTION_TRACE_END >+ >+NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(nsPerformanceTiming) >+ NS_IMPL_CYCLE_COLLECTION_UNLINK_PRESERVED_WRAPPER >+ NS_IMPL_CYCLE_COLLECTION_UNLINK_NSCOMPTR(mPerformance) >+NS_IMPL_CYCLE_COLLECTION_UNLINK_END >+ >+NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(nsPerformanceTiming) >+ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_SCRIPT_OBJECTS >+ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NSCOMPTR(mPerformance) >+NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END I believe you can replace all that with: NS_IMPL_CYCLE_COLLECTION_WRAPPERCACHE_1(nsPerformanceTiming, mPerformance); >+NS_IMPL_CYCLE_COLLECTION_CLASS(nsPerformanceNavigation) Similar here. >+NS_IMPL_CYCLE_COLLECTION_CLASS(nsPerformance) And here, though you'll need to add NS_IMPL_CYCLE_COLLECTION_WRAPPERCACHE_3 to nsWrapperCache.h >+++ b/dom/bindings/Bindings.conf >+'Performance': { >+ 'nativeType': 'nsPerformance', >+ 'infallible': [ 'now', 'timing', 'navigation' ] You probably want to add: 'resultNotAddRefed': [ 'timing', 'navigation' ] to this so the binding won't do an extra addref/release on them. I really need to get Peter to review my patch to move the infallibility annotations into the webidl.... ;) >+++ b/dom/webidl/PerformanceTiming.webidl >+ // readonly attribute unsigned long long secureConnectionStart; Hmm. Do we have a bug filed on that? If not, would you mind filing? I wonder whether the XPCOM interfaces for these are used in C++... If not, we can probably nix those too. Probably in a followup. r=me with the above nits. Were there any particular pain points you ran into?

This patch was causing leaks. In order to fix that, I broke the cycle of nsDOMNavigationTiming objects by making nsPerformanceTiming and nsPerformanceNavigation not hold strong refs to it and just ask nsPerformance about it (and thus making those two objects one word smaller), and also participating nsPerformance in the cycle collection of nsGlobalWindow.

(In reply to Boris Zbarsky (:bz) from comment #3) > I really need to get Peter to review my patch to move the infallibility > annotations into the webidl.... ;) That would be nice! What's the bug #? > >+++ b/dom/webidl/PerformanceTiming.webidl > >+ // readonly attribute unsigned long long secureConnectionStart; > > Hmm. Do we have a bug filed on that? If not, would you mind filing? -> bug 772589. > I wonder whether the XPCOM interfaces for these are used in C++... If not, > we can probably nix those too. Probably in a followup. I'll do that when I land this patch in another bug. I don't know what happens if I remove the classinfo stuff for these interfaces and I didn't have enough time to figure it out last night. :-) > r=me with the above nits. Were there any particular pain points you ran > into? No particular pain points besides setting up the cycle collection participation (and that was just as painful as it usually is!). One thing that I would like to see in the documentation is sort of a higher overview on how these tie the js/C++ code, and what happens if your interface has the new bindings in addition to quickstubs for example (I think SetIsDOMBinding() tells the runtime code to ignore the quickstubs -- but are they safe to be removed?). Also, it would be nice for the docs to point to a simple conversion patch. The WebGL and 2d canvas patches were huge. I was hoping for this one to be smaller but that didn't really happen either!

> That would be nice! What's the bug #? Bug 757164. > I think SetIsDOMBinding() tells the runtime code to ignore the quickstubs -- but are they > safe to be removed? Yes, and yes. I'll add that to the docs. I think pointing to this patch for now makes sense, until something smaller comes along. ;)

(In reply to comment #6) > > I think SetIsDOMBinding() tells the runtime code to ignore the quickstubs -- but are they > > safe to be removed? > > Yes, and yes. I'll add that to the docs. So, should we remove the quickstubs entries for the interfaces here and also for things like XHRs?

(In reply to Boris Zbarsky (:bz) from comment #6) > I think pointing to this patch for now makes sense, until something smaller > comes along. ;) Done.

> So, should we remove the quickstubs entries for the interfaces here and also for things > like XHRs? Here, yes. For XHR, we should do that, and also remove the "prefable: True" bit we have for it right now. And look into whether there are any C++ consumers, for that matter. I think khuey had some patches along those lines, in fact.

Removed the quickstub entries.

Comment on attachment 640899 [details] [diff] [review] Patch (v3) r=me

Landed with the infallibility annotations moved to the WebIDL files. https://hg.mozilla.org/integration/mozilla-inbound/rev/dd08c10193c6

Filed bug 774556 for removing nsIDOMWindowPerformance* interfaces.

Added a comment mentioning bug 772589 (which I forgot to do when landing): https://hg.mozilla.org/integration/mozilla-inbound/rev/fb282ed64bf7