Bug 1424066 - Rename nsResolveHostCallback.OnLookupComplete and nsHostResolver.OnLookupComplete r=mayhemer draft
authorValentin Gosu <valentin.gosu@gmail.com>
Thu, 07 Dec 2017 23:19:48 +0100
changeset 709292 6d06b50c1fa2ba678a887b93f28e1e602e209da4
parent 709280 126e066b771b54d5a56d64b645bb60461f4478cd
child 743383 7873b9f6530b42eb939d8eedff6d2afa7ef46d33
push id92601
push uservalentin.gosu@gmail.com
push dateThu, 07 Dec 2017 22:22:55 +0000
reviewersmayhemer
bugs1424066
milestone59.0a1
Bug 1424066 - Rename nsResolveHostCallback.OnLookupComplete and nsHostResolver.OnLookupComplete r=mayhemer We have several different methods called OnLookupComplete, with similar but different arguments. This makes it difficult to reason about the code, when lots of methods are called the same. * renames nsResolveHostCallback.OnLookupComplete to nsResolveHostCallback.OnResolveHostComplete * renames nsHostResolver.OnLookupComplete to nsHostResolver.CompleteLookup MozReview-Commit-ID: AeWTErs4OQM
netwerk/dns/nsDNSService2.cpp
netwerk/dns/nsHostResolver.cpp
netwerk/dns/nsHostResolver.h
--- a/netwerk/dns/nsDNSService2.cpp
+++ b/netwerk/dns/nsDNSService2.cpp
@@ -311,17 +311,17 @@ public:
         : mResolver(res)
         , mHost(host)
         , mOriginAttributes(attrs)
         , mListener(listener)
         , mFlags(flags)
         , mAF(af)
         , mNetworkInterface(netInterface) {}
 
-    void OnLookupComplete(nsHostResolver *, nsHostRecord *, nsresult) override;
+    void OnResolveHostComplete(nsHostResolver *, nsHostRecord *, nsresult) override;
     // Returns TRUE if the DNS listener arg is the same as the member listener
     // Used in Cancellations to remove DNS requests associated with a
     // particular hostname and nsIDNSListener
     bool EqualsAsyncListener(nsIDNSListener *aListener) override;
 
     size_t SizeOfIncludingThis(mozilla::MallocSizeOf) const override;
 
     RefPtr<nsHostResolver> mResolver;
@@ -329,19 +329,19 @@ public:
     const OriginAttributes   mOriginAttributes; // The originAttributes for this resolving
     nsCOMPtr<nsIDNSListener> mListener;
     uint16_t                 mFlags;
     uint16_t                 mAF;
     nsCString                mNetworkInterface;
 };
 
 void
