Closed Bug 1025395 Opened 5 years ago Closed 4 years ago

mach's webidl-example uses NS_IMPL_CYCLE_COLLECTION_INHERITED_0, which no longer exists

Categories

(Core :: DOM: Core & HTML, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla41
Tracking Status
firefox41 --- fixed

People

(Reporter: qdot, Assigned: bzbarsky)

References

Details

Attachments

(3 files)

mach needs to be updated to use NS_ADDREF_IMPL_INHERITED, then list the attributes that should be collected. This will require figuring out types from the attributes in the webidl interface.
Component: mach → DOM
The example generator doesn't output any data members.

It also doesn't output NS_ADDREF_IMPL_INHERITED_0 anywhere...
I've attached the webidl and the corresponding cpp file put out by running mach webidl-example. There's a NS_ADDREF_IMPL_INHERITED_0 there.
Attachment #8440324 - Attachment mime type: text/x-c++src → text/plain
Attachment #8440323 - Attachment mime type: text/x-csrc → text/plain
> There's a NS_ADDREF_IMPL_INHERITED_0 there.

There is?  Where?

I see NS_IMPL_ADDREF_INHERITED and I see NS_IMPL_CYCLE_COLLECTION_INHERITED_0.  Did you mean the latter?

If so, that never existed, as far as I know.  You're supposed to get a compile error and actually use the right thing there that CCs your members, or remove the CC bits if you have no members to cycle collect...  I'm definitely up for changes to the output to make that clearer.
Wow. Ok. I seem to have caught some sort of macro related dyslexia, sorry about that. They're like, all right next to each other. :| Will update bug.

Anyways, I meant NS_IMPL_CYCLE_COLLECTION_INHERITED_0. To fix it for the moment, I ended up just using NS_IMPL_CYCLE_COLLECTION_CLASS to initially get things to compile, but it sounds like the solution might be to have an error macro in there for when people try to just cut/paste like I did?
Summary: mach's webidl-example uses NS_ADDREF_IMPL_INHERITED_0, which no longer exists → mach's webidl-example uses NS_IMPL_CYCLE_COLLECTION_INHERITED_0, which no longer exists
Yeah, that might make sense.
The code that outputs NS_IMPL_CYCLE_COLLECTION_INHERITED_0 is here
http://mxr.mozilla.org/mozilla-central/source/dom/bindings/Codegen.py#13395

13391         if self.parentIface:
13392             ccImpl = dedent("""
13393 
13394                 // Only needed for refcounted objects.
13395                 NS_IMPL_CYCLE_COLLECTION_INHERITED_0(${nativeType}, ${parentType})
13396                 NS_IMPL_ADDREF_INHERITED(${nativeType}, ${parentType})
13397                 NS_IMPL_RELEASE_INHERITED(${nativeType}, ${parentType})
13398                 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(${nativeType})
13399                 NS_INTERFACE_MAP_END_INHERITING(${parentType})
13400 
13401                 """)

maybe that should be
NS_IMPL_CYCLE_COLLECTION_INHERITED(${nativeType}, ${parentTypes})
with parentTypes being the list of parent types?

Maybe this depends on whether the classes in the list are nsISupports or only the last one?
What does webidl-example output for the members of the list parentTypes?
> maybe that should be

No, it shouldn't.  See comment 5.  Maybe we should put that into an actual comment in the generated code....
How about issuing something like:

#error if the class has refcounted members then add them variables NS_IMPL_CYCLE_COLLECTION_INHERITED(${nativeType}, ${parentType}, mMember1, mMember2) // nsCOMPtr<nsIMember1Class> mMember1; see WebIDL_bindings

instead of the current NS_IMPL_CYCLE_COLLECTION_INHERITED_0 code that yields a cryptic error message."... expected constructor, destructor, or type conversion before ‘MozExternalRefCountType’"

-Axel
Or maybe we should actually have a NS_IMPL_CYCLE_COLLECTION_INHERITED_0?  I see some service worker code working around its lack with an mCCDummy member...
Flags: needinfo?(continuation)
(In reply to Not doing reviews right now from comment #11)
> Or maybe we should actually have a NS_IMPL_CYCLE_COLLECTION_INHERITED_0?  I
> see some service worker code working around its lack with an mCCDummy
> member...

I think that can be eliminated. I have a patch.
Flags: needinfo?(continuation)
(In reply to Andrew McCreight [:mccr8] from comment #12)
> I think that can be eliminated. I have a patch.

I filed bug 1166488.

Anyways, the basic idea is that generally any time you might want to use INHERITED_0 you can instead just not declare any CC or ISUPPORTS stuff and inherit it all from your parent.  I suppose there could be some situation wherein you implement an additional nsIFoo interface, but don't have any additional CCed members, and then maybe you'd need it.  Also, you may want to override AddRef and Release to get better error messages, again without having any additional CCed members.  So I guess there are some use cases.
Assignee: kyle → bzbarsky
Status: NEW → ASSIGNED
Attachment #8607758 - Flags: review?(peterv) → review+
https://hg.mozilla.org/mozilla-central/rev/2579eae7696b
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Duplicate of this bug: 1025392
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.