Bug 1616014 - Get rid off nsMainThreadPtrHandle in DNSListenerProxy r=valentin

Differential Revision: https://phabricator.services.mozilla.com/D75485

netwerk/dns/ChildDNSService.cpp

@@ -10,6 +10,7 @@
 #include "nsIXPConnect.h"
 #include "nsIProtocolProxyService.h"
 #include "nsNetCID.h"
+#include "nsQueryObject.h"
 #include "mozilla/ClearOnShutdown.h"
 #include "mozilla/StaticPtr.h"
 #include "mozilla/net/NeckoChild.h"
@@ -61,7 +62,7 @@ ChildDNSService::ChildDNSService()
 void ChildDNSService::GetDNSRecordHashKey(
     const nsACString& aHost, const nsACString& aTrrServer, uint16_t aType,
     const OriginAttributes& aOriginAttributes, uint32_t aFlags,
-    nsIDNSListener* aListener, nsACString& aHashKey) {
+    uintptr_t aListenerAddr, nsACString& aHashKey) {
   aHashKey.Assign(aHost);
   aHashKey.Assign(aTrrServer);
   aHashKey.AppendInt(aType);
@@ -71,7 +72,7 @@ void ChildDNSService::GetDNSRecordHashKey(
   aHashKey.Append(originSuffix);
 
   aHashKey.AppendInt(aFlags);
-  aHashKey.AppendPrintf("%p", aListener);
+  aHashKey.AppendPrintf("0x%" PRIxPTR, aListenerAddr);
 }
 
 nsresult ChildDNSService::AsyncResolveInternal(
@@ -92,7 +93,7 @@ nsresult ChildDNSService::AsyncResolveInternal(
   }
 
   // We need original listener for the pending requests hash.
-  nsIDNSListener* originalListener = listener;
+  uintptr_t originalListenerAddr = reinterpret_cast<uintptr_t>(listener);
 
   // make sure JS callers get notification on the main thread
   nsCOMPtr<nsIEventTarget> target = target_;
@@ -119,7 +120,7 @@ nsresult ChildDNSService::AsyncResolveInternal(
     MutexAutoLock lock(mPendingRequestsLock);
     nsCString key;
     GetDNSRecordHashKey(hostname, aTrrServer, type, aOriginAttributes, flags,
-                        originalListener, key);
+                        originalListenerAddr, key);
     auto entry = mPendingRequests.LookupForAdd(key);
     if (entry) {
       entry.Data()->AppendElement(sender);
@@ -149,8 +150,9 @@ nsresult ChildDNSService::CancelAsyncResolveInternal(
   MutexAutoLock lock(mPendingRequestsLock);
   nsTArray<RefPtr<DNSRequestSender>>* hashEntry;
   nsCString key;
+  uintptr_t listenerAddr = reinterpret_cast<uintptr_t>(aListener);
   GetDNSRecordHashKey(aHostname, aTrrServer, aType, aOriginAttributes, aFlags,
-                      aListener, key);
+                      listenerAddr, key);
   if (mPendingRequests.Get(key, &hashEntry)) {
     // We cancel just one.
     hashEntry->ElementAt(0)->Cancel(aReason);
@@ -417,14 +419,11 @@ ChildDNSService::GetMyHostName(nsACString& result) {
 void ChildDNSService::NotifyRequestDone(DNSRequestSender* aDnsRequest) {
   // We need the original flags and listener for the pending requests hash.
   uint32_t originalFlags = aDnsRequest->mFlags & ~RESOLVE_OFFLINE;
-  nsCOMPtr<nsIDNSListener> originalListener = aDnsRequest->mListener;
-  nsCOMPtr<nsIDNSListenerProxy> wrapper = do_QueryInterface(originalListener);
+  uintptr_t originalListenerAddr =
+      reinterpret_cast<uintptr_t>(aDnsRequest->mListener.get());
+  RefPtr<DNSListenerProxy> wrapper = do_QueryObject(aDnsRequest->mListener);
   if (wrapper) {
-    wrapper->GetOriginalListener(getter_AddRefs(originalListener));
-    if (NS_WARN_IF(!originalListener)) {
-      MOZ_ASSERT(originalListener);
-      return;
-    }
+    originalListenerAddr = wrapper->GetOriginalListenerAddress();
   }
 
   MutexAutoLock lock(mPendingRequestsLock);
@@ -432,7 +431,7 @@ void ChildDNSService::NotifyRequestDone(DNSRequestSender* aDnsRequest) {
   nsCString key;
   GetDNSRecordHashKey(aDnsRequest->mHost, aDnsRequest->mTrrServer,
                       aDnsRequest->mType, aDnsRequest->mOriginAttributes,
-                      originalFlags, originalListener, key);
+                      originalFlags, originalListenerAddr, key);
 
   nsTArray<RefPtr<DNSRequestSender>>* hashEntry;
 

netwerk/dns/ChildDNSService.h

@@ -41,7 +41,7 @@ class ChildDNSService final : public nsPIDNSService, public nsIObserver {
   void MOZ_ALWAYS_INLINE GetDNSRecordHashKey(
       const nsACString& aHost, const nsACString& aTrrServer, uint16_t aType,
       const OriginAttributes& aOriginAttributes, uint32_t aFlags,
-      nsIDNSListener* aListener, nsACString& aHashKey);
+      uintptr_t aListenerAddr, nsACString& aHashKey);
   nsresult AsyncResolveInternal(const nsACString& hostname,
                                 const nsACString& aTrrServer, uint16_t type,
                                 uint32_t flags, nsIDNSListener* listener,

netwerk/dns/DNSListenerProxy.cpp

@@ -10,26 +10,27 @@
 namespace mozilla {
 namespace net {
 
-NS_IMPL_ISUPPORTS(DNSListenerProxy, nsIDNSListener, nsIDNSListenerProxy)
+NS_IMPL_ADDREF(DNSListenerProxy)
+NS_IMPL_RELEASE(DNSListenerProxy)
+NS_INTERFACE_MAP_BEGIN(DNSListenerProxy)
+  NS_INTERFACE_MAP_ENTRY(nsIDNSListener)
+  NS_INTERFACE_MAP_ENTRY_CONCRETE(DNSListenerProxy)
+NS_INTERFACE_MAP_END
 
 NS_IMETHODIMP
 DNSListenerProxy::OnLookupComplete(nsICancelable* aRequest,
                                    nsIDNSRecord* aRecord, nsresult aStatus) {
-  RefPtr<OnLookupCompleteRunnable> r =
-      new OnLookupCompleteRunnable(mListener, aRequest, aRecord, aStatus);
-  return mTargetThread->Dispatch(r, NS_DISPATCH_NORMAL);
-}
-
-NS_IMETHODIMP
-DNSListenerProxy::OnLookupCompleteRunnable::Run() {
-  mListener->OnLookupComplete(mRequest, mRecord, mStatus);
-  return NS_OK;
-}
-
-NS_IMETHODIMP
-DNSListenerProxy::GetOriginalListener(nsIDNSListener** aOriginalListener) {
-  NS_IF_ADDREF(*aOriginalListener = mListener);
-  return NS_OK;
+  RefPtr<DNSListenerProxy> self = this;
+  nsCOMPtr<nsICancelable> request = aRequest;
+  nsCOMPtr<nsIDNSRecord> record = aRecord;
+  return mTargetThread->Dispatch(
+      NS_NewRunnableFunction("DNSListenerProxy::OnLookupComplete",
+                             [self, request, record, aStatus]() {
+                               Unused << self->mListener->OnLookupComplete(
+                                   request, record, aStatus);
+                               self->mListener = nullptr;
+                             }),
+      NS_DISPATCH_NORMAL);
 }
 
 }  // namespace net

netwerk/dns/DNSListenerProxy.h

@@ -18,50 +18,44 @@ class nsICancelable;
 namespace mozilla {
 namespace net {
 
-class DNSListenerProxy final : public nsIDNSListener,
-                               public nsIDNSListenerProxy {
+#define DNS_LISTENER_PROXY_IID                       \
+  {                                                  \
+    0x8f172ca3, 0x7a7f, 0x4941, {                    \
+      0xa7, 0x0b, 0xbc, 0x72, 0x80, 0x2e, 0x9d, 0x9b \
+    }                                                \
+  }
+
+class DNSListenerProxy final : public nsIDNSListener {
  public:
   DNSListenerProxy(nsIDNSListener* aListener, nsIEventTarget* aTargetThread)
-      // Sometimes aListener is a main-thread only object like XPCWrappedJS, and
-      // sometimes it's a threadsafe object like nsSOCKSSocketInfo. Use a main-
-      // thread pointer holder, but disable strict enforcement of thread
-      // invariants. The AddRef implementation of XPCWrappedJS will assert if we
-      // go wrong here.
-      : mListener(new nsMainThreadPtrHolder<nsIDNSListener>(
-            "DNSListenerProxy::mListener", aListener, false)),
-        mTargetThread(aTargetThread) {}
+      // We want to make sure that |aListener| is only accessed on the target
+      // thread.
+      : mListener(aListener),
+        mTargetThread(aTargetThread),
+        mListenerAddress(reinterpret_cast<uintptr_t>(aListener)) {
+    MOZ_ASSERT(mListener);
+    MOZ_ASSERT(mTargetThread);
+  }
 
   NS_DECL_THREADSAFE_ISUPPORTS
   NS_DECL_NSIDNSLISTENER
-  NS_DECL_NSIDNSLISTENERPROXY
+  NS_DECLARE_STATIC_IID_ACCESSOR(DNS_LISTENER_PROXY_IID)
 
-  class OnLookupCompleteRunnable : public Runnable {
-   public:
-    OnLookupCompleteRunnable(
-        const nsMainThreadPtrHandle<nsIDNSListener>& aListener,
-        nsICancelable* aRequest, nsIDNSRecord* aRecord, nsresult aStatus)
-        : Runnable("DNSListenerProxy::OnLookupCompleteRunnable"),
-          mListener(aListener),
-          mRequest(aRequest),
-          mRecord(aRecord),
-          mStatus(aStatus) {}
-
-    NS_DECL_NSIRUNNABLE
-
-   private:
-    nsMainThreadPtrHandle<nsIDNSListener> mListener;
-    nsCOMPtr<nsICancelable> mRequest;
-    nsCOMPtr<nsIDNSRecord> mRecord;
-    nsresult mStatus;
-  };
+  uintptr_t GetOriginalListenerAddress() const { return mListenerAddress; }
 
  private:
-  ~DNSListenerProxy() {}
+  ~DNSListenerProxy() {
+    NS_ProxyRelease("DNSListenerProxy::mListener", mTargetThread,
+                    mListener.forget());
+  }
 
-  nsMainThreadPtrHandle<nsIDNSListener> mListener;
+  nsCOMPtr<nsIDNSListener> mListener;
   nsCOMPtr<nsIEventTarget> mTargetThread;
+  uintptr_t mListenerAddress;
 };
 
+NS_DEFINE_STATIC_IID_ACCESSOR(DNSListenerProxy, DNS_LISTENER_PROXY_IID)
+
 }  // namespace net
 }  // namespace mozilla
 

netwerk/dns/nsDNSService2.cpp

@@ -437,13 +437,14 @@ void nsDNSAsyncRequest::OnResolveHostComplete(nsHostResolver* resolver,
 }
 
 bool nsDNSAsyncRequest::EqualsAsyncListener(nsIDNSListener* aListener) {
-  nsCOMPtr<nsIDNSListenerProxy> wrapper = do_QueryInterface(mListener);
+  uintptr_t originalListenerAddr = reinterpret_cast<uintptr_t>(mListener.get());
+  RefPtr<DNSListenerProxy> wrapper = do_QueryObject(mListener);
   if (wrapper) {
-    nsCOMPtr<nsIDNSListener> originalListener;
-    wrapper->GetOriginalListener(getter_AddRefs(originalListener));
-    return aListener == originalListener;
+    originalListenerAddr = wrapper->GetOriginalListenerAddress();
   }
-  return (aListener == mListener);
+
+  uintptr_t listenerAddr = reinterpret_cast<uintptr_t>(aListener);
+  return (listenerAddr == originalListenerAddr);
 }
 
 size_t nsDNSAsyncRequest::SizeOfIncludingThis(MallocSizeOf mallocSizeOf) const {

netwerk/dns/nsIDNSListener.idl

@@ -29,19 +29,3 @@ interface nsIDNSListener : nsISupports
                           in nsIDNSRecord  aRecord,
                           in nsresult      aStatus);
 };
-
-/**
- * nsIDNSListenerProxy:
- *
- * Must be implemented by classes that wrap the original listener passed to
- * nsIDNSService.AsyncResolve, so we have access to original listener for
- * comparison purposes.
- */
-[uuid(60eff0e4-6f7c-493c-add9-1cbea59063ad)]
-interface nsIDNSListenerProxy : nsISupports
-{
-  /*
-   * The original nsIDNSListener which requested hostname resolution.
-   */
-  readonly attribute nsIDNSListener originalListener;
-};