1155643 - Consolidate nsIDebug and nsIDebug2 interfaces

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[:Paolo Amadini]

This is a good next bug to get familiar with how XPCOM and XPConnect works.

Until recently, many XPCOM interfaces in the code base were present in different versions, because the set of methods they defined used to be "frozen" to provide binary compatibility.

Now that binary compatibility isn't a requirement anymore, interfaces have been consolidated over time. nsIDebug and nsIDebug2 are a simple example of one interface that still needs to be consolidated. Since many consumers reference nsIDebug2, we should probably keep it and remove nsIDebug, as we've done in other similar cases.

The files involved can be found from this search:

https://dxr.mozilla.org/mozilla-central/search?q=nsIDebug&=mozilla-central&redirect=true

These are the required steps:
* Move the methods of nsIDebug.idl into nsIDebug2.idl
* Remove nsIDebug.idl, and the inheritance reference from nsIDebug2
* Adjust all the build system and registration references
* Adjust the call sites that use nsIDebug to use nsIDebug2

Comment 1

[Ganesh Sahukari]

i would like to work on this bug. Can you assign me the bug

Comment 2

[Ganesh Sahukari]

Attachment: bug-1155643-fix

Removed nsIDebug.idl, moved all it's methods to nsIDebug2.idl. Took care of all dependencies of nsIDebug.

Comment 3

[:Paolo Amadini]

Comment on attachment 8598048 [details] [diff] [review]
bug-1155643-fix

Review of attachment 8598048 [details] [diff] [review]:
-----------------------------------------------------------------

Hi Ganesh, sorry for the unusual delay on this review. This looks great and I've started a tryserver build:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1baa0b32f214

::: xpcom/base/nsDebugImpl.h
@@ +36,5 @@
> +{ /* cb6cdb94-e417-4601-b4a5-f991bf41453d */         \
> +  0xcb6cdb94,                                        \
> +    0xe417,                                          \
> +    0x4601,                                          \
> +    {0xb4, 0xa5, 0xf9, 0x91, 0xbf, 0x41, 0x45, 0x3d} \

As a side note, if I remember correctly, the CID of the component doesn't need to be updated when the interfaces change. This is because it refers to a concrete component that, over time, may change and implement different interfaces. However, changing the CID here doesn't hurt, so let's keep the new one. The IID change is definitely the important one, since the methods available on the interface changed.

::: xpcom/base/nsIDebug2.idl
@@ +48,5 @@
> +     *
> +     */
> +    void assertion(in string aStr,
> +                   in string aExpr,
> +                   in string aFile, 

nit: while you're copying the code, you may also remove the trailing whitespace that was incorrectly present in the original file.

Comment 4

[Ganesh Sahukari]

Attachment: bug-1155643-fix-v2.patch

Removed the trailing whitespace in nsIDdebug2.idl

Comment 5

[:Paolo Amadini]

Comment on attachment 8602510 [details] [diff] [review]
bug-1155643-fix-v2.patch

I didn't enable the unit tests in the previous try run by mistake, so I have to run it again:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2c953fed42bf

There were x64 build failures in the previous one, but they might actually be infrastructure problems so let's see how it goes.

If there are no test failures related to this patch, feel free to set the "checkin-needed" keyword, so that the patch can be checked in even if I'm not around.

Comment 6

[:Paolo Amadini]

Comment on attachment 8598048 [details] [diff] [review]
bug-1155643-fix

You can also make older versions as "obsolete" when uploading a new one, this makes it clear which attachment is the most recent, especially useful when someone else is handling the checkin.

Comment 7

[Ganesh Sahukari]

Try build has been failed for some test cases, but there are bugs raised for those.

Comment 8

[:Paolo Amadini]

Thanks!

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/90e184f46b30

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/90e184f46b30

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/90e184f46b30