Eliminate Newborn/NoAddRef (bug 5907, r=ds).

core/ConCmdManager.cpp

@@ -383,7 +383,7 @@ bool ConCmdManager::AddAdminCommand(IPluginFunction *pFunction,
 	{
 		if (!m_CmdGrps.add(i, group))
 			return false;
-		i->value = NoAddRef(new CommandGroup());
+		i->value = new CommandGroup();
 	}
 	Ref<CommandGroup> cmdgroup = i->value;
 

core/logic/GameConfigs.cpp

@@ -1082,6 +1082,7 @@ bool GameConfigManager::LoadGameConfigFile(const char *file, IGameConfig **_pCon
 	}
 
 	pConfig = new CGameConfig(file);
+	pConfig->AddRef();
 
 	/* :HACKHACK: Don't parse the main config file */
 	bool retval = true;

core/logic/ShareSys.cpp

@@ -375,7 +375,7 @@ PassRef<Native> ShareSystem::AddNativeToCache(CNativeOwner *pOwner, const sp_nat
 	if (i.found())
 		return NULL;
 
-	Ref<Native> entry = Newborn<Native>(new Native(pOwner, ntv));
+	Ref<Native> entry = new Native(pOwner, ntv);
 	m_NtvCache.insert(ntv->name, entry);
 	return entry;
 }
@@ -415,7 +415,7 @@ PassRef<Native> ShareSystem::AddFakeNative(IPluginFunction *pFunc, const char *n
 
 	CNativeOwner *owner = g_PluginSys.GetPluginByCtx(fake->ctx->GetContext());
 
-	entry = Newborn<Native>(new Native(owner, fake.take()));
+	entry = new Native(owner, fake.take());
 	m_NtvCache.insert(name, entry);
 
 	return entry;

core/logic/smn_adt_trie.cpp

@@ -163,7 +163,7 @@ private:
 	cell_t data_;
 };
 
-struct CellTrie : public ke::Refcounted<CellTrie>
+struct CellTrie
 {
 	StringHashMap<Entry> map;
 };
@@ -204,8 +204,7 @@ public: //IHandleTypeDispatch
 	{
 		if (type == htCellTrie)
 		{
-			CellTrie *pTrie = (CellTrie *)object;
+			delete (CellTrie *)object;
-			pTrie->Release();
 		} else {
 			TrieSnapshot *snapshot = (TrieSnapshot *)object;
 			delete snapshot;

extensions/clientprefs/extension.cpp

@@ -219,7 +219,7 @@ void ClientPrefs::DatabaseConnect()
 	char error[256];
 	int errCode = 0;
 
-	Database = Newborn<IDatabase>(Driver->Connect(DBInfo, true, error, sizeof(error)));
+	Database = AdoptRef(Driver->Connect(DBInfo, true, error, sizeof(error)));
 
 	if (!Database)
 	{

public/amtl/am-refcounting-threadsafe.h

@@ -35,12 +35,15 @@
 
 namespace ke {
 
+// See the comment above Refcounted<T> for more information. This class is
+// identical, except changing the reference count is guaranteed to be atomic
+// with respect to other threads changing the reference count.
 template <typename T>
 class RefcountedThreadsafe
 {
   public:
     RefcountedThreadsafe()
-     : refcount_(1)
+     : refcount_(0)
     {
     }
 

public/amtl/am-refcounting.h

@@ -37,18 +37,28 @@ namespace ke {
 
 template <typename T> class Ref;
 
-// Holds a refcounted T without addrefing it. This is similar to PassRef<>
+// Objects in AMTL inheriting from Refcounted will have an initial refcount
-// below, but is intended only for freshly allocated objects which start
+// of 0. However, in some systems (such as COM), the initial refcount is 1,
-// with reference count 1, and we don't want to add an extra ref just by
+// or functions may return raw pointers that have been AddRef'd. In these
-// assigning to PassRef<> or Ref<>.
+// cases it would be a mistake to use Ref<> or PassRef<>, since the object
+// would leak an extra reference.
+//
+// This container holds a refcounted object without addrefing it. This is
+// intended only for interacting with functions which return an object that
+// has been manually AddRef'd. Note that this will perform a Release(), so
+// so it is necessary to assign it to retain the object.
 template <typename T>
-class Newborn
+class AlreadyRefed
 {
   public:
-    Newborn(T *t)
+    AlreadyRefed(T *t)
       : thing_(t)
     {
     }
+    ~AlreadyRefed() {
+        if (thing_)
+            thing_->Release();
+    }
 
     T *release() const {
         return ReturnAndVoid(thing_);
@@ -59,10 +69,10 @@ class Newborn
 };
 
 template <typename T>
-static inline Newborn<T>
+static inline AlreadyRefed<T>
-NoAddRef(T *t)
+AdoptRef(T *t)
 {
-    return Newborn<T>(t);
+    return AlreadyRefed<T>(t);
 }
 
 // When returning a value, we'd rather not be needlessly changing the refcount,
@@ -81,7 +91,14 @@ class PassRef
     {
     }
 
-    PassRef(const Newborn<T *> &other)
+    PassRef(const AlreadyRefed<T *> &other)
+      : thing_(other.release())
+    {
+        // Don't addref, newborn means already addref'd.
+    }
+
+    template <typename S>
+    PassRef(const AlreadyRefed<S *> &other)
       : thing_(other.release())
     {
         // Don't addref, newborn means already addref'd.
@@ -134,7 +151,7 @@ class PassRef
   private:
     // Disallowed operators.
     PassRef &operator =(T *other);
-    PassRef &operator =(Newborn<T> &other);
+    PassRef &operator =(AlreadyRefed<T> &other);
 
     void AddRef() {
         if (thing_)
@@ -149,13 +166,18 @@ class PassRef
     mutable T *thing_;
 };
 
-// Classes which are refcounted should inherit from this.
+// Classes which are refcounted should inherit from this. Note that reference
+// counts start at 0 in AMTL, rather than 1. This avoids the complexity of
+// having to adopt the initial ref upon allocation. However, this also means
+// invoking Release() on a newly allocated object is illegal. Newborn objects
+// must either be assigned to a Ref or PassRef (NOT an AdoptRef/AlreadyRefed),
+// or must be deleted using |delete|.
 template <typename T>
 class Refcounted
 {
   public:
     Refcounted()
-      : refcount_(1)
+      : refcount_(0)
     {
     }
 
@@ -217,7 +239,12 @@ class Ref
       : thing_(other.release())
     {
     }
-    Ref(const Newborn<T> &other)
+    Ref(const AlreadyRefed<T> &other)
+      : thing_(other.release())
+    {
+    }
+    template <typename S>
+    Ref(const AlreadyRefed<S> &other)
       : thing_(other.release())
     {
     }
@@ -255,7 +282,7 @@ class Ref
     }
 
     template <typename S>
-    Ref &operator =(const Newborn<S> &other) {
+    Ref &operator =(const AlreadyRefed<S> &other) {
         Release();
         thing_ = other.release();
         return *this;