Bug 1286955. Don't use an ErrorResult on multiple threads in the Worker URL implementation. r=bzbarsky
☠☠ backed out by d12f1688d471 ☠ ☠
authorAndrea Marchesini <amarchesini@mozilla.com>
Fri, 15 Jul 2016 22:35:12 -0400
changeset 330252 cf2bb72412a04c4c67aac4b42762464d86fab642
parent 330251 db8491cc618f73adf1778a0209877edcbebd4452
child 330253 03362dd7616d404ea77da256bf5a7245163cc9c9
push id9858
push userjlund@mozilla.com
push dateMon, 01 Aug 2016 14:37:10 +0000
treeherdermozilla-aurora@203106ef6cb6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbzbarsky
bugs1286955
milestone50.0a1
Bug 1286955. Don't use an ErrorResult on multiple threads in the Worker URL implementation. r=bzbarsky
dom/url/URL.cpp
--- a/dom/url/URL.cpp
+++ b/dom/url/URL.cpp
@@ -917,75 +917,79 @@ public:
 // This class creates a URL object on the main thread.
 class ConstructorRunnable : public WorkerMainThreadRunnable
 {
 private:
   const nsString mURL;
 
   nsString mBase; // IsVoid() if we have no base URI string.
   RefPtr<URLProxy> mBaseProxy;
-  ErrorResult& mRv;
 
   RefPtr<URLProxy> mRetval;
 
 public:
   ConstructorRunnable(WorkerPrivate* aWorkerPrivate,
-                      const nsAString& aURL, const Optional<nsAString>& aBase,
-                      ErrorResult& aRv)
+                      const nsAString& aURL, const Optional<nsAString>& aBase)
   : WorkerMainThreadRunnable(aWorkerPrivate,
                              NS_LITERAL_CSTRING("URL :: Constructor"))
   , mURL(aURL)
-  , mRv(aRv)
   {
     if (aBase.WasPassed()) {
       mBase = aBase.Value();
     } else {
       mBase.SetIsVoid(true);
     }
     mWorkerPrivate->AssertIsOnWorkerThread();
   }
 
   ConstructorRunnable(WorkerPrivate* aWorkerPrivate,
-                      const nsAString& aURL, URLProxy* aBaseProxy,
-                      ErrorResult& aRv)
+                      const nsAString& aURL, URLProxy* aBaseProxy)
   : WorkerMainThreadRunnable(aWorkerPrivate,
                              NS_LITERAL_CSTRING("URL :: Constructor with BaseURL"))
   , mURL(aURL)
   , mBaseProxy(aBaseProxy)
-  , mRv(aRv)
   {
     mBase.SetIsVoid(true);
     mWorkerPrivate->AssertIsOnWorkerThread();
   }
 
   bool
   MainThreadRun()
   {
     AssertIsOnMainThread();
 
+    ErrorResult rv;
     RefPtr<URLMainThread> url;
     if (mBaseProxy) {
-      url = URLMainThread::Constructor(nullptr, mURL, mBaseProxy->URI(), mRv);
+      url = URLMainThread::Constructor(nullptr, mURL, mBaseProxy->URI(), rv);
     } else if (!mBase.IsVoid()) {
-      url = URLMainThread::Constructor(nullptr, mURL, mBase, mRv);
+      url = URLMainThread::Constructor(nullptr, mURL, mBase, rv);
     } else {
-      url = URLMainThread::Constructor(nullptr, mURL, nullptr, mRv);
+      url = URLMainThread::Constructor(nullptr, mURL, nullptr, rv);
     }
 
-    if (mRv.Failed()) {
+    if (rv.Failed()) {
+      rv.SuppressException();
       return true;
     }
 
     mRetval = new URLProxy(url.forget());
     return true;
   }
 
   URLProxy*
-  GetURLProxy()
+  GetURLProxy(ErrorResult& aRv) const
   {
+    MOZ_ASSERT(mWorkerPrivate);
+    mWorkerPrivate->AssertIsOnWorkerThread();
+
+    if (!mRetval) {
+      aRv.ThrowTypeError<MSG_INVALID_URL>(mURL);
+    }
+
     return mRetval;
   }
 };
 
 class TeardownURLRunnable : public Runnable
 {
 public:
   explicit TeardownURLRunnable(URLProxy* aURLProxy)
@@ -1206,19 +1210,18 @@ already_AddRefed<URLWorker>
 FinishConstructor(JSContext* aCx, WorkerPrivate* aPrivate,
                   ConstructorRunnable* aRunnable, ErrorResult& aRv)
 {
   aRunnable->Dispatch(aRv);
   if (NS_WARN_IF(aRv.Failed())) {
     return nullptr;
   }
 
-  RefPtr<URLProxy> proxy = aRunnable->GetURLProxy();
-  if (NS_WARN_IF(!proxy)) {
-    aRv.Throw(NS_ERROR_DOM_SYNTAX_ERR);
+  RefPtr<URLProxy> proxy = aRunnable->GetURLProxy(aRv);
+  if (NS_WARN_IF(aRv.Failed())) {
     return nullptr;
   }
 
   RefPtr<URLWorker> url = new URLWorker(aPrivate, proxy);
   return url.forget();
 }
 
 /* static */ already_AddRefed<URLWorker>
@@ -1227,46 +1230,46 @@ URLWorker::Constructor(const GlobalObjec
 {
   MOZ_ASSERT(!NS_IsMainThread());
 
   JSContext* cx = aGlobal.Context();
   WorkerPrivate* workerPrivate = GetWorkerPrivateFromContext(cx);
 
   URLWorker& base = static_cast<URLWorker&>(aBase);
   RefPtr<ConstructorRunnable> runnable =
-    new ConstructorRunnable(workerPrivate, aURL, base.GetURLProxy(), aRv);
+    new ConstructorRunnable(workerPrivate, aURL, base.GetURLProxy());
 
   return FinishConstructor(cx, workerPrivate, runnable, aRv);
 }
 
 /* static */ already_AddRefed<URLWorker>
 URLWorker::Constructor(const GlobalObject& aGlobal, const nsAString& aURL,
                        const Optional<nsAString>& aBase, ErrorResult& aRv)
 {
   JSContext* cx = aGlobal.Context();
   WorkerPrivate* workerPrivate = GetWorkerPrivateFromContext(cx);
 
   RefPtr<ConstructorRunnable> runnable =
-    new ConstructorRunnable(workerPrivate, aURL, aBase, aRv);
+    new ConstructorRunnable(workerPrivate, aURL, aBase);
 
   return FinishConstructor(cx, workerPrivate, runnable, aRv);
 }
 
 /* static */ already_AddRefed<URLWorker>
 URLWorker::Constructor(const GlobalObject& aGlobal, const nsAString& aURL,
                        const nsAString& aBase, ErrorResult& aRv)
 {
   JSContext* cx = aGlobal.Context();
   WorkerPrivate* workerPrivate = GetWorkerPrivateFromContext(cx);
 
   Optional<nsAString> base;
   base = &aBase;
 
   RefPtr<ConstructorRunnable> runnable =
-    new ConstructorRunnable(workerPrivate, aURL, base, aRv);
+    new ConstructorRunnable(workerPrivate, aURL, base);
 
   return FinishConstructor(cx, workerPrivate, runnable, aRv);
 }
 
 /* static */ void
 URLWorker::CreateObjectURL(const GlobalObject& aGlobal, Blob& aBlob,
                            const mozilla::dom::objectURLOptions& aOptions,
                            nsAString& aResult, mozilla::ErrorResult& aRv)