Last Comment Bug 517070 - Replace manual refcounting of nsTimeouts in nsGlobalWindow.cpp with equialent nsRefPtr usage
: Replace manual refcounting of nsTimeouts in nsGlobalWindow.cpp with equialent...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: General (show other bugs)
: Trunk
: All All
: -- minor (vote)
: mozilla7
Assigned To: Wade Smith
:
Mentors:
: 395563 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-09-16 14:37 PDT by Ben Newman (:bnewman) (:benjamn)
Modified: 2011-06-21 10:24 PDT (History)
8 users (show)
mounir: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch to replace a couple of manual ref-countings with automatic (4.42 KB, patch)
2011-06-08 22:19 PDT, Wade Smith
jst: review+
Details | Diff | Review

Description Ben Newman (:bnewman) (:benjamn) 2009-09-16 14:37:26 PDT
I suspect there may be bugs in all the calls to timeout->AddRef() and timeout->Release() found in nsGlobalWindow.cpp.  For better or worse, most of them are meticulously commented, e.g.

  // Hold on to the timeout to ensure it doesn't go away while it's
  // being handled (aka kungFuDeathGrip).
  timeout->AddRef();
  timeout->mWindow->RunTimeout(timeout);
  // Drop our reference to the timeout now that we're done with it.
  timeout->Release();

This kind of discipline is unnecessary and error-prone.  Almost all of these usages can be replaced with nsRefPtr stack-based auto-refcounting.
Comment 1 Wade Smith 2011-06-08 22:19:17 PDT
Created attachment 538173 [details] [diff] [review]
Patch to replace a couple of manual ref-countings with automatic

This is the first patch I've ever written, so let me know if I've done something wrong so I can learn :). There were a couple of instances of calls to AddRef() that didn't seem like they could be removed, but there were two places where AddRef() was called and then Release() called at every point the function returned which have been removed and replaced with an nsRefPtr
Comment 2 Josh Matthews [:jdm] 2011-06-09 00:45:47 PDT
Hey, this is great! Thanks! Let's see what jst thinks.
Comment 3 Johnny Stenback (:jst, jst@mozilla.com) 2011-06-17 14:26:57 PDT
Comment on attachment 538173 [details] [diff] [review]
Patch to replace a couple of manual ref-countings with automatic

Yup, this looks good to me. Thanks Wade!
Comment 4 Phil Ringnalda (:philor) 2011-06-18 20:19:34 PDT
http://hg.mozilla.org/integration/mozilla-inbound/rev/2cc9093cdd23
Comment 5 Mounir Lamouri (:mounir) 2011-06-19 04:07:51 PDT
Merged:
http://hg.mozilla.org/mozilla-central/rev/2cc9093cdd23
Comment 6 :Ms2ger 2011-06-21 10:24:46 PDT
*** Bug 395563 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.