Closed Bug 794694 Opened 7 years ago Closed 7 years ago
Make sure to trace all the gray GCthings, not only wrapper
Still thinking some more generic approach.
https://tbpl.mozilla.org/?tree=Try&rev=f86066e0da03 (Haven't tested the patch at all yet. FF starts and closes without leaks.)
Comment on attachment 665202 [details] [diff] [review] WIP nsGlobalWindow would might need something similar, but it seems to not trace js stuff during traverse, so it has all different problems. Fixing that all needs a new bug.
Attachment #665202 - Flags: review?(continuation)
Comment on attachment 665202 [details] [diff] [review] WIP Review of attachment 665202 [details] [diff] [review]: ----------------------------------------------------------------- ::: content/base/src/nsXMLHttpRequest.cpp @@ +534,5 @@ > NS_IMPL_CYCLE_COLLECTION_CAN_SKIP_IN_CC_END > > NS_IMPL_CYCLE_COLLECTION_CAN_SKIP_THIS_BEGIN(nsXMLHttpRequest) > + return tmp-> > + IsBlackAndDoesNotNeedTracing(static_cast<nsDOMEventTargetHelper*>(tmp)); see my comment for CanSkipThis in nsDOMEventTargetHelper. ::: content/events/src/nsDOMEventTargetHelper.cpp @@ +57,4 @@ > NS_IMPL_CYCLE_COLLECTION_CAN_SKIP_IN_CC_END > > NS_IMPL_CYCLE_COLLECTION_CAN_SKIP_THIS_BEGIN(nsDOMEventTargetHelper) > + return tmp->IsBlackAndDoesNotNeedTracing(tmp); You don't really need to change this for CanSkipThis. CanSkipInCC is used for roots, and CanSkipThis is used for children (maybe we should change their names now that this is consistent?). We only really need to worry about JS holders when we are adding roots. If we later come across the same object as a child, then we know that if it hasn't been added yet, then it must not have any gray JS children. You could add an assertion to this effect I guess. But I don't know how much it matters, except that you will be making things a little slower. ::: dom/base/nsWrapperCacheInlines.h @@ +24,5 @@ > return o && !xpc_IsGrayGCThing(o); > } > > +static void > +SearchGray(void* aGCThing, const char* aName, void* aClosure) CheckParticipatesInCycleCollection in XPCJSRuntime does almost the same thing as this, but I guess it would be hard to share code here. You should probably check AddToCCKind(js_GetGCThingTraceKind(aGCThing)), though I don't know how much that matters in practice...
Attachment #665202 - Flags: review?(continuation) → review+
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.