1794811 - Use the generated nsISupports.h

Status: RESOLVED FIXED

(Core :: XPCOM, task, P3)

Description

[Andrew McCreight (out of office until 8/21) [:mccr8]]

nsISupports.idl disables the generated code by using #ifdef 0 inside of a %{C++ block. Instead, the actual definition of nsISupports in C++ is in nsISupportsBase.h, which nsISupports.idl includes. This is a bit confusing, so it would be nice to avoid.

The only value I can see in this arrangement is that it makes sure that the return type of AddRef and Release are MozExternalRefCountType instead of the XPIDL type nsrefcnt, which is uint32_t in C++. Bug 1792920 deals with that issue, so once that is fixed, it is straightforward to make us use the generated nsISupports.h and get rid of nsSupportsBase.h.

There's a comment in nsISupports.idl that says "This is all just to deal with the Mac specific ": public __comobject" thing.", but it looks like this was already obsolete in 2001 (see bug 98281 comment 1 where it was removed).

Comment 1

[Andrew McCreight (out of office until 8/21) [:mccr8]]

Attachment: Bug 1794811, part 1 - Include nsISupports.h instead of nsISupportsBase.h.

nsISupports.h includes nsISupportsBase.h, so it should be equivalent.

In the next patch, I'm changing things so that nsISupports is defined in
nsISupports.h instead of nsISupportsBase.h, and deleting the latter, so
this change will be needed anyways. I'm guessing people were using IWYU
or something like that.

Comment 2

[Andrew McCreight (out of office until 8/21) [:mccr8]]

Attachment: Bug 1794811, part 2 - Use the automatically generated nsISupports.h.

Now that the return type of AddRef and Release are consistent between
C++ and XPIDL, we can use the generated nsISupports.h instead of skipping
the guts of it with an #if 0 and importing nsISupportsBase.h.

nsISupportsBase.h had two #includes, nscore.h and nsID.h. nsISupports.h
needs to continue to include both of these files so that things don't
break. It gets nscore.h via nsrootidl.idl, but we need to add an
explicit #include for nsID.h. This needs to be done before the
definition of nsISupports, because the generated code uses some
macros from nsID.h.

Mostly this patch moves over comments from nsISupportsBase.h.

The "Mac specific ": public __comobject" thing" comment seems to have
been obsolete since at least 2001.

Comment 3

[Andrew McCreight (out of office until 8/21) [:mccr8]]

Attachment: nsISupports-before.h

This is the current state of nsISupports.h, with nsISupportsBase.h inlined (without the include guards), all of the comments, the #if 0 code, and a bit of white space removed.

Comment 4

[Andrew McCreight (out of office until 8/21) [:mccr8]]

Attachment: nsISupports-after.h

This is the automatically generated nsISupports.h, with my patches for bug 1792920 and from this bug applied. I deleted most of the comments and cleaned up a tiny bit of whitespace.

The differences between the two look to be:

• There are some new macros: NS_DECL_NSISUPPORTS, NS_ISUPPORTS_IID_STR, NS_DECL_NON_VIRTUAL_NSISUPPORTS, NS_FORWARD_NSISUPPORTS, NS_FORWARD_SAFE_NSISUPPORTS.
• QueryInterface now has a JS_HAZ_CAN_RUN_SCRIPT annotation.
• nsISupports now has this additional declaration: using ScriptableInterfaceType = nsISupports;
• They include the same set of files, although nscore.h is now included indirectly from nsrootidl.h.
• Some include guards for nscore.h and nsID.h are gone, though I assume with modern compilers that is not needed.

Comment 6

[Andrew McCreight (out of office until 8/21) [:mccr8]]

There's a bit of orange on my Linux+Windows debug+opt but nothing looks perma so hopefully it is okay. I'm going to land this plus bug 1792920 when I've rebased it and rebuilt it, and made sure that the browser still starts.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/519f65b19a0e
part 1 - Include nsISupports.h instead of nsISupportsBase.h. r=necko-reviewers,nika,valentin
https://hg.mozilla.org/integration/autoland/rev/9b91d0a00b6e
part 2 - Use the automatically generated nsISupports.h. r=nika

Comment 8

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/519f65b19a0e
https://hg.mozilla.org/mozilla-central/rev/9b91d0a00b6e