111088 - Unnecessary NS_IMPL_ISUPPORTS1 in nsDeviceContextGTK

Status: RESOLVED FIXED

(Core Graveyard :: GFX, defect, P3)

Description

[rbs]

nsDeviceContextGTK is derived from DeviceContextImpl. However its implementation
does NS_IMPL_ISUPPORTS1(nsDeviceContextGTK, nsIDeviceContext), whereas 
DeviceContextImpl supports nsIDeviceContext and nsIObserver. It might be cleaner 
to remove the cruff of ISUPPORTS1/NS_INIT_REFCNT() from GfxGTK/GfxBeOS/etc since 
the base class already provides all this for free. GfxWin doesn't bother 
re-implementing this.

Comment 1

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

I just checked in the fix for this since your changes to turn the observers back
on caused all the builds to go orange since this wasn't in yet.

Comment 2

[rbs]

Thanks for stepping in at the right moment.
Didn't you want to remove NS_INIT_REFCNT() too and simply call the constructor
of the base class?

Comment 3

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Reopening to remove the NS_INIT_REFCNT, and note the base class constructor
explicitly (although it's still called either way).

Comment 4

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Attachment: patch, along with a bunch of other cleanup (mostly in xlib)

Comment 5

[rbs]

Comment on attachment 62518 [details] [diff] [review]
patch, along with a bunch of other cleanup (mostly in xlib)

r=rbs

Comment 6

[jag (Peter Annema)]

Comment on attachment 62518 [details] [diff] [review]
patch, along with a bunch of other cleanup (mostly in xlib)

>Index: widget/src/xlib/nsCheckButton.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/widget/src/xlib/nsCheckButton.cpp,v
>retrieving revision 1.4
>diff -u -d -r1.4 nsCheckButton.cpp
>--- widget/src/xlib/nsCheckButton.cpp	28 Sep 2001 20:12:05 -0000	1.4
>+++ widget/src/xlib/nsCheckButton.cpp	21 Dec 2001 14:57:29 -0000
>@@ -37,33 +37,16 @@
> 
> #include "nsCheckButton.h"
> 
>-NS_IMPL_ADDREF(nsCheckButton)
>-NS_IMPL_RELEASE(nsCheckButton)
>-
> nsCheckButton::nsCheckButton() : nsWidget() , nsICheckButton(),

You can remove the |nsICheckButton()| here.

sr=jag with that nit fixed.

Comment 7

[Roland Mainz]

Comment on attachment 62518 [details] [diff] [review]
patch, along with a bunch of other cleanup (mostly in xlib)

the change to gfx/src/xlib/nsDeviceContextSpecXlib.cpp screws-up printing - I
am getting the famours "Printing failed for some reason"-dialog.
Please don't do that... :)

The remaining parts of the patch look OK and seems to work.

Comment 8

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Oops.  That's just because I got the #ifdef backwards.

Comment 9

[Roland Mainz]

Comment on attachment 62518 [details] [diff] [review]
patch, along with a bunch of other cleanup (mostly in xlib)

Changing that part to
-- snip --
#ifdef USE_XPRINT
NS_IMPL_ISUPPORTS3(nsDeviceContextSpecXlib,
		   nsIDeviceContextSpec,
		   nsIDeviceContextSpecPS,
		   nsIDeviceContextSpecXp)

#else
NS_IMPL_ISUPPORTS2(nsDeviceContextSpecXlib,
		   nsIDeviceContextSpec,
		   nsIDeviceContextSpecPS)
#endif /* USE_XPRINT */
-- snip --
and it works (note the case - it's 'nsIDeviceContextSpecXp', not
'nsIDeviceContextSpecXP')... :-)

COuld you please kill |NS_IMETHODIMP nsDeviceContextSpecGTK ::
QueryInterface(REFNSIID aIID, void** aInstancePtr)|, too ?

Comment 10

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Patch checked in 2002-01-06 10:37 PDT.  I'll attach another patch for
nsDeviceContextSpecGTK.

Comment 11

[Roland Mainz]

Attachment: GTK+ gfx version of Xlib patch

Comment 12

[Simon Fraser [no longer active]]

Comment on attachment 64598 [details] [diff] [review]
GTK+ gfx version of Xlib patch

sr=sfraser

Comment 13

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Attachment 64598 [details] [diff] checked in, r=dbaron, 2002-01-11 19:14 PST.  Marking fixed.