517070 - Replace manual refcounting of nsTimeouts in nsGlobalWindow.cpp with equialent nsRefPtr usage

Status: RESOLVED FIXED

(Core :: General, defect)

Description

[Ben Newman (:bnewman) (:benjamn)]

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]

Attachment: 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]]

Hey, this is great! Thanks! Let's see what jst thinks.

Comment 3

[Johnny Stenback (:jst)]

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)]

http://hg.mozilla.org/integration/mozilla-inbound/rev/2cc9093cdd23

Comment 5

[Mounir Lamouri (:mounir)]

Merged:
http://hg.mozilla.org/mozilla-central/rev/2cc9093cdd23