265336 - GetService klookandfeel instead of CreateInstance

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[timeless]

People aren't consistent, it has no state and should be used as a service. this
triggers an assertion of the form:

###!!! ASSERTION: You are calling CreateInstance
"{a61e6398-2057-40fd-9c81-873b908d24e7}" when a service for this CID already
exists!: 'Error', file r:/mozilla/xpcom/components/nsComponentManager.cpp, line 1880

Comment 1

[timeless]

Attachment: changes

Comment 2

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 162762 [details] [diff] [review]
changes

+++ mozilla/layout/xul/base/src/nsSplitterFrame.cpp
+//static NS_DEFINE_IID(3ndFeelCID,  NS_LOOKANDFEEL_CID);

this looks wrong :-)

+// make it real time drag for now due to problems

can you indent this correctly while you're here?

I doubt that the scoping of the lookandfeel pointer is needed...

diff -w would've been nice

mozilla/widget/src/photon/nsWidget.cpp

CallGetService(kLookAndFeelCID, &sLookAndFeel); ?

Comment 3

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 162762 [details] [diff] [review]
changes

>Index: mozilla/layout/xul/base/src/nsSplitterFrame.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/mozilla/layout/xul/base/src/nsSplitterFrame.cpp,v
>retrieving revision 1.122
>diff -up -r1.122 mozilla/layout/xul/base/src/nsSplitterFrame.cpp
>--- mozilla/layout/xul/base/src/nsSplitterFrame.cpp
>+++ mozilla/layout/xul/base/src/nsSplitterFrame.cpp
>@@ -79,7 +79,7 @@
> 
> const PRInt32 kMaxZ = 0x7fffffff; //XXX: Shouldn't there be a define somewhere for MaxInt for PRInt32
> // was used in nsSplitterFrame::Init but now commented out
>-//static NS_DEFINE_IID(kLookAndFeelCID,  NS_LOOKANDFEEL_CID);
>+//static NS_DEFINE_IID(3ndFeelCID,  NS_LOOKANDFEEL_CID);
> PRInt32 realTimeDrag;
> 
> class nsSplitterInfo {
>@@ -328,14 +328,18 @@ nsSplitterFrame::Init(nsPresContext*  aP
>   mInner->mChildInfosBefore = nsnull;
>   mInner->mState = nsSplitterFrameInner::Open;
>   mInner->mDragging = PR_FALSE;
>-/* make it real time drag for now due to problems
>-  nsILookAndFeel* lookAndFeel;
>-  if (NS_SUCCEEDED(nsComponentManager::CreateInstance(kLookAndFeelCID, nsnull, NS_GET_IID(nsILookAndFeel), (void**)&lookAndFeel))) {
>-    lookAndFeel->GetMetric(nsILookAndFeel::eMetric_DragFullWindow, realTimeDrag);
>-    NS_RELEASE(lookAndFeel);
>+
>+  {
>+#if 0
>+// make it real time drag for now due to problems
>+    nsCOMPtr<nsILookAndFeel> lookAndFeel = do_GetService(kLookAndFeelCID);
>+    if (lookAndFeel) {
>+      lookAndFeel->GetMetric(nsILookAndFeel::eMetric_DragFullWindow, realTimeDrag);
>+    }
>+    else
>+#endif
>+      realTimeDrag = 1;
>   }
>-  else */
>-    realTimeDrag = 1;
> 
>   // determine orientation of parent, and if vertical, set orient to vertical
>   // on splitter content, then re-resolve style

So why are you changing the semantics here with the #if 0?

>Index: mozilla/widget/src/photon/nsWidget.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/mozilla/widget/src/photon/nsWidget.cpp,v
>retrieving revision 1.116
>diff -up -r1.116 mozilla/widget/src/photon/nsWidget.cpp
>--- mozilla/widget/src/photon/nsWidget.cpp
>+++ mozilla/widget/src/photon/nsWidget.cpp
>@@ -92,12 +92,9 @@ nsWidget*						nsWidget::sFocusWidget = 
> nsWidget::nsWidget()
> {
>   if (!sLookAndFeel) {
>-    if (NS_OK != nsComponentManager::CreateInstance(kLookAndFeelCID,
>-                                                    nsnull,
>-                                                    NS_GET_IID(nsILookAndFeel),
>-                                                    (void**)&sLookAndFeel))
>-		sLookAndFeel = nsnull;
>-  	}
>+    nsCOMPtr<nsILookAndFeel> lookAndFeel = do_GetService(kLookAndFeelCID);
>+    lookAndFeel.swap(sLookAndFeel);
>+  }
> 
>   if( sLookAndFeel )
>     sLookAndFeel->GetColor( nsILookAndFeel::eColor_WindowBackground, mBackground );

Why does it make sense to use an nsCOMPtr here at all?

Comment 4

[timeless]

Attachment: per biesi

Comment 5

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 163635 [details] [diff] [review]
per biesi

sr=dmose

Comment 6

[timeless]

mozilla/layout/html/document/src/nsFrameSetFrame.cpp 	3.153
mozilla/widget/src/photon/nsWidget.cpp 	1.117
mozilla/widget/src/beos/nsTextWidget.cpp 	1.16
mozilla/widget/src/beos/nsButton.cpp 	1.13
mozilla/webshell/tests/viewer/windows/nsTextWidget.cpp 	1.15
mozilla/webshell/tests/viewer/windows/nsButton.cpp 	1.12
mozilla/webshell/tests/viewer/os2/nsTextWidget.cpp 	1.8
mozilla/webshell/tests/viewer/os2/nsButton.cpp 	1.6
mozilla/webshell/tests/viewer/nsViewerApp.cpp 	1.200
mozilla/webshell/tests/viewer/nsBrowserWindow.cpp 	3.438
mozilla/layout/xul/base/src/nsSplitterFrame.cpp 	1.123
mozilla/layout/xul/base/src/nsMenuFrame.cpp 	1.279