Bug 470334 - 'Workers: relative urls for XHR are incorrect'. r+sr=jst, a=blocking1.9+
authorBen Turner <bent.mozilla@gmail.com>
Tue, 30 Dec 2008 16:24:58 -0600
changeset 23201 8b659f81bab89b7cfdf60e2d1204a16251ded1d7
parent 23200 a04c4267c6c3d2dc2a850b2dc34dca980fcff11d
child 23202 1c68eb288c858e52c028007c6023d202c87e7244
push id4387
push userbturner@mozilla.com
push dateTue, 30 Dec 2008 22:25:50 +0000
treeherdermozilla-central@1c68eb288c85 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersblocking1.9
bugs470334
milestone1.9.2a1pre
Bug 470334 - 'Workers: relative urls for XHR are incorrect'. r+sr=jst, a=blocking1.9+
content/base/public/nsIXMLHttpRequest.idl
content/base/src/nsXMLHttpRequest.cpp
content/base/src/nsXMLHttpRequest.h
content/base/test/TestNativeXMLHttpRequest.cpp
content/xul/templates/src/nsXULTemplateQueryProcessorXML.cpp
dom/src/threads/nsDOMWorker.h
dom/src/threads/nsDOMWorkerPool.cpp
dom/src/threads/nsDOMWorkerPool.h
dom/src/threads/nsDOMWorkerXHR.cpp
dom/src/threads/nsDOMWorkerXHRProxy.cpp
dom/src/threads/test/relativeLoad_sub_worker.js
dom/src/threads/test/relativeLoad_worker.js
--- a/content/base/public/nsIXMLHttpRequest.idl
+++ b/content/base/public/nsIXMLHttpRequest.idl
@@ -37,16 +37,17 @@
 
 #include "nsIDOMEventTarget.idl"
 
 interface nsIChannel;
 interface nsIDOMDocument;
 interface nsIDOMEventListener;
 interface nsIPrincipal;
 interface nsIScriptContext;
+interface nsIURI;
 interface nsIVariant;
 interface nsPIDOMWindow;
 
 [scriptable, uuid(6ce0a193-b033-4c3d-b748-f851b09261f5)]
 interface nsIXMLHttpRequestEventTarget : nsIDOMEventTarget {
   // event handler attributes
   attribute nsIDOMEventListener onabort;
   attribute nsIDOMEventListener onerror;
@@ -96,17 +97,17 @@ interface nsIXMLHttpRequestUpload : nsIX
  *   The 'onload', 'onerror', and 'onreadystatechange' attributes moved to
  *   nsIJSXMLHttpRequest, but if you're coding in C++ you should avoid using
  *   those.
  *
  * Conclusion: Do not use event listeners on XMLHttpRequest from C++, unless
  * you're aware of all the security implications.  And then think twice about
  * it.
  */
-[scriptable, uuid(48ce10a0-c585-4e8f-a5f5-1ac1e47cc501)]
+[scriptable, uuid(ad78bf21-2227-447e-8ed5-824a017c265f)]
 interface nsIXMLHttpRequest : nsISupports
 {
   /**
    * The request uses a channel in order to perform the
    * request.  This attribute represents the channel used
    * for the request.  NULL if the channel has not yet been
    * created.
    *
@@ -335,20 +336,23 @@ interface nsIXMLHttpRequest : nsISupport
    * Initialize the object for use from C++ code with the principal, script
    * context, and owner window that should be used.
    *
    * @param principal The principal to use for the request. This must not be
    *                  null.
    * @param scriptContext The script context to use for the request. May be
    *                      null.
    * @param ownerWindow The associated window for the request. May be null.
+   * @param baseURI The base URI to use when resolving relative URIs. May be
+   *                null.
    */
   [noscript] void init(in nsIPrincipal principal,
                        in nsIScriptContext scriptContext,
-                       in nsPIDOMWindow ownerWindow);
+                       in nsPIDOMWindow ownerWindow,
+                       in nsIURI baseURI);
 
   /**
    * Upload process can be tracked by adding event listener to |upload|.
    */
   readonly attribute nsIXMLHttpRequestUpload upload;
 
   /**
    * Meant to be a script-only mechanism for setting a callback function.
--- a/content/base/src/nsXMLHttpRequest.cpp
+++ b/content/base/src/nsXMLHttpRequest.cpp
@@ -1103,32 +1103,34 @@ nsXMLHttpRequest::Init()
   return NS_OK;
 }
 /**
  * This Init method should only be called by C++ consumers.
  */
 NS_IMETHODIMP
 nsXMLHttpRequest::Init(nsIPrincipal* aPrincipal,
                        nsIScriptContext* aScriptContext,
-                       nsPIDOMWindow* aOwnerWindow)
+                       nsPIDOMWindow* aOwnerWindow,
+                       nsIURI* aBaseURI)
 {
   NS_ENSURE_ARG_POINTER(aPrincipal);
 
   // This object may have already been initialized in the other Init call above
   // if JS was on the stack. Clear the old values for mScriptContext and mOwner
   // if new ones are not supplied here.
 
   mPrincipal = aPrincipal;
   mScriptContext = aScriptContext;
   if (aOwnerWindow) {
     mOwner = aOwnerWindow->GetCurrentInnerWindow();
   }
   else {
     mOwner = nsnull;
   }
+  mBaseURI = aBaseURI;
 
   return NS_OK;
 }
 
 /**
  * This Initialize method is called from XPConnect via nsIJSNativeInitializer.
  */
 NS_IMETHODIMP