-nsDNSAsyncRequest::OnLookupComplete(nsHostResolver *resolver,
-                                    nsHostRecord   *hostRecord,
-                                    nsresult        status)
+nsDNSAsyncRequest::OnResolveHostComplete(nsHostResolver *resolver,
+                                         nsHostRecord   *hostRecord,
+                                         nsresult        status)
 {
     // need to have an owning ref when we issue the callback to enable
     // the caller to be able to addref/release multiple times without
     // destroying the record prematurely.
     nsCOMPtr<nsIDNSRecord> rec;
     if (NS_SUCCEEDED(status)) {
         NS_ASSERTION(hostRecord, "no host record");
         rec = new nsDNSRecord(hostRecord);
@@ -397,32 +397,32 @@ class nsDNSSyncRequest : public nsResolv
 {
 public:
     explicit nsDNSSyncRequest(PRMonitor *mon)
         : mDone(false)
         , mStatus(NS_OK)
         , mMonitor(mon) {}
     virtual ~nsDNSSyncRequest() = default;
 
-    void OnLookupComplete(nsHostResolver *, nsHostRecord *, nsresult) override;
+    void OnResolveHostComplete(nsHostResolver *, nsHostRecord *, nsresult) override;
     bool EqualsAsyncListener(nsIDNSListener *aListener) override;
     size_t SizeOfIncludingThis(mozilla::MallocSizeOf) const override;
 
     bool                   mDone;
     nsresult               mStatus;
     RefPtr<nsHostRecord> mHostRecord;
 
 private:
     PRMonitor             *mMonitor;
 };
 
 void
-nsDNSSyncRequest::OnLookupComplete(nsHostResolver *resolver,
-                                   nsHostRecord   *hostRecord,
-                                   nsresult        status)
+nsDNSSyncRequest::OnResolveHostComplete(nsHostResolver *resolver,
+                                        nsHostRecord   *hostRecord,
+                                        nsresult        status)
 {
     // store results, and wake up nsDNSService::Resolve to process results.
     PR_EnterMonitor(mMonitor);
     mDone = true;
     mStatus = status;
     mHostRecord = hostRecord;
     PR_Notify(mMonitor);
     PR_ExitMonitor(mMonitor);
@@ -852,17 +852,17 @@ nsDNSService::AsyncResolveExtendedNative
 
     auto *req =
         new nsDNSAsyncRequest(res, hostname, aOriginAttributes, listener, flags, af,
                               aNetworkInterface);
     if (!req)
         return NS_ERROR_OUT_OF_MEMORY;
     NS_ADDREF(*result = req);
 
-    // addref for resolver; will be released when OnLookupComplete is called.
+    // addref for resolver; will be released when OnResolveHostComplete is called.
     NS_ADDREF(req);
     rv = res->ResolveHost(req->mHost.get(), req->mOriginAttributes, flags, af,
                           req->mNetworkInterface.get(), req);
     if (NS_FAILED(rv)) {
         NS_RELEASE(req);
         NS_RELEASE(*result);
     }
     return rv;
--- a/netwerk/dns/nsHostResolver.cpp
+++ b/netwerk/dns/nsHostResolver.cpp
@@ -594,17 +594,17 @@ void
 nsHostResolver::ClearPendingQueue(PRCList *aPendingQ)
 {
     // loop through pending queue, erroring out pending lookups.
     if (!PR_CLIST_IS_EMPTY(aPendingQ)) {
         PRCList *node = aPendingQ->next;
         while (node != aPendingQ) {
             nsHostRecord *rec = static_cast<nsHostRecord *>(node);
             node = node->next;
-            OnLookupComplete(rec, NS_ERROR_ABORT, nullptr);
+            CompleteLookup(rec, NS_ERROR_ABORT, nullptr);
         }
     }
 }
 
 //
 // FlushCache() is what we call when the network has changed. We must not
 // trust names that were resolved before this change. They may resolve
 // differently now.
@@ -972,17 +972,17 @@ nsHostResolver::ResolveHost(const char  
                         he->rec->flags = flags;
                         mIdleThreadCV.Notify();
                     }
                 }
             }
         }
     }
     if (result) {
-        callback->OnLookupComplete(this, result, status);
+        callback->OnResolveHostComplete(this, result, status);
     }
 
     return rv;
 }
 
 void
 nsHostResolver::DetachCallback(const char             *host,
                                const OriginAttributes &aOriginAttributes,
@@ -1014,17 +1014,17 @@ nsHostResolver::DetachCallback(const cha
                 node = node->next;
             }
         }
     }
 
     // complete callback with the given status code; this would only be done if
     // the record was in the process of being resolved.
     if (rec)
-        callback->OnLookupComplete(this, rec, status);
+        callback->OnResolveHostComplete(this, rec, status);
 }
 
 nsresult
 nsHostResolver::ConditionallyCreateThread(nsHostRecord *rec)
 {
     if (mNumIdleThreads) {
         // wake up idle thread to process this lookup
         mIdleThreadCV.Notify();
@@ -1280,21 +1280,21 @@ different_rrset(AddrInfo *rrset1, AddrIn
             return true;
         }
     }
     LOG(("different_rrset false\n"));
     return false;
 }
 
 //
-// OnLookupComplete() checks if the resolving should be redone and if so it
+// CompleteLookup() checks if the resolving should be redone and if so it
 // returns LOOKUP_RESOLVEAGAIN, but only if 'status' is not NS_ERROR_ABORT.
 // takes ownership of AddrInfo parameter
 nsHostResolver::LookupStatus
-nsHostResolver::OnLookupComplete(nsHostRecord* rec, nsresult status, AddrInfo* newRRSet)
+nsHostResolver::CompleteLookup(nsHostRecord* rec, nsresult status, AddrInfo* newRRSet)
 {
     // get the list of pending callbacks for this lookup, and notify
     // them that the lookup is complete.
     PRCList cbs;
     PR_INIT_CLIST(&cbs);
     {
         MutexAutoLock lock(mLock);
 
@@ -1375,17 +1375,17 @@ nsHostResolver::OnLookupComplete(nsHostR
     }
 
     if (!PR_CLIST_IS_EMPTY(&cbs)) {
         PRCList *node = cbs.next;
         while (node != &cbs) {
             nsResolveHostCallback *callback =
                     static_cast<nsResolveHostCallback *>(node);
             node = node->next;
-            callback->OnLookupComplete(this, rec, status);
+            callback->OnResolveHostComplete(this, rec, status);
             // NOTE: callback must not be dereferenced after this point!!
         }
     }
 
     NS_RELEASE(rec);
 
     return LOOKUP_OK;
 }
@@ -1415,17 +1415,17 @@ nsHostResolver::CancelAsyncRequest(const
         // supplied listener object
         while (node != &he->rec->callbacks) {
             nsResolveHostCallback *callback
                 = static_cast<nsResolveHostCallback *>(node);
             if (callback && (callback->EqualsAsyncListener(aListener))) {
                 // Remove from the list of callbacks
                 PR_REMOVE_LINK(callback);
                 recPtr = he->rec;
-                callback->OnLookupComplete(this, recPtr, status);
+                callback->OnResolveHostComplete(this, recPtr, status);
                 break;
             }
             node = node->next;
         }
 
         // If there are no more callbacks, remove the hash table entry
         if (recPtr && PR_CLIST_IS_EMPTY(&recPtr->callbacks)) {
             mDB.Remove((nsHostKey *)recPtr);
@@ -1518,22 +1518,22 @@ nsHostResolver::ThreadFunc(void *arg)
                     }
                     Telemetry::Accumulate(histogramID, millis);
                 } else {
                     Telemetry::Accumulate(Telemetry::DNS_FAILED_LOOKUP_TIME, millis);
                 }
             }
         }
 
-        // OnLookupComplete may release "rec", long before we lose it.
+        // CompleteLookup may release "rec", long before we lose it.
         LOG(("DNS lookup thread - lookup completed for host [%s%s%s]: %s.\n",
              LOG_HOST(rec->host, rec->netInterface),
              ai ? "success" : "failure: unknown host"));
 
-        if (LOOKUP_RESOLVEAGAIN == resolver->OnLookupComplete(rec, status, ai)) {
+        if (LOOKUP_RESOLVEAGAIN == resolver->CompleteLookup(rec, status, ai)) {
             // leave 'rec' assigned and loop to make a renewed host resolve
             LOG(("DNS lookup thread - Re-resolving host [%s%s%s].\n",
                  LOG_HOST(rec->host, rec->netInterface)));
         } else {
             rec = nullptr;
         }
     }
     resolver->mThreadCount--;
--- a/netwerk/dns/nsHostResolver.h
+++ b/netwerk/dns/nsHostResolver.h
@@ -64,17 +64,17 @@ public:
      * 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.
      */
 
     /* the lock protects |addr_info| and |addr_info_gencnt| because they
      * are mutable and accessed by the resolver worker thread and the
      * nsDNSService2 class.  |addr| doesn't change after it has been
      * assigned a value.  only the resolver worker thread modifies
-     * nsHostRecord (and only in nsHostResolver::OnLookupComplete);
+     * nsHostRecord (and only in nsHostResolver::CompleteLookup);
      * the other threads just read it.  therefore the resolver worker
      * thread doesn't need to lock when reading |addr_info|.
      */
     Mutex        addr_info_lock;
     int          addr_info_gencnt; /* generation count of |addr_info| */
     mozilla::net::AddrInfo *addr_info;
     mozilla::UniquePtr<mozilla::net::NetAddr> addr;
     bool         negative;   /* True if this record is a cache of a failed lookup.
@@ -165,35 +165,35 @@ private:
 /**
  * ResolveHost callback object.  It's PRCList members are used by
  * the nsHostResolver and should not be used by anything else.
  */
 class NS_NO_VTABLE nsResolveHostCallback : public PRCList
 {
 public:
     /**
-     * OnLookupComplete
+     * OnResolveHostComplete
      *
      * this function is called to complete a host lookup initiated by
      * nsHostResolver::ResolveHost.  it may be invoked recursively from
      * ResolveHost or on an unspecified background thread.
      *
      * NOTE: it is the responsibility of the implementor of this method
      * to handle the callback in a thread safe manner.
      *
      * @param resolver
      *        nsHostResolver object associated with this result
      * @param record
      *        the host record containing the results of the lookup
      * @param status
      *        if successful, |record| contains non-null results
      */
-    virtual void OnLookupComplete(nsHostResolver *resolver,
-                                  nsHostRecord   *record,
-                                  nsresult        status) = 0;
+    virtual void OnResolveHostComplete(nsHostResolver *resolver,
+                                       nsHostRecord   *record,
+                                       nsresult        status) = 0;
     /**
      * EqualsAsyncListener
      *
      * Determines if the listener argument matches the listener member var.
      * For subclasses not implementing a member listener, should return false.
      * For subclasses having a member listener, the function should check if
      * they are the same.  Used for cases where a pointer to an object
      * implementing nsResolveHostCallback is unknown, but a pointer to
@@ -313,17 +313,17 @@ private:
     nsresult IssueLookup(nsHostRecord *);
     bool     GetHostToLookup(nsHostRecord **m);
 
     enum LookupStatus {
       LOOKUP_OK,
       LOOKUP_RESOLVEAGAIN,
     };
 
-    LookupStatus OnLookupComplete(nsHostRecord *, nsresult, mozilla::net::AddrInfo *);
+    LookupStatus CompleteLookup(nsHostRecord *, nsresult, mozilla::net::AddrInfo *);
     void     DeQueue(PRCList &aQ, nsHostRecord **aResult);
     void     ClearPendingQueue(PRCList *aPendingQueue);
     nsresult ConditionallyCreateThread(nsHostRecord *rec);
 
     /**
      * Starts a new lookup in the background for entries that are in the grace
      * period with a failed connect or all cached entries are negative.
      */