Convert existing code to use the NS_IMPL_ISUPPORTS_CYCLE_COLLECTION_INHERITED macro

RESOLVED FIXED in Firefox 57

Status

()

Core
XPCOM
RESOLVED FIXED
10 months ago
10 months ago

People

(Reporter: mccr8, Assigned: mccr8)

Tracking

unspecified
mozilla57
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox57 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Assignee)

Description

10 months ago
Lots of places use a lot of boilerplate to define QI and ISupports stuff, but I added some macros in bug 1390660 to make this easier. These NS_IMPL_ISUPPORTS_CYCLE_COLLECTION and NS_IMPL_QUERY_INTERFACE_CYCLE_COLLECTION macros should be used everywhere they can. This will require adding new variants, but that should be easy.
(Assignee)

Updated

10 months ago
Assignee: nobody → continuation
(Assignee)

Comment 1

10 months ago
I didn't convert nsXULElement, because it does something odd with tearoffs.
Summary: Convert existing code to use NS_IMPL_ISUPPORTS_CYCLE_COLLECTION* macros → Convert existing code to use the NS_IMPL_ISUPPORTS_CYCLE_COLLECTION_INHERITED macro
Comment hidden (mozreview-request)

Comment 4

10 months ago
mozreview-review
Comment on attachment 8903680 [details]
Bug 1395636 - Convert code to use NS_IMPL_ISUPPORTS_CYCLE_COLLECTION_INHERITED.

https://reviewboard.mozilla.org/r/175458/#review181326

\o/

::: dom/svg/SVGAElement.cpp:48
(Diff revision 1)
> -                               nsIDOMElement,
> +                                             nsIDOMElement,
> -                               nsIDOMSVGElement,
> +                                             nsIDOMSVGElement,
> -                               Link)
> +                                             Link)
> -NS_INTERFACE_TABLE_TAIL_INHERITING(SVGAElementBase)
>  
>  NS_IMPL_CYCLE_COLLECTION_CLASS(SVGAElement)

Huh, couldn't we just remove the cycle collection stuff here?
Attachment #8903680 - Flags: review?(peterv) → review+
(Assignee)

Comment 5

10 months ago
(In reply to Peter Van der Beken [:peterv] from comment #4)
> Huh, couldn't we just remove the cycle collection stuff here?

I filed bug 1396896 for that.
Comment hidden (mozreview-request)
(Assignee)

Comment 7

10 months ago
I'll rebase this and land it once bug 1396896 merges to central.
Depends on: 1396896
Comment hidden (mozreview-request)

Comment 9

10 months ago
Pushed by amccreight@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9306866e8df0
Convert code to use NS_IMPL_ISUPPORTS_CYCLE_COLLECTION_INHERITED. r=peterv
Backed out for build bustage:

https://hg.mozilla.org/integration/autoland/rev/5857b229d11f90fbc8890e8580eed1bca2bdd8c1

Push with bustage: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=9306866e8df05caa6995169d31111cd4e16c3587&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Build log: https://treeherder.mozilla.org/logviewer.html#?job_id=128977477&repo=autoland

[task 2017-09-06T17:18:00.722747Z] 17:18:00     INFO -  ../../dom/svg/Unified_cpp_dom_svg0.o:(.data.rel.ro._ZTVN7mozilla3dom11SVGAElementE[_ZTVN7mozilla3dom11SVGAElementE]+0x778): undefined reference to `non-virtual thunk to mozilla::dom::SVGAElement::Release()'
[task 2017-09-06T17:18:00.722863Z] 17:18:00     INFO -  collect2: error: ld returned 1 exit status
[task 2017-09-06T17:18:00.723015Z] 17:18:00     INFO -  /builds/worker/workspace/build/src/config/rules.mk:721: recipe for target 'libxul.so' failed
[task 2017-09-06T17:18:00.723201Z] 17:18:00     INFO -  gmake[5]: *** [libxul.so] Error 1
[task 2017-09-06T17:18:00.723337Z] 17:18:00     INFO -  gmake[5]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/toolkit/library'
[task 2017-09-06T17:18:00.723477Z] 17:18:00     INFO -  /builds/worker/workspace/build/src/config/recurse.mk:73: recipe for target 'toolkit/library/target' failed
[task 2017-09-06T17:18:00.723646Z] 17:18:00     INFO -  gmake[4]: *** [toolkit/library/target] Error 2
[task 2017-09-06T17:18:00.723807Z] 17:18:00     INFO -  /builds/worker/workspace/build/src/config/recurse.mk:32: recipe for target 'compile' failed
[task 2017-09-06T17:18:00.723924Z] 17:18:00     INFO -  gmake[3]: *** [compile] Error 2
[task 2017-09-06T17:18:00.724097Z] 17:18:00     INFO -  /builds/worker/workspace/build/src/config/rules.mk:453: recipe for target 'default' failed
[task 2017-09-06T17:18:00.724219Z] 17:18:00     INFO -  gmake[2]: *** [default] Error 2
[task 2017-09-06T17:18:00.724367Z] 17:18:00     INFO -  /builds/worker/workspace/build/src/client.mk:424: recipe for target 'realbuild' failed
[task 2017-09-06T17:18:00.724521Z] 17:18:00     INFO -  gmake[1]: *** [realbuild] Error 2
[task 2017-09-06T17:18:00.724658Z] 17:18:00     INFO -  gmake[1]: Leaving directory '/builds/worker/workspace/build/src'
[task 2017-09-06T17:18:00.724797Z] 17:18:00     INFO -  client.mk:175: recipe for target 'build' failed
[task 2017-09-06T17:18:00.724955Z] 17:18:00     INFO -  gmake: *** [build] Error 2
Flags: needinfo?(continuation)
(Assignee)

Comment 11

10 months ago
Bah, I accidentally deleted two more lines than I meant to when rebasing.
Flags: needinfo?(continuation)
Comment hidden (mozreview-request)

Comment 13

10 months ago
Pushed by amccreight@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/fe6a8ed3e407
Convert code to use NS_IMPL_ISUPPORTS_CYCLE_COLLECTION_INHERITED. r=peterv
https://hg.mozilla.org/mozilla-central/rev/fe6a8ed3e407
Status: NEW → RESOLVED
Last Resolved: 10 months ago
status-firefox57: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.