@@ -1821,17 +1823,20 @@ nsXMLHttpRequest::OpenRequest(const nsAC
     mState &= ~XML_HTTP_REQUEST_ASYNC;
   }
 
   mState &= ~XML_HTTP_REQUEST_MPART_HEADERS;
 
   nsCOMPtr<nsIDocument> doc = GetDocumentFromScriptContext(mScriptContext);
   
   nsCOMPtr<nsIURI> baseURI;
-  if (doc) {
+  if (mBaseURI) {
+    baseURI = mBaseURI;
+  }
+  else if (doc) {
     baseURI = doc->GetBaseURI();
   }
 
   rv = NS_NewURI(getter_AddRefs(uri), url, nsnull, baseURI);
   if (NS_FAILED(rv)) return rv;
 
   // mScriptContext should be initialized because of GetBaseURI() above.
   // Still need to consider the case that doc is nsnull however.
--- a/content/base/src/nsXMLHttpRequest.h
+++ b/content/base/src/nsXMLHttpRequest.h
@@ -473,16 +473,18 @@ protected:
    * want to also be notified for.  These are inited lazily if we're
    * asked for the relevant interface.
    */
   nsCOMPtr<nsIChannelEventSink> mChannelEventSink;
   nsCOMPtr<nsIProgressEventSink> mProgressEventSink;
 
   nsIRequestObserver* mRequestObserver;
 
+  nsCOMPtr<nsIURI> mBaseURI;
+
   PRUint32 mState;
 
   nsRefPtr<nsXMLHttpRequestUpload> mUpload;
   PRUint64 mUploadTransferred;
   PRUint64 mUploadTotal;
   PRPackedBool mUploadComplete;
   PRUint64 mUploadProgress; // For legacy
   PRUint64 mUploadProgressMax; // For legacy
--- a/content/base/test/TestNativeXMLHttpRequest.cpp
+++ b/content/base/test/TestNativeXMLHttpRequest.cpp
@@ -84,17 +84,17 @@ nsresult TestNativeXMLHttpRequest()
   nsCOMPtr<nsIScriptSecurityManager> secman =
     do_GetService(NS_SCRIPTSECURITYMANAGER_CONTRACTID, &rv);
   TEST_ENSURE_SUCCESS(rv, "Couldn't get script security manager!");
 
   nsCOMPtr<nsIPrincipal> systemPrincipal;
   rv = secman->GetSystemPrincipal(getter_AddRefs(systemPrincipal));
   TEST_ENSURE_SUCCESS(rv, "Couldn't get system principal!");
 
-  rv = xhr->Init(systemPrincipal, nsnull, nsnull);
+  rv = xhr->Init(systemPrincipal, nsnull, nsnull, nsnull);
   TEST_ENSURE_SUCCESS(rv, "Couldn't initialize the XHR!");
 
   rv = xhr->OpenRequest(getString, testURL, PR_FALSE, empty, empty);
   TEST_ENSURE_SUCCESS(rv, "OpenRequest failed!");
 
   rv = xhr->Send(nsnull);
   TEST_ENSURE_SUCCESS(rv, "Send failed!");
 
--- a/content/xul/templates/src/nsXULTemplateQueryProcessorXML.cpp
+++ b/content/xul/templates/src/nsXULTemplateQueryProcessorXML.cpp
@@ -186,17 +186,17 @@ nsXULTemplateQueryProcessorXML::GetDatas
     nsIScriptContext *context = scriptObject->GetContext();
     NS_ENSURE_TRUE(context, NS_OK);
 
     nsCOMPtr<nsIXMLHttpRequest> req =
         do_CreateInstance(NS_XMLHTTPREQUEST_CONTRACTID, &rv);
     NS_ENSURE_SUCCESS(rv, rv);
 
     nsCOMPtr<nsPIDOMWindow> owner = do_QueryInterface(scriptObject);
-    req->Init(docPrincipal, context, owner);
+    req->Init(docPrincipal, context, owner, nsnull);
 
     rv = req->OpenRequest(NS_LITERAL_CSTRING("GET"), uriStr, PR_TRUE,
                           EmptyString(), EmptyString());
     NS_ENSURE_SUCCESS(rv, rv);
 
     nsCOMPtr<nsIDOMEventTarget> target(do_QueryInterface(req));
     rv = target->AddEventListener(NS_LITERAL_STRING("load"), this, PR_FALSE);
     NS_ENSURE_SUCCESS(rv, rv);
--- a/dom/src/threads/nsDOMWorker.h
+++ b/dom/src/threads/nsDOMWorker.h
@@ -72,16 +72,17 @@ class nsDOMWorker : public nsIWorker,
 {
   friend class nsDOMWorkerFeature;
   friend class nsDOMWorkerFunctions;
   friend class nsDOMWorkerRefPtr;
   friend class nsDOMWorkerScope;
   friend class nsDOMWorkerScriptLoader;
   friend class nsDOMWorkerTimeout;
   friend class nsDOMWorkerXHR;
+  friend class nsDOMWorkerXHRProxy;
   friend class nsReportErrorRunnable;
 
   friend JSBool DOMWorkerOperationCallback(JSContext* aCx);
   friend void DOMWorkerErrorReporter(JSContext* aCx,
                                      const char* aMessage,
                                      JSErrorReport* aReport);
 
 #ifdef DEBUG
--- a/dom/src/threads/nsDOMWorkerPool.cpp
+++ b/dom/src/threads/nsDOMWorkerPool.cpp
@@ -38,17 +38,16 @@
  * ***** END LICENSE BLOCK ***** */
 
 #include "nsDOMWorkerPool.h"
 
 // Interfaces
 #include "nsIDocument.h"
 #include "nsIDOMClassInfo.h"
 #include "nsIJSContextStack.h"
-#include "nsIScriptContext.h"
 #include "nsIScriptGlobalObject.h"
 #include "nsIServiceManager.h"
 #include "nsIThreadManager.h"
 #include "nsIXPConnect.h"
 #include "nsPIDOMWindow.h"
 
 // Other includes
 #include "nsAutoLock.h"
@@ -226,17 +225,8 @@ nsDOMWorkerPool::Resume()
   if (count) {
     for (PRUint32 index = 0; index < count; index++) {
       workers[index]->Resume();
     }
     nsAutoMonitor mon(mMonitor);
     mon.NotifyAll();
   }
 }
-
-nsIScriptContext*
-nsDOMWorkerPool::ScriptContext()
-{
-  NS_ASSERTION(NS_IsMainThread(),
-               "Don't touch the non-threadsafe script context off the main "
-               "thread!");
-  return mParentGlobal->GetContext();
-}
--- a/dom/src/threads/nsDOMWorkerPool.h
+++ b/dom/src/threads/nsDOMWorkerPool.h
@@ -57,18 +57,16 @@ class nsDOMWorkerPool
 {
 public:
   nsDOMWorkerPool(nsIScriptGlobalObject* aGlobalObject,
                   nsIDocument* aDocument);
 
   NS_IMETHOD_(nsrefcnt) AddRef();
   NS_IMETHOD_(nsrefcnt) Release();
 
-  nsIScriptContext* ScriptContext();
-
   nsIScriptGlobalObject* ScriptGlobalObject() {
     return mParentGlobal;
   }
 
   nsIDocument* ParentDocument() {
     return mParentDocument;
   }
 
--- a/dom/src/threads/nsDOMWorkerXHR.cpp
+++ b/dom/src/threads/nsDOMWorkerXHR.cpp
@@ -813,17 +813,18 @@ nsDOMWorkerXHR::SetMozBackgroundRequest(
     return NS_ERROR_NOT_AVAILABLE;
   }
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsDOMWorkerXHR::Init(nsIPrincipal* aPrincipal,
                      nsIScriptContext* aScriptContext,
-                     nsPIDOMWindow* aOwnerWindow)
+                     nsPIDOMWindow* aOwnerWindow,
+                     nsIURI* aBaseURI)
 {
   NS_ASSERTION(!NS_IsMainThread(), "Wrong thread!");
   NS_NOTREACHED("No one should be calling this!");
   return NS_ERROR_NOT_IMPLEMENTED;
 }
 
 NS_IMETHODIMP
 nsDOMWorkerXHR::GetUpload(nsIXMLHttpRequestUpload** aUpload)
--- a/dom/src/threads/nsDOMWorkerXHRProxy.cpp
+++ b/dom/src/threads/nsDOMWorkerXHRProxy.cpp
@@ -399,23 +399,22 @@ nsDOMWorkerXHRProxy::InitInternal()
   nsDOMWorker* worker = mWorkerXHR->mWorker;
   nsRefPtr<nsDOMWorkerPool> pool = worker->Pool();
 
   if (worker->IsCanceled()) {
     return NS_ERROR_ABORT;
   }
 
   nsIPrincipal* nodePrincipal = pool->ParentDocument()->NodePrincipal();
-  nsIScriptContext* scriptContext = pool->ScriptContext();
-  NS_ASSERTION(nodePrincipal && scriptContext, "Shouldn't be null!");
 
   nsRefPtr<nsXMLHttpRequest> xhrConcrete = new nsXMLHttpRequest();
   NS_ENSURE_TRUE(xhrConcrete, NS_ERROR_OUT_OF_MEMORY);
 
-  nsresult rv = xhrConcrete->Init(nodePrincipal, scriptContext, nsnull);
+  nsresult rv = xhrConcrete->Init(nodePrincipal, nsnull, nsnull,
+                                  worker->GetURI());
   NS_ENSURE_SUCCESS(rv, rv);
 
   // Call QI manually here to avoid keeping up with the cast madness of
   // nsXMLHttpRequest.
   nsCOMPtr<nsIXMLHttpRequest> xhr =
     do_QueryInterface(static_cast<nsIXMLHttpRequest*>(xhrConcrete));
   NS_ENSURE_TRUE(xhr, NS_ERROR_NO_INTERFACE);
 
@@ -732,16 +731,20 @@ nsDOMWorkerXHRProxy::HandleEvent(nsIDOME
   PRBool isUpload = IsUploadEvent(aEvent);
 
   if ((isUpload && type >= MAX_UPLOAD_LISTENER_TYPE) ||
       (!isUpload && type >= MAX_XHR_LISTENER_TYPE)) {
     NS_ERROR("We shouldn't ever get a strange event from main thread XHR!");
     return NS_OK;
   }
 
+  // This will be filled if the request has completed and we're running in
+  // sync mode.
+  nsRefPtr<nsDOMWorkerXHRFinishSyncXHRRunnable> syncFinishedRunnable;
+
   PRBool requestDone;
   if (type == LISTENER_TYPE_ABORT || type == LISTENER_TYPE_ERROR ||
       type == LISTENER_TYPE_LOAD) {
     requestDone = PR_TRUE;
 
     nsAutoPtr<ProgressInfo>& progressInfo = isUpload ?
                                             mUploadProgressInfo :
                                             mDownloadProgressInfo;
@@ -755,18 +758,20 @@ nsDOMWorkerXHRProxy::HandleEvent(nsIDOME
       NS_ASSERTION(progressEvent, "Should always QI to nsIDOMProgressEvent!");
       if (progressEvent) {
         progressEvent->GetLengthComputable(&progressInfo->computable);
         progressEvent->GetLoaded(&progressInfo->loaded);
         progressEvent->GetTotal(&progressInfo->total);
       }
     }
 
-    // Dummy memory barrier.
+    NS_ASSERTION(!syncFinishedRunnable, "This shouldn't be set!");
+
     nsAutoLock lock(mWorkerXHR->Lock());
+    mSyncFinishedRunnable.swap(syncFinishedRunnable);
   }
   else {
     requestDone = PR_FALSE;
   }
 
   if (mCanceled) {
     // When Abort is called on nsXMLHttpRequest (either from a proxied Abort
     // call or from DestroyInternal) the OnStopRequest call is not run
@@ -813,17 +818,23 @@ nsDOMWorkerXHRProxy::HandleEvent(nsIDOME
       if (newEvent) {
         // Already had a saved progress/load event so no need to generate
         // another. Bail out rather than dispatching runnable.
         return NS_OK;
       }
     }
   }
 
-  return HandleEventRunnable(runnable);
+  rv = HandleEventRunnable(runnable);
+
+  if (syncFinishedRunnable) {
+    syncFinishedRunnable->Dispatch();
+  }
+
+  return rv;
 }
 
 nsresult
 nsDOMWorkerXHRProxy::HandleEventRunnable(nsIRunnable* aRunnable)
 {
   NS_ASSERTION(aRunnable, "Null pointer!");
 
   nsresult rv;
@@ -1157,21 +1168,10 @@ nsDOMWorkerXHRProxy::OnStopRequest(nsIRe
                                    nsresult /* aStatus */)
 {
   NS_ASSERTION(NS_IsMainThread(), "Wrong thread!");
 
   NS_ASSERTION(mOwnedByXHR, "Inconsistent state!");
 
   FlipOwnership();
 
-  nsRefPtr<nsDOMWorkerXHRFinishSyncXHRRunnable> syncFinishedRunnable;
-  {
-    nsAutoLock lock(mWorkerXHR->Lock());
-    mSyncFinishedRunnable.swap(syncFinishedRunnable);
-  }
-
-  if (syncFinishedRunnable) {
-    nsresult rv = syncFinishedRunnable->Dispatch();
-    NS_ENSURE_SUCCESS(rv, rv);
-  }
-
   return NS_OK;
 }
--- a/dom/src/threads/test/relativeLoad_sub_worker.js
+++ b/dom/src/threads/test/relativeLoad_sub_worker.js
@@ -1,11 +1,18 @@
 const importSubURL = "relativeLoad_sub_import.js";
 
 onmessage = function(event) {
+  var xhr = new XMLHttpRequest();
+  xhr.open("GET", "testXHR.txt", false);
+  xhr.send(null);
+  if (xhr.status != 404) {
+    throw "Loaded an xhr from the wrong location!";
+  }
+
   importScripts(importSubURL);
   var worker = new Worker("relativeLoad_sub_worker2.js");
   worker.onerror = function(event) {
     throw event.data;
   };
   worker.onmessage = function(event) {
     if (event.data != workerSubURL) {
       throw "Bad data!";
--- a/dom/src/threads/test/relativeLoad_worker.js
+++ b/dom/src/threads/test/relativeLoad_worker.js
@@ -1,11 +1,20 @@
 const importURL = "relativeLoad_import.js";
 
 onmessage = function(event) {
+  var xhr = new XMLHttpRequest();
+  xhr.open("GET", "testXHR.txt", false);
+  xhr.send(null);
+  dump("XXXben: " + xhr.responseText + "\n");
+  if (xhr.status != 200 ||
+      xhr.responseText != "A noisy noise annoys an oyster.") {
+    throw "Couldn't get xhr text from where we wanted it!";
+  }
+
   importScripts(importURL);
   var worker = new Worker("relativeLoad_worker2.js");
   worker.onerror = function(event) {
     throw event.data;
   };
   worker.onmessage = function(event) {
     if (event.data != workerURL) {
       throw "Bad data!";