Bug 882200 nsLDAPConnection should only addref the message callback on the main thread r=Standard8
authorNeil Rashbrook <neil@parkwaycc.co.uk>
Mon, 24 Jun 2013 19:26:07 +0100
changeset 12648 67e02a3c68e1c1820e93813fd030061b6321132e
parent 12647 9311b3990406e1c5e580e9d46659395efbb5dc3c
child 12649 9ff0c7ce1ea245e1ed3d9a2217673b0df5ad95aa
push id9286
push userneil@parkwaycc.co.uk
push dateMon, 24 Jun 2013 18:26:18 +0000
treeherdercomm-central@67e02a3c68e1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersStandard8
bugs882200
Bug 882200 nsLDAPConnection should only addref the message callback on the main thread r=Standard8
ldap/xpcom/src/nsLDAPConnection.cpp
ldap/xpcom/src/nsLDAPMessage.h
--- a/ldap/xpcom/src/nsLDAPConnection.cpp
+++ b/ldap/xpcom/src/nsLDAPConnection.cpp
@@ -375,106 +375,79 @@ nsLDAPConnection::RemovePendingOperation
           mPendingOperations.Count()));
 
   return NS_OK;
 }
 
 class nsOnLDAPMessageRunnable : public nsRunnable
 {
 public:
-  nsOnLDAPMessageRunnable(nsILDAPMessageListener *aListener,
-                          nsILDAPMessage *aMsg);
+  nsOnLDAPMessageRunnable(nsLDAPMessage *aMsg, bool aClear)
+    : m_msg(aMsg)
+    , m_clear(aClear)
+  {}
   NS_DECL_NSIRUNNABLE
 private:
-  nsCOMPtr<nsILDAPMessage> m_msg;
-  nsCOMPtr<nsILDAPMessageListener> m_listener;
+  nsRefPtr<nsLDAPMessage> m_msg;
+  bool m_clear;
 };
 
-nsOnLDAPMessageRunnable::nsOnLDAPMessageRunnable(nsILDAPMessageListener *aListener,
-                                                 nsILDAPMessage *aMsg) :
-  m_msg(aMsg), m_listener(aListener)
-{
-}
-
 NS_IMETHODIMP nsOnLDAPMessageRunnable::Run()
 {
-  return m_listener->OnLDAPMessage(m_msg);
-}
+  // get the message listener object.
+  nsLDAPOperation *nsoperation = static_cast<nsLDAPOperation *>(m_msg->mOperation.get());
+  nsCOMPtr<nsILDAPMessageListener> listener;
+  nsresult rv = nsoperation->GetMessageListener(getter_AddRefs(listener));
 
-class nsOnLDAPInitMessageRunnable : public nsRunnable
-{
-public:
-  nsOnLDAPInitMessageRunnable(nsILDAPMessageListener *aListener,
-                              nsILDAPConnection *aConn,
-                              nsresult aStatus);
-  NS_DECL_NSIRUNNABLE
-private:
-  nsCOMPtr<nsILDAPConnection> m_conn;
-  nsCOMPtr<nsILDAPMessageListener> m_listener;
-  nsresult m_status;
-};
+  if (m_clear)
+  {
+    // try to break cycles
+    nsoperation->Clear();
+  }
 
-nsOnLDAPInitMessageRunnable::nsOnLDAPInitMessageRunnable(nsILDAPMessageListener *aListener,
-                                                         nsILDAPConnection *aConn,
-                                                         nsresult aStatus) :
-  m_listener(aListener), m_conn(aConn), m_status(aStatus)
-{
-}
+  if (!listener)
+  {
+    NS_ERROR("nsLDAPConnection::InvokeMessageCallback(): probable "
+             "memory corruption: GetMessageListener() returned nullptr");
+    return rv;
+  }
 
-NS_IMETHODIMP nsOnLDAPInitMessageRunnable::Run()
-{
-  return m_listener->OnLDAPInit(m_conn, m_status);
+  return listener->OnLDAPMessage(m_msg);
 }
 
 nsresult
 nsLDAPConnection::InvokeMessageCallback(LDAPMessage *aMsgHandle,
                                         nsILDAPMessage *aMsg,
                                         int32_t aOperation,
                                         bool aRemoveOpFromConnQ)
 {
 #if defined(DEBUG)
   // We only want this being logged for debug builds so as not to affect performance too much.
   PR_LOG(gLDAPLogModule, PR_LOG_DEBUG, ("InvokeMessageCallback entered\n"));
 #endif
 
-  nsresult rv;
   // Get the operation.
   nsCOMPtr<nsILDAPOperation> operation;
   mPendingOperations.Get((uint32_t)aOperation, getter_AddRefs(operation));
 
   NS_ENSURE_TRUE(operation, NS_ERROR_NULL_POINTER);
 
-  static_cast<nsLDAPMessage *>(aMsg)->mOperation = operation;
+  nsLDAPMessage *msg = static_cast<nsLDAPMessage *>(aMsg);
+  msg->mOperation = operation;
 
-  // get the message listener object.
-  nsCOMPtr<nsILDAPMessageListener> listener;
-  rv = operation->GetMessageListener(getter_AddRefs(listener));
-  if (NS_FAILED(rv))
-  {
-    NS_ERROR("nsLDAPConnection::InvokeMessageCallback(): probable "
-             "memory corruption: GetMessageListener() returned error");
-    return NS_ERROR_UNEXPECTED;
-  }
   // proxy the listener callback to the ui thread.
-  if (listener)
-  {
-    nsRefPtr<nsOnLDAPMessageRunnable> runnable =
-      new nsOnLDAPMessageRunnable(listener, aMsg);
-    // invoke the callback
-    NS_DispatchToMainThread(runnable);
-  }
+  nsRefPtr<nsOnLDAPMessageRunnable> runnable =
+    new nsOnLDAPMessageRunnable(msg, aRemoveOpFromConnQ);
+  // invoke the callback
+  NS_DispatchToMainThread(runnable);
 
   // if requested (ie the operation is done), remove the operation
   // from the connection queue.
   if (aRemoveOpFromConnQ)
   {
-    // try to break cycles
-    nsLDAPOperation* nsoperation = static_cast<nsLDAPOperation *>(operation.get());
-    if (nsoperation)
-      nsoperation->Clear();
     mPendingOperations.Remove(aOperation);
 
     PR_LOG(gLDAPLogModule, PR_LOG_DEBUG,
            ("pending operation removed; total pending operations now ="
             " %d\n", mPendingOperations.Count()));
   }
 
   return NS_OK;
--- a/ldap/xpcom/src/nsLDAPMessage.h
+++ b/ldap/xpcom/src/nsLDAPMessage.h
@@ -18,16 +18,17 @@
 { 0x76e061ad, 0xa59f, 0x43b6, \
   { 0xb8, 0x12, 0xee, 0x6e, 0x8e, 0x69, 0x42, 0x3f }}
 
 class nsLDAPMessage : public nsILDAPMessage
 {
     friend class nsLDAPOperation;
     friend class nsLDAPConnection;
     friend class nsLDAPConnectionRunnable;
+    friend class nsOnLDAPMessageRunnable;
 
   public:
 
     NS_DECL_ISUPPORTS
     NS_DECL_NSILDAPMESSAGE
 
     // constructor & destructor
     //