Bug 1502641 - Change ref-counting for nsHostRecord, change nsCOMPtr<*HostRecord> into RefPtr<*HostRecord>, remove pure virtual functions from nsHostRecord. r=valentin
authorDragana Damjanovic <dd.mozilla@gmail.com>
Mon, 19 Nov 2018 15:23:39 +0000
changeset 446999 b0863b729ae965aed746510ef756b0727cc12916
parent 446998 335fa26ef0c43bd0304d7c9fe36edee070f97717
child 447000 f4c51b6f89cbf940b0f5f65fcbec258c42c1064e
push id35065
push userrmaries@mozilla.com
push dateMon, 19 Nov 2018 21:56:32 +0000
treeherdermozilla-central@bd4cebdbed4b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersvalentin
bugs1502641
milestone65.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1502641 - Change ref-counting for nsHostRecord, change nsCOMPtr<*HostRecord> into RefPtr<*HostRecord>, remove pure virtual functions from nsHostRecord. r=valentin Differential Revision: https://phabricator.services.mozilla.com/D12121
netwerk/dns/nsDNSService2.cpp
netwerk/dns/nsHostResolver.cpp
netwerk/dns/nsHostResolver.h
--- a/netwerk/dns/nsDNSService2.cpp
+++ b/netwerk/dns/nsDNSService2.cpp
@@ -25,16 +25,17 @@
 #include "prsystem.h"
 #include "prnetdb.h"
 #include "prmon.h"
 #include "prio.h"
 #include "plstr.h"
 #include "nsCharSeparatedTokenizer.h"
 #include "nsNetAddr.h"
 #include "nsProxyRelease.h"
+#include "nsQueryObject.h"
 #include "nsIObserverService.h"
 #include "nsINetworkLinkService.h"
 #include "TRRService.h"
 
 #include "mozilla/Attributes.h"
 #include "mozilla/ClearOnShutdown.h"
 #include "mozilla/net/NeckoCommon.h"
 #include "mozilla/net/ChildDNSService.h"
@@ -66,23 +67,23 @@ public:
     NS_DECL_THREADSAFE_ISUPPORTS
     NS_DECL_NSIDNSRECORD
 
     explicit nsDNSRecord(nsHostRecord *hostRecord)
         : mIter(nullptr)
         , mIterGenCnt(-1)
         , mDone(false)
     {
-        mHostRecord = do_QueryInterface(hostRecord);
+        mHostRecord = do_QueryObject(hostRecord);
     }
 
 private:
     virtual ~nsDNSRecord() = default;
 
-    nsCOMPtr<AddrHostRecord>  mHostRecord;
+    RefPtr<AddrHostRecord>  mHostRecord;
     NetAddrElement            *mIter;
     int                        mIterGenCnt; // the generation count of
                                             // mHostRecord->addr_info when we
                                             // start iterating
     bool                       mDone;
 };
 
 NS_IMPL_ISUPPORTS(nsDNSRecord, nsIDNSRecord)
@@ -312,22 +313,22 @@ nsDNSRecord::ReportUnusable(uint16_t aPo
 class nsDNSByTypeRecord : public nsIDNSByTypeRecord
 {
 public:
     NS_DECL_THREADSAFE_ISUPPORTS
     NS_DECL_NSIDNSBYTYPERECORD
 
     explicit nsDNSByTypeRecord(nsHostRecord *hostRecord)
     {
-        mHostRecord = do_QueryInterface(hostRecord);
+        mHostRecord = do_QueryObject(hostRecord);
     }
 
 private:
     virtual ~nsDNSByTypeRecord() = default;
-    nsCOMPtr<TypeHostRecord>  mHostRecord;
+    RefPtr<TypeHostRecord>  mHostRecord;
 };
 
 NS_IMPL_ISUPPORTS(nsDNSByTypeRecord, nsIDNSByTypeRecord)
 
 NS_IMETHODIMP
 nsDNSByTypeRecord::GetRecords(nsTArray<nsCString> &aRecords)
 {
    // deep copy
--- a/netwerk/dns/nsHostResolver.cpp
+++ b/netwerk/dns/nsHostResolver.cpp
@@ -24,16 +24,17 @@
 #include "nsPrintfCString.h"
 #include "nsXPCOMCIDInternal.h"
 #include "prthread.h"
 #include "prerror.h"
 #include "prtime.h"
 #include "mozilla/Logging.h"
 #include "PLDHashTable.h"
 #include "plstr.h"
+#include "nsQueryObject.h"
 #include "nsURLHelper.h"
 #include "nsThreadUtils.h"
 #include "nsThreadPool.h"
 #include "GetAddrInfo.h"
 #include "GeckoProfiler.h"
 #include "TRR.h"
 #include "TRRService.h"
 
@@ -196,30 +197,27 @@ size_t
 nsHostKey::SizeOfExcludingThis(mozilla::MallocSizeOf mallocSizeOf) const
 {
     size_t n = 0;
     n += host.SizeOfExcludingThisIfUnshared(mallocSizeOf);
     n += originSuffix.SizeOfExcludingThisIfUnshared(mallocSizeOf);
     return n;
 }
 
+NS_IMPL_ISUPPORTS0(nsHostRecord)
+
 nsHostRecord::nsHostRecord(const nsHostKey& key)
     : nsHostKey(key)
     , mResolverMode(MODE_NATIVEONLY)
     , mResolving(0)
     , negative(false)
     , mDoomed(false)
 {
 }
 
-nsHostRecord::~nsHostRecord()
-{
-    mCallbacks.clear();
-}
-
 void
 nsHostRecord::Invalidate()
 {
     mDoomed = true;
 }
 
 nsHostRecord::ExpirationStatus
 nsHostRecord::CheckExpiration(const mozilla::TimeStamp& now) const
@@ -289,17 +287,17 @@ SizeOfResolveHostCallbackListExcludingHe
 
     for (const nsResolveHostCallback* t = aCallbacks.getFirst(); t; t = t->getNext()) {
       n += t->SizeOfIncludingThis(mallocSizeOf);
     }
 
     return n;
 }
 
-NS_IMPL_ISUPPORTS(AddrHostRecord, nsISupports, AddrHostRecord, nsHostRecord)
+NS_IMPL_ISUPPORTS_INHERITED(AddrHostRecord, nsHostRecord, AddrHostRecord)
 
 AddrHostRecord::AddrHostRecord(const nsHostKey& key)
     : nsHostRecord(key)
     , addr_info_lock("AddrHostRecord.addr_info_lock")
     , addr_info_gencnt(0)
     , addr_info(nullptr)
     , addr(nullptr)
     , mFirstTRRresult(NS_OK)
@@ -317,17 +315,17 @@ AddrHostRecord::AddrHostRecord(const nsH
     , mTrrAAAAUsed(INIT)
     , mTrrLock("AddrHostRecord.mTrrLock")
     , mBlacklistedCount(0)
 {
 }
 
 AddrHostRecord::~AddrHostRecord()
 {
-
+    mCallbacks.clear();
     Telemetry::Accumulate(Telemetry::DNS_BLACKLIST_COUNT, mBlacklistedCount);
     delete addr_info;
 }
 
 bool
 AddrHostRecord::Blacklisted(NetAddr *aQuery)
 {
     // must call locked
@@ -562,26 +560,29 @@ AddrHostRecord::GetPriority(uint16_t aFl
     }
     if (IsMediumPriority(aFlags)) {
         return AddrHostRecord::DNS_PRIORITY_MEDIUM;
     }
 
     return AddrHostRecord::DNS_PRIORITY_LOW;
 }
 
-NS_IMPL_ISUPPORTS(TypeHostRecord, nsISupports, TypeHostRecord, nsHostRecord)
+NS_IMPL_ISUPPORTS_INHERITED(TypeHostRecord, nsHostRecord, TypeHostRecord)
 
 TypeHostRecord::TypeHostRecord(const nsHostKey& key)
     : nsHostRecord(key)
     , mTrrLock("TypeHostRecord.mTrrLock")
     , mResultsLock("TypeHostRecord.mResultsLock")
 {
 }
 
-TypeHostRecord::~TypeHostRecord() = default;
+TypeHostRecord::~TypeHostRecord()
+{
+    mCallbacks.clear();
+}
 
 bool
 TypeHostRecord::HasUsableResultInternal() const
 {
     return !mResults.IsEmpty();
 }
 
 void
@@ -781,17 +782,17 @@ nsHostResolver::FlushCache()
 
     // Refresh the cache entries that are resolving RIGHT now, remove the rest.
     for (auto iter = mRecordDB.Iter(); !iter.Done(); iter.Next()) {
         nsHostRecord* record = iter.UserData();
         // Try to remove the record, or mark it for refresh.
         // By-type records are from TRR. We do not need to flush those entry
         // when the network has change, because they are not local.
         if (record->IsAddrRecord()) {
-            nsCOMPtr<AddrHostRecord> addrRec = do_QueryInterface(record);
+            RefPtr<AddrHostRecord> addrRec = do_QueryObject(record);
             MOZ_ASSERT(addrRec);
             if (addrRec->RemoveOrRefresh()) {
                 if (record->isInList()) {
                     record->remove();
                 }
                 iter.Remove();
             }
         }
@@ -878,17 +879,17 @@ nsHostResolver::GetHostRecord(const nsAC
             entry = new AddrHostRecord(key);
         } else {
             entry = new TypeHostRecord(key);
         }
     }
 
     RefPtr<nsHostRecord> rec = entry;
     if (rec->IsAddrRecord()) {
-        nsCOMPtr<AddrHostRecord> addrRec = do_QueryInterface(rec);
+        RefPtr<AddrHostRecord> addrRec = do_QueryObject(rec);
         if (addrRec->addr) {
             return NS_ERROR_FAILURE;
         }
     }
 
     if (rec->mResolving) {
         return NS_ERROR_FAILURE;
     }
@@ -965,17 +966,17 @@ nsHostResolver::ResolveHost(const nsACSt
                 if (IS_ADDR_TYPE(type)) {
                     entry = new AddrHostRecord(key);
                 } else {
                     entry = new TypeHostRecord(key);
                 }
             }
 
             RefPtr<nsHostRecord> rec = entry;
-            nsCOMPtr<AddrHostRecord> addrRec = do_QueryInterface(rec);
+            RefPtr<AddrHostRecord> addrRec = do_QueryObject(rec);
             MOZ_ASSERT(rec, "Record should not be null");
             MOZ_ASSERT((IS_ADDR_TYPE(type) && rec->IsAddrRecord() && addrRec) ||
                        (IS_OTHER_TYPE(type) && !rec->IsAddrRecord()));
 
             // Check if the entry is vaild.
             if (!(flags & RES_BYPASS_CACHE) &&
                 rec->HasUsableResult(TimeStamp::NowLoRes(), flags)) {
                 LOG(("  Using cached record for host [%s].\n", host.get()));
@@ -1063,18 +1064,18 @@ nsHostResolver::ResolveHost(const nsACSt
                                               (aOriginAttributes.mPrivateBrowsingId > 0),
                                               originSuffix);
                     RefPtr<nsHostRecord> unspecRec = mRecordDB.Get(unspecKey);
 
                     TimeStamp now = TimeStamp::NowLoRes();
                     if (unspecRec && unspecRec->HasUsableResult(now, flags)) {
                         MOZ_ASSERT(unspecRec->IsAddrRecord());
 
-                        nsCOMPtr<AddrHostRecord> addrUnspecRec =
-                            do_QueryInterface(unspecRec);
+                        RefPtr<AddrHostRecord> addrUnspecRec =
+                            do_QueryObject(unspecRec);
                         MOZ_ASSERT(addrUnspecRec);
                         MOZ_ASSERT(addrUnspecRec->addr_info ||
                                    addrUnspecRec->negative,
                                    "Entry should be resolved or negative.");
 
                         LOG(("  Trying AF_UNSPEC entry for host [%s] af: %s.\n", host.get(),
                              (af == PR_AF_INET) ? "AF_INET" : "AF_INET6"));
 
@@ -1305,24 +1306,24 @@ nsHostResolver::TrrLookup_unlocked(nsHos
 // returns error if no TRR resolve is issued
 // it is impt this is not called while a native lookup is going on
 nsresult
 nsHostResolver::TrrLookup(nsHostRecord *aRec, TRR *pushedTRR)
 {
     RefPtr<nsHostRecord> rec(aRec);
     mLock.AssertCurrentThreadOwns();
 
-    nsCOMPtr<AddrHostRecord> addrRec;
-    nsCOMPtr<TypeHostRecord> typeRec;
+    RefPtr<AddrHostRecord> addrRec;
+    RefPtr<TypeHostRecord> typeRec;
 
     if (rec->IsAddrRecord()) {
-        addrRec = do_QueryInterface(rec);
+        addrRec = do_QueryObject(rec);
         MOZ_ASSERT(addrRec);
     } else {
-        typeRec = do_QueryInterface(rec);
+        typeRec = do_QueryObject(rec);
         MOZ_ASSERT(typeRec);
     }
 
 #ifdef DEBUG
     if (rec->IsAddrRecord()) {
         MutexAutoLock trrlock(addrRec->mTrrLock);
         MOZ_ASSERT(!TRROutstanding());
     }
@@ -1434,18 +1435,18 @@ nsHostResolver::AssertOnQ(nsHostRecord *
 nsresult
 nsHostResolver::NativeLookup(nsHostRecord *aRec)
 {
     // Only A/AAAA request are resolve natively.
     MOZ_ASSERT(aRec->IsAddrRecord());
     mLock.AssertCurrentThreadOwns();
 
     RefPtr<nsHostRecord> rec(aRec);
-    nsCOMPtr<AddrHostRecord> addrRec;
-    addrRec = do_QueryInterface(rec);
+    RefPtr<AddrHostRecord> addrRec;
+    addrRec = do_QueryObject(rec);
     MOZ_ASSERT(addrRec);
 
     addrRec->mNativeStart = TimeStamp::Now();
 
     // Add rec to one of the pending queues, possibly removing it from mEvictionQ.
     if (rec->isInList()) {
         MOZ_ASSERT(mEvictionQSize);
         AssertOnQ(rec, mEvictionQ);
@@ -1502,18 +1503,17 @@ nsHostResolver::NameLookup(nsHostRecord 
     if (rec->mResolving) {
         LOG(("NameLookup %s while already resolving\n", rec->host.get()));
         return NS_OK;
     }
 
     ResolverMode mode = rec->mResolverMode = Mode();
 
     if (rec->IsAddrRecord()) {
-        nsCOMPtr<AddrHostRecord> addrRec;
-        addrRec = do_QueryInterface(rec);
+        RefPtr<AddrHostRecord> addrRec = do_QueryObject(rec);
         MOZ_ASSERT(addrRec);
 
         addrRec->mNativeUsed = false;
         addrRec->mTRRUsed = false;
         addrRec->mNativeSuccess = false;
         addrRec->mTRRSuccess = 0;
         addrRec->mDidCallbacks = false;
         addrRec->mTrrAUsed = AddrHostRecord::INIT;
@@ -1565,18 +1565,17 @@ nsHostResolver::ConditionallyRefreshReco
 
 void
 nsHostResolver::DeQueue(LinkedList<RefPtr<nsHostRecord>>& aQ,
                         AddrHostRecord **aResult)
 {
     RefPtr<nsHostRecord> rec = aQ.popFirst();
     mPendingCount--;
     MOZ_ASSERT(rec->IsAddrRecord());
-    nsCOMPtr<AddrHostRecord> addrRec;
-    addrRec = do_QueryInterface(rec);
+    RefPtr<AddrHostRecord> addrRec = do_QueryObject(rec);
     MOZ_ASSERT(addrRec);
     addrRec->onQueue = false;
     addrRec.forget(aResult);
 }
 
 bool
 nsHostResolver::GetHostToLookup(AddrHostRecord **result)
 {
@@ -1791,17 +1790,17 @@ nsHostResolver::LookupStatus
 nsHostResolver::CompleteLookup(nsHostRecord* rec, nsresult status, AddrInfo* aNewRRSet, bool pb,
                                const nsACString & aOriginsuffix)
 {
     MutexAutoLock lock(mLock);
     MOZ_ASSERT(rec);
     MOZ_ASSERT(rec->pb == pb);
     MOZ_ASSERT(rec->IsAddrRecord());
 
-    nsCOMPtr<AddrHostRecord> addrRec = do_QueryInterface(rec);
+    RefPtr<AddrHostRecord> addrRec = do_QueryObject(rec);
     MOZ_ASSERT(addrRec);
 
     // newRRSet needs to be taken into the hostrecord (which will then own it)
     // or deleted on early return.
     nsAutoPtr<AddrInfo> newRRSet(aNewRRSet);
 
     bool trrResult = newRRSet && newRRSet->IsTRR();
 
@@ -2034,18 +2033,17 @@ nsHostResolver::CompleteLookupByType(nsH
                                      const nsTArray<nsCString> *aResult,
                                      uint32_t aTtl, bool pb)
 {
     MutexAutoLock lock(mLock);
     MOZ_ASSERT(rec);
     MOZ_ASSERT(rec->pb == pb);
     MOZ_ASSERT(!rec->IsAddrRecord());
 
-    nsCOMPtr<TypeHostRecord> typeRec;
-    typeRec = do_QueryInterface(rec);
+    RefPtr<TypeHostRecord> typeRec = do_QueryObject(rec);
     MOZ_ASSERT(typeRec);
 
     MOZ_ASSERT(typeRec->mResolving);
     typeRec->mResolving--;
 
     MutexAutoLock trrlock(typeRec->mTrrLock);
     typeRec->mTrr = nullptr;
 
@@ -2155,22 +2153,22 @@ nsHostResolver::SizeOfIncludingThis(Mall
 void
 nsHostResolver::ThreadFunc()
 {
     LOG(("DNS lookup thread - starting execution.\n"));
 
 #if defined(RES_RETRY_ON_FAILURE)
     nsResState rs;
 #endif
-    nsCOMPtr<AddrHostRecord> rec;
+    RefPtr<AddrHostRecord> rec;
     AddrInfo *ai = nullptr;
 
     do {
         if (!rec) {
-            nsCOMPtr<AddrHostRecord> tmpRec;
+            RefPtr<AddrHostRecord> tmpRec;
             if (!GetHostToLookup(getter_AddRefs(tmpRec))) {
                 break; // thread shutdown signal
             }
             // GetHostToLookup() returns an owning reference
             MOZ_ASSERT(tmpRec);
             rec.swap(tmpRec);
         }
 
@@ -2279,17 +2277,17 @@ nsHostResolver::GetDNSCacheEntries(nsTAr
             continue;
         }
 
         // For now we only show A/AAAA records.
         if (!rec->IsAddrRecord()) {
             continue;
         }
 
-        nsCOMPtr<AddrHostRecord> addrRec = do_QueryInterface(rec);
+        RefPtr<AddrHostRecord> addrRec = do_QueryObject(rec);
         MOZ_ASSERT(addrRec);
         if (!addrRec || !addrRec->addr_info) {
             continue;
         }
 
         DNSCacheEntries info;
         info.hostname = rec->host;
         info.family = rec->af;
--- a/netwerk/dns/nsHostResolver.h
+++ b/netwerk/dns/nsHostResolver.h
@@ -60,38 +60,37 @@ struct nsHostKey
     const nsCString originSuffix;
     explicit nsHostKey(const nsACString& host, uint16_t  type, uint16_t flags,
                        uint16_t af, bool pb, const nsACString& originSuffix);
     bool operator==(const nsHostKey& other) const;
     size_t SizeOfExcludingThis(mozilla::MallocSizeOf mallocSizeOf) const;
     PLDHashNumber Hash() const;
 };
 
-// 9c29024a-e7ea-48b0-945e-058a8687247b
-#define NS_HOSTRECORD_IID \
-{ 0x9c29024a, 0xe7ea, 0x48b0, {0x94, 0x5e, 0x05, 0x8a, 0x86, 0x87, 0x24, 0x7b }}
-
 /**
  * nsHostRecord - ref counted object type stored in host resolver cache.
  */
 class nsHostRecord :
     public mozilla::LinkedListElement<RefPtr<nsHostRecord>>,
     public nsHostKey,
     public nsISupports
 {
 public:
-    NS_DECLARE_STATIC_IID_ACCESSOR(NS_HOSTRECORD_IID)
+    NS_DECL_THREADSAFE_ISUPPORTS
 
-    virtual size_t SizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf) const = 0;
+    virtual size_t SizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf) const
+    {
+      return 0;
+    }
 
 protected:
     friend class nsHostResolver;
 
     explicit nsHostRecord(const nsHostKey& key);
-    virtual ~nsHostRecord();
+    virtual ~nsHostRecord() = default;
 
     // Mark hostrecord as not usable
     void Invalidate();
 
     enum ExpirationStatus {
         EXP_VALID,
         EXP_GRACE,
         EXP_EXPIRED,
@@ -110,19 +109,19 @@ protected:
 
     enum DnsPriority {
         DNS_PRIORITY_LOW,
         DNS_PRIORITY_MEDIUM,
         DNS_PRIORITY_HIGH,
     };
     static DnsPriority GetPriority(uint16_t aFlags);
 
-    virtual void Cancel() = 0;
+    virtual void Cancel() {}
 
-    virtual bool HasUsableResultInternal() const = 0;
+    virtual bool HasUsableResultInternal() const { return false; }
 
     mozilla::LinkedList<RefPtr<nsResolveHostCallback>> mCallbacks;
 
     bool IsAddrRecord() const {
         return type == nsIDNSService::RESOLVE_TYPE_DEFAULT;
     }
 
     // When the record began being valid. Used mainly for bookkeeping.
@@ -144,29 +143,27 @@ protected:
 
     uint8_t negative : 1;   /* True if this record is a cache of a failed lookup.
                                Negative cache entries are valid just like any other
                                (though never for more than 60 seconds), but a use
                                of that negative entry forces an asynchronous refresh. */
     uint8_t mDoomed : 1;    // explicitly expired
 };
 
-NS_DEFINE_STATIC_IID_ACCESSOR(nsHostRecord, NS_HOSTRECORD_IID)
-
 // b020e996-f6ab-45e5-9bf5-1da71dd0053a
 #define ADDRHOSTRECORD_IID \
 { 0xb020e996, 0xf6ab, 0x45e5, {0x9b, 0xf5, 0x1d, 0xa7, 0x1d, 0xd0, 0x05, 0x3a }}
 
 class AddrHostRecord final : public nsHostRecord
 {
     typedef mozilla::Mutex Mutex;
 
 public:
     NS_DECLARE_STATIC_IID_ACCESSOR(ADDRHOSTRECORD_IID)
-    NS_DECL_THREADSAFE_ISUPPORTS
+    NS_DECL_ISUPPORTS_INHERITED
 
     /* a fully resolved host record has either a non-null |addr_info| or |addr|
      * field.  if |addr_info| is null, it implies that the |host| is an IP
      * address literal.  in which case, |addr| contains the parsed address.
      * otherwise, if |addr_info| is non-null, then it contains one or many
      * IP addresses corresponding to the given host name.  if both |addr_info|
      * and |addr| are null, then the given host has not yet been fully resolved.
      * |af| is the address family of the record we are querying for.
@@ -268,17 +265,17 @@ NS_DEFINE_STATIC_IID_ACCESSOR(AddrHostRe
 // 77b786a7-04be-44f2-987c-ab8aa96676e0
 #define TYPEHOSTRECORD_IID \
 { 0x77b786a7, 0x04be, 0x44f2, {0x98, 0x7c, 0xab, 0x8a, 0xa9, 0x66, 0x76, 0xe0 }}
 
 class TypeHostRecord final : public nsHostRecord
 {
 public:
     NS_DECLARE_STATIC_IID_ACCESSOR(TYPEHOSTRECORD_IID)
-    NS_DECL_THREADSAFE_ISUPPORTS
+    NS_DECL_ISUPPORTS_INHERITED
 
     void GetRecords(nsTArray<nsCString> &aRecords);
     void GetRecordsAsOneString(nsACString &aRecords);
 
     size_t SizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf) const override;
 
 private:
     friend class nsHostResolver;