Bug 458322 - 'Workers: Don't run queued XHR events if Abort/Open have since been called'. r+sr=sicking.
authorBen Turner <bent.mozilla@gmail.com>
Tue, 14 Oct 2008 11:16:37 -0700
changeset 20475 5f4b2c8f89cc83c26995a6e25d5bfbcf0a91aaf4
parent 20474 f0804610995bc53ce6f7dcee8657cd7b097e07fb
child 20476 240adb221de097d99d827a53d463cf1e2de76aa4
push id2894
push userbturner@mozilla.com
push dateTue, 14 Oct 2008 18:19:37 +0000
treeherderautoland@a043e83d291f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs458322
milestone1.9.1b2pre
Bug 458322 - 'Workers: Don't run queued XHR events if Abort/Open have since been called'. r+sr=sicking.
dom/src/threads/nsDOMWorkerXHRProxiedFunctions.h
dom/src/threads/nsDOMWorkerXHRProxy.cpp
dom/src/threads/nsDOMWorkerXHRProxy.h
--- a/dom/src/threads/nsDOMWorkerXHRProxiedFunctions.h
+++ b/dom/src/threads/nsDOMWorkerXHRProxiedFunctions.h
@@ -91,88 +91,16 @@
       } \
       return NS_OK; \
     } \
   private: \
     _arg1 mArg1; \
     _arg2 mArg2; \
   }
 
-#define MAKE_PROXIED_FUNCTION3(_name, _arg1, _arg2, _arg3) \
-  class _name : public SyncEventCapturingRunnable \
-  { \
-  public: \
-    _name (nsDOMWorkerXHRProxy* aXHR, SyncEventQueue* aQueue, _arg1 aArg1, \
-           _arg2 aArg2, _arg3 aArg3) \
-    : SyncEventCapturingRunnable(aXHR, aQueue), mArg1(aArg1), mArg2(aArg2), \
-      mArg3(aArg3) { } \
-  \
-    virtual nsresult RunInternal() \
-    { \
-      nsCOMPtr<nsIXMLHttpRequest> xhr = mXHR->GetXMLHttpRequest(); \
-      if (xhr) { \
-        return xhr-> _name (mArg1, mArg2, mArg3); \
-      } \
-      return NS_OK; \
-    } \
-  private: \
-    _arg1 mArg1; \
-    _arg2 mArg2; \
-    _arg3 mArg3; \
-  }
-
-#define MAKE_PROXIED_FUNCTION4(_name, _arg1, _arg2, _arg3, _arg4) \
-  class _name : public SyncEventCapturingRunnable \
-  { \
-  public: \
-    _name (nsDOMWorkerXHRProxy* aXHR, SyncEventQueue* aQueue, _arg1 aArg1, \
-           _arg2 aArg2, _arg3 aArg3, _arg4 aArg4) \
-    : SyncEventCapturingRunnable(aXHR, aQueue), mArg1(aArg1), mArg2(aArg2), \
-      mArg3(aArg3), mArg4(aArg4) { } \
-  \
-    virtual nsresult RunInternal() \
-    { \
-      nsCOMPtr<nsIXMLHttpRequest> xhr = mXHR->GetXMLHttpRequest(); \
-      if (xhr) { \
-        return xhr-> _name (mArg1, mArg2, mArg3, mArg4); \
-      } \
-      return NS_OK; \
-    } \
-  private: \
-    _arg1 mArg1; \
-    _arg2 mArg2; \
-    _arg3 mArg3; \
-    _arg4 mArg4; \
-  }
-
-#define MAKE_PROXIED_FUNCTION5(_name, _arg1, _arg2, _arg3, _arg4, _arg5) \
-  class _name : public SyncEventCapturingRunnable \
-  { \
-  public: \
-    _name (nsDOMWorkerXHRProxy* aXHR, SyncEventQueue* aQueue, _arg1 aArg1, \
-           _arg2 aArg2, _arg3 aArg3, _arg4 aArg4, _arg5 aArg5) \
-    : SyncEventCapturingRunnable(aXHR, aQueue), mArg1(aArg1), mArg2(aArg2), \
-      mArg3(aArg3), mArg4(aArg4), mArg5(aArg5) { } \
-  \
-    virtual nsresult RunInternal() \
-    { \
-      nsCOMPtr<nsIXMLHttpRequest> xhr = mXHR->GetXMLHttpRequest(); \
-      if (xhr) { \
-        return xhr-> _name (mArg1, mArg2, mArg3, mArg4, mArg5); \
-      } \
-      return NS_OK; \
-    } \
-  private: \
-    _arg1 mArg1; \
-    _arg2 mArg2; \
-    _arg3 mArg3; \
-    _arg4 mArg4; \
-    _arg5 mArg5; \
-  }
-
 #define RUN_PROXIED_FUNCTION(_name, _args) \
   PR_BEGIN_MACRO \
     NS_ASSERTION(!NS_IsMainThread(), "Wrong thread!"); \
     \
     if (mCanceled) { \
       return NS_ERROR_ABORT; \
     } \
     SyncEventQueue queue; \
@@ -233,23 +161,42 @@ namespace nsDOMWorkerProxiedXHRFunctions
     Abort (nsDOMWorkerXHRProxy* aXHR, SyncEventQueue* aQueue)
     : SyncEventCapturingRunnable(aXHR, aQueue) { }
 
     virtual nsresult RunInternal() {
       return mXHR->Abort();
     }
   };
 
+  class OpenRequest : public SyncEventCapturingRunnable
+  {
+  public:
+    OpenRequest(nsDOMWorkerXHRProxy* aXHR, SyncEventQueue* aQueue,
+                const nsACString& aMethod, const nsACString& aUrl,
+                PRBool aAsync, const nsAString& aUser,
+                const nsAString& aPassword)
+    : SyncEventCapturingRunnable(aXHR, aQueue), mMethod(aMethod), mUrl(aUrl),
+      mAsync(aAsync), mUser(aUser), mPassword(aPassword) { }
+  
+    virtual nsresult RunInternal() {
+      return mXHR->OpenRequest(mMethod, mUrl, mAsync, mUser, mPassword);
+    }
+
+  private:
+    nsCString mMethod;
+    nsCString mUrl;
+    PRBool mAsync;
+    nsString mUser;
+    nsString mPassword;
+  };
+
   MAKE_PROXIED_FUNCTION1(GetAllResponseHeaders, char**);
 
   MAKE_PROXIED_FUNCTION2(GetResponseHeader, const nsACString&, nsACString&);
 
-  MAKE_PROXIED_FUNCTION5(OpenRequest, const nsACString&, const nsACString&,
-                         PRBool, const nsAString&, const nsAString&);
-
   MAKE_PROXIED_FUNCTION1(Send, nsIVariant*);
 
   MAKE_PROXIED_FUNCTION1(SendAsBinary, const nsAString&);
 
   MAKE_PROXIED_FUNCTION2(SetRequestHeader, const nsACString&,
                          const nsACString&);
 
   MAKE_PROXIED_FUNCTION1(OverrideMimeType, const nsACString&);
--- a/dom/src/threads/nsDOMWorkerXHRProxy.cpp
+++ b/dom/src/threads/nsDOMWorkerXHRProxy.cpp
@@ -151,32 +151,34 @@ protected:
   PRUint16 mEventPhase;
   DOMTimeStamp mTimeStamp;
   nsString mResponseText;
   nsCString mStatusText;
   nsresult mStatus;
   PRInt32 mReadyState;
   PRUint32 mLoaded;
   PRUint32 mTotal;
+  PRInt32 mChannelID;
   PRPackedBool mBubbles;
   PRPackedBool mCancelable;
   PRPackedBool mUploadEvent;
   PRPackedBool mProgressEvent;
   PRPackedBool mLengthComputable;
 };
 
 nsDOMWorkerXHREvent::nsDOMWorkerXHREvent(nsDOMWorkerXHRProxy* aXHRProxy)
 : mXHRProxy(aXHRProxy),
   mType(PR_UINT32_MAX),
   mEventPhase(0),
   mTimeStamp(0),
   mStatus(NS_OK),
   mReadyState(0),
   mLoaded(0),
   mTotal(0),
+  mChannelID(-1),
   mBubbles(PR_FALSE),
   mCancelable(PR_FALSE),
   mUploadEvent(PR_FALSE),
   mProgressEvent(PR_FALSE),
   mLengthComputable(PR_FALSE)
 {
   NS_ASSERTION(aXHRProxy, "Can't be null!");
 }
@@ -196,16 +198,18 @@ NS_IMPL_CI_INTERFACE_GETTER1(nsDOMWorker
 NS_IMPL_THREADSAFE_DOM_CI(nsDOMWorkerXHREvent)
 
 nsresult
 nsDOMWorkerXHREvent::Init(nsIDOMEvent* aEvent)
 {
   NS_ASSERTION(NS_IsMainThread(), "Wrong thread!");
   NS_ASSERTION(aEvent, "Don't pass null here!");
 
+  mChannelID = mXHRProxy->ChannelID();
+
   nsresult rv = aEvent->GetType(mTypeString);
   NS_ENSURE_SUCCESS(rv, rv);
 
   mType = nsDOMWorkerXHREventTarget::GetListenerTypeFromString(mTypeString);
   if (mType >= MAX_XHR_LISTENER_TYPE) {
     NS_ERROR("Shouldn't get this type of event!");
     return NS_ERROR_INVALID_ARG;
   }
@@ -438,16 +442,17 @@ NS_IMPL_THREADSAFE_ISUPPORTS1(nsDOMWorke
                               nsIDOMEventListener)
 
 nsDOMWorkerXHRProxy::nsDOMWorkerXHRProxy(nsDOMWorkerXHR* aWorkerXHR)
 : mWorkerXHR(aWorkerXHR),
   mXHR(nsnull),
   mConcreteXHR(nsnull),
   mUpload(nsnull),
   mSyncEventQueue(nsnull),
+  mChannelID(-1),
   mOwnedByXHR(PR_FALSE),
   mCanceled(PR_FALSE)
 {
   NS_ASSERTION(!NS_IsMainThread(), "Wrong thread!");
   NS_ASSERTION(MAX_XHR_LISTENER_TYPE >= MAX_UPLOAD_LISTENER_TYPE,
                "Upload should support a subset of XHR's event types!");
 }
 
@@ -762,16 +767,20 @@ nsDOMWorkerXHRProxy::GetOnXListener(PRUi
 
 nsresult
 nsDOMWorkerXHRProxy::HandleWorkerEvent(nsDOMWorkerXHREvent* aEvent,
                                        PRBool aUploadEvent)
 {
   NS_ASSERTION(!NS_IsMainThread(), "Wrong thread!");
   NS_ASSERTION(aEvent, "Should not be null!");
 
+  if (aEvent->mChannelID != -1 && aEvent->mChannelID != mChannelID) {
+    return NS_OK;
+  }
+
   mLastXHREvent = aEvent;
 
   nsresult rv = HandleEventInternal(aEvent->mType, aEvent, aUploadEvent);
   NS_ENSURE_SUCCESS(rv, rv);
 
   return NS_OK;
 }
 
@@ -909,45 +918,82 @@ nsDOMWorkerXHRProxy::HandleEvent(nsIDOME
 
   nsresult rv = newEvent->Init(aEvent);
   NS_ENSURE_SUCCESS(rv, rv);
 
   // If we're supposed to be capturing events for synchronous execution then
   // place this event in the queue. Otherwise schedule it for the worker via
   // the thread service.
   if (mSyncEventQueue) {
+    // Always run this event!
+    newEvent->mChannelID = -1;
+
     nsCOMPtr<nsIRunnable>* newElement =
       mSyncEventQueue->AppendElement(newEvent);
     NS_ENSURE_TRUE(newElement, NS_ERROR_OUT_OF_MEMORY);
   }
   else {
     rv = nsDOMThreadService::get()->Dispatch(mWorkerXHR->mWorker, newEvent);
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
   return NS_OK;
 }
 
 nsresult
 nsDOMWorkerXHRProxy::Abort()
 {
-  if (NS_IsMainThread()) {
-    if (mCanceled) {
-      return NS_ERROR_ABORT;
-    }
+  if (!NS_IsMainThread()) {
+    RUN_PROXIED_FUNCTION(Abort, (this, &queue));
+    return NS_OK;
+  }
 
-    nsCOMPtr<nsIXMLHttpRequest> xhr = mXHR;
-    if (mOwnedByXHR) {
-      FlipOwnership();
-    }
+  if (mCanceled) {
+    return NS_ERROR_ABORT;
+  }
 
-    return xhr->Abort();
+  nsCOMPtr<nsIXMLHttpRequest> xhr = mXHR;
+  if (mOwnedByXHR) {
+    FlipOwnership();
   }
 
-  RUN_PROXIED_FUNCTION(Abort, (this, &queue));
+  nsresult rv = xhr->Abort();
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  // Don't allow further events from this channel.
+  mChannelID++;
+
+  return NS_OK;
+}
+
+nsresult
+nsDOMWorkerXHRProxy::OpenRequest(const nsACString& aMethod,
+                                 const nsACString& aUrl,
+                                 PRBool aAsync,
+                                 const nsAString& aUser,
+                                 const nsAString& aPassword)
+{
+  if (!NS_IsMainThread()) {
+    RUN_PROXIED_FUNCTION(OpenRequest, (this, &queue, aMethod, aUrl, aAsync,
+                                       aUser, aPassword));
+    return NS_OK;
+  }
+
+  if (mCanceled) {
+    return NS_ERROR_ABORT;
+  }
+
+  nsresult rv = mXHR->OpenRequest(aMethod, aUrl, aAsync, aUser, aPassword);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  // Do this after OpenRequest is called so that we will continue to run events
+  // from the old channel if OpenRequest fails. Any events generated by the
+  // OpenRequest method will always run regardless of channel ID.
+  mChannelID++;
+
   return NS_OK;
 }
 
 nsDOMWorkerXHRProxy::SyncEventQueue*
 nsDOMWorkerXHRProxy::SetSyncEventQueue(SyncEventQueue* aQueue)
 {
   NS_ASSERTION(NS_IsMainThread(), "Wrong thread!");
   SyncEventQueue* oldQueue = mSyncEventQueue;
@@ -966,28 +1012,16 @@ nsresult
 nsDOMWorkerXHRProxy::GetResponseHeader(const nsACString& aHeader,
                                        nsACString& _retval)
 {
   RUN_PROXIED_FUNCTION(GetResponseHeader, (this, &queue, aHeader, _retval));
   return NS_OK;
 }
 
 nsresult
-nsDOMWorkerXHRProxy::OpenRequest(const nsACString& aMethod,
-                                 const nsACString& aUrl,
-                                 PRBool aAsync,
-                                 const nsAString& aUser,
-                                 const nsAString& aPassword)
-{
-  RUN_PROXIED_FUNCTION(OpenRequest, (this, &queue, aMethod, aUrl, aAsync, aUser,
-                                     aPassword));
-  return NS_OK;
-}
-
-nsresult
 nsDOMWorkerXHRProxy::Send(nsIVariant* aBody)
 {
   RUN_PROXIED_FUNCTION(Send, (this, &queue, aBody));
   return NS_OK;
 }
 
 nsresult
 nsDOMWorkerXHRProxy::SendAsBinary(const nsAString& aBody)
--- a/dom/src/threads/nsDOMWorkerXHRProxy.h
+++ b/dom/src/threads/nsDOMWorkerXHRProxy.h
@@ -89,18 +89,28 @@ public:
   virtual ~nsDOMWorkerXHRProxy();
 
   nsresult Init();
 
   nsIXMLHttpRequest* GetXMLHttpRequest();
 
   nsresult Abort();
 
+  nsresult OpenRequest(const nsACString& aMethod,
+                       const nsACString& aUrl,
+                       PRBool aAsync,
+                       const nsAString& aUser,
+                       const nsAString& aPassword);
+
   SyncEventQueue* SetSyncEventQueue(SyncEventQueue* aQueue);
 
+  PRInt32 ChannelID() {
+    return mChannelID;
+  }
+
 protected:
   nsresult InitInternal();
   void DestroyInternal();
 
   nsresult Destroy();
 
   void FlipOwnership();
 
@@ -125,21 +135,16 @@ protected:
                                PRBool aUploadEvent);
 
   void ClearEventListeners();
 
   // Methods of nsIXMLHttpRequest that we implement
   nsresult GetAllResponseHeaders(char** _retval);
   nsresult GetResponseHeader(const nsACString& aHeader,
                              nsACString& _retval);
-  nsresult OpenRequest(const nsACString& aMethod,
-                       const nsACString& aUrl,
-                       PRBool aAsync,
-                       const nsAString& aUser,
-                       const nsAString& aPassword);
   nsresult Send(nsIVariant* aBody);
   nsresult SendAsBinary(const nsAString& aBody);
   nsresult GetResponseText(nsAString& _retval);
   nsresult GetStatusText(nsACString& _retval);
   nsresult GetStatus(nsresult* _retval);
   nsresult GetReadyState(PRInt32* _retval);
   nsresult SetRequestHeader(const nsACString& aHeader,
                             const nsACString& aValue);
@@ -164,15 +169,17 @@ protected:
   nsTArray<ListenerArray> mXHRListeners;
   nsTArray<WrappedListener> mXHROnXListeners;
 
   nsTArray<ListenerArray> mUploadListeners;
   nsTArray<WrappedListener> mUploadOnXListeners;
 
   SyncEventQueue* mSyncEventQueue;
 
+  PRInt32 mChannelID;
+
   // Whether or not this object is owned by the real XHR object.
   PRPackedBool mOwnedByXHR;
 
   PRPackedBool mCanceled;
 };
 
 #endif /* __NSDOMWORKERXHRPROXY_H__ */