207469 - optimize timer usage in nsComposerCommandsUpdater::PrimeUpdateTimer()

Status: VERIFIED FIXED

(SeaMonkey :: Composer, defect)

Description

[leon.zhang]

In nsComposerCommandsUpdater::PrimeUpdateTimer(), timer always be deleted and
recreated each time, to enhance perf, we can reInit it and not create each time.
 btw, add/remove timer in sol8 is time-consumed.

Comment 1

[leon.zhang]

take it

Comment 2

[leon.zhang]

Attachment: proposed fix

Comment 3

[leon.zhang]

Attachment: proposed fix(good format)

Comment 4

[Simon Fraser [no longer active]]

Comment on attachment 124429 [details] [diff] [review]
proposed fix(good format) 

>Index: editor/composer/src/nsComposerCommandsUpdater.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/editor/composer/src/nsComposerCommandsUpdater.cpp,v
>retrieving revision 1.12
>diff -u -r1.12 nsComposerCommandsUpdater.cpp
>--- editor/composer/src/nsComposerCommandsUpdater.cpp	6 Feb 2003 05:34:47 -0000	1.12
>+++ editor/composer/src/nsComposerCommandsUpdater.cpp	29 May 2003 10:17:22 -0000
>@@ -40,7 +40,8 @@
> #include "nsITransactionManager.h"
> 
> nsComposerCommandsUpdater::nsComposerCommandsUpdater()
>-:  mDOMWindow(nsnull)
>+:  mUpdateTimer(nsnull)

mUpdateTimer is an nsCOMPtr member variable, which is automatically initted to
null, so you don't have to init it here.

> nsComposerCommandsUpdater::PrimeUpdateTimer()
> {
>-  nsresult rv = NS_OK;
>-    
>-  if (mUpdateTimer)
>+  if (!mUpdateTimer)
>   {
>-    // i'd love to be able to just call SetDelay on the existing timer, but
>-    // i think i have to tear it down and make a new one.
>-    mUpdateTimer->Cancel();
>-    mUpdateTimer = NULL;      // free it
>+    nsresult rv = NS_OK;
>+    mUpdateTimer = do_CreateInstance("@mozilla.org/timer;1", &rv);
>+    if (NS_FAILED(rv)) return rv;
>   }
>-  
>-  mUpdateTimer = do_CreateInstance("@mozilla.org/timer;1", &rv);
>-  if (NS_FAILED(rv)) return rv;
> 
>   const PRUint32 kUpdateTimerDelay = 150;
>   return mUpdateTimer->InitWithCallback(NS_STATIC_CAST(nsITimerCallback*, this),

Looks good. Thanks for fixing! Fix the ctor, and sr=sfraser

Comment 5

[Henry Jia]

Comment on attachment 124429 [details] [diff] [review]
proposed fix(good format) 

similar to 204005. r=Henry

Comment 6

[leon.zhang]

Comment on attachment 124429 [details] [diff] [review]
proposed fix(good format) 

checked in according to comments of sfraser.
thx sfraser,henry

Comment 7

[leon.zhang]

fixed in trunk

Comment 8

[Chris Petersen]

Verified on trunk per last comments.

Comment 9

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

This patch caused the leak described in bug 215163 due to incorrect use of the
timer API.