From f603b7b7c80b7ec59d493e5f11ed85a21d015757 Mon Sep 17 00:00:00 2001 From: Ben Turner Date: Thu, 27 Nov 2008 01:16:41 -0500 Subject: [PATCH] Bug 465141 - 'Workers: Implement expression flavor of timeouts, use nsAutoJSValHolder.' r+sr=jst, a=blocking1.9.1+ --- dom/src/threads/nsDOMThreadService.cpp | 4 - dom/src/threads/nsDOMWorkerTimeout.cpp | 122 ++++++++---------- dom/src/threads/nsDOMWorkerTimeout.h | 12 +- dom/src/threads/test/test_threadTimeouts.html | 3 + dom/src/threads/test/threadTimeouts_worker.js | 3 + 5 files changed, 67 insertions(+), 77 deletions(-) diff --git a/dom/src/threads/nsDOMThreadService.cpp b/dom/src/threads/nsDOMThreadService.cpp index 4158befb517..3d394700347 100644 --- a/dom/src/threads/nsDOMThreadService.cpp +++ b/dom/src/threads/nsDOMThreadService.cpp @@ -294,11 +294,7 @@ protected: // Clear out any old cruft hanging around in the regexp statics. JS_ClearRegExpStatics(cx); -#ifdef DEBUG - nsresult rv = -#endif runnable->Run(); - NS_WARN_IF_FALSE(NS_SUCCEEDED(rv), "Runnable failed!"); } } diff --git a/dom/src/threads/nsDOMWorkerTimeout.cpp b/dom/src/threads/nsDOMWorkerTimeout.cpp index a067b1331c9..901285e15b9 100644 --- a/dom/src/threads/nsDOMWorkerTimeout.cpp +++ b/dom/src/threads/nsDOMWorkerTimeout.cpp @@ -53,6 +53,7 @@ // DOMWorker includes #include "nsDOMThreadService.h" +#include "nsDOMWorkerSecurityManager.h" #define LOG(_args) PR_LOG(gDOMThreadsLog, PR_LOG_DEBUG, _args) @@ -73,63 +74,38 @@ static const char* kSetTimeoutStr = "setTimeout"; nsDOMWorkerTimeout::FunctionCallback::FunctionCallback(PRUint32 aArgc, jsval* aArgv, nsresult* aRv) -: mCallback(nsnull), - mCallbackArgs(nsnull), - mCallbackArgsLength(0), - mRuntime(NULL) +: mCallbackArgsLength(0) { MOZ_COUNT_CTOR(nsDOMWorkerTimeout::FunctionCallback); - *aRv = nsDOMThreadService::JSRuntimeService()->GetRuntime(&mRuntime); + JSRuntime* rt; + *aRv = nsDOMThreadService::JSRuntimeService()->GetRuntime(&rt); NS_ENSURE_SUCCESS(*aRv,); - PRBool success = JS_AddNamedRootRT(mRuntime, &mCallback, - "nsDOMWorkerTimeout Callback Object"); - CONSTRUCTOR_ENSURE_TRUE(success, *aRv); + JSBool ok = mCallback.Hold(rt); + CONSTRUCTOR_ENSURE_TRUE(ok, *aRv); mCallback = aArgv[0]; // We want enough space for an extra lateness arg. mCallbackArgsLength = aArgc > 2 ? aArgc - 1 : 1; - mCallbackArgs = new jsval[mCallbackArgsLength]; - if (NS_UNLIKELY(!mCallbackArgs)) { - // Reset this! - mCallbackArgsLength = 0; + PRBool success = mCallbackArgs.SetLength(mCallbackArgsLength); + CONSTRUCTOR_ENSURE_TRUE(success, *aRv); - NS_ERROR("Out of memory!"); - *aRv = NS_ERROR_OUT_OF_MEMORY; - return; - } + PRUint32 index = 0; + for (; index < mCallbackArgsLength - 1; index++) { + ok = mCallbackArgs[index].Hold(rt); + CONSTRUCTOR_ENSURE_TRUE(ok, *aRv); - for (PRUint32 i = 0; i < mCallbackArgsLength - 1; i++) { - mCallbackArgs[i] = aArgv[i + 2]; - success = JS_AddNamedRootRT(mRuntime, &mCallbackArgs[i], - "nsDOMWorkerTimeout Callback Arg"); - if (NS_UNLIKELY(!success)) { - // Set this to i so that the destructor only unroots the right number of - // values. - mCallbackArgsLength = i; - - NS_WARNING("Failed to add root!"); - *aRv = NS_ERROR_FAILURE; - return; - } + mCallbackArgs[index] = aArgv[index + 2]; } // Take care of the last arg. - mCallbackArgs[mCallbackArgsLength - 1] = 0; - success = JS_AddNamedRootRT(mRuntime, &mCallbackArgs[mCallbackArgsLength - 1], - "nsDOMWorkerTimeout Callback Final Arg"); - if (NS_UNLIKELY(!success)) { - // Decrement this so that the destructor only unroots the right number of - // values. - mCallbackArgsLength -= 1; + index = mCallbackArgsLength - 1; - NS_WARNING("Failed to add root!"); - *aRv = NS_ERROR_FAILURE; - return; - } + ok = mCallbackArgs[index].Hold(rt); + CONSTRUCTOR_ENSURE_TRUE(ok, *aRv); *aRv = NS_OK; } @@ -137,15 +113,6 @@ nsDOMWorkerTimeout::FunctionCallback::FunctionCallback(PRUint32 aArgc, nsDOMWorkerTimeout::FunctionCallback::~FunctionCallback() { MOZ_COUNT_DTOR(nsDOMWorkerTimeout::FunctionCallback); - - if (mCallback) { - for (PRUint32 i = 0; i < mCallbackArgsLength; i++) { - JS_RemoveRootRT(mRuntime, &mCallbackArgs[i]); - } - JS_RemoveRootRT(mRuntime, &mCallback); - } - - delete [] mCallbackArgs; } nsresult @@ -159,11 +126,16 @@ nsDOMWorkerTimeout::FunctionCallback::Run(nsDOMWorkerTimeout* aTimeout, JSObject* global = JS_GetGlobalObject(aCx); NS_ENSURE_TRUE(global, NS_ERROR_FAILURE); + jsval argv[mCallbackArgsLength]; + for (PRUint32 index = 0; index < mCallbackArgsLength; index++) { + argv[index] = mCallbackArgs[index]; + } + jsval rval; - PRBool success = + JSBool ok = JS_CallFunctionValue(aCx, global, mCallback, mCallbackArgsLength, - mCallbackArgs, &rval); - NS_ENSURE_TRUE(success, NS_ERROR_FAILURE); + argv, &rval); + NS_ENSURE_TRUE(ok, NS_ERROR_FAILURE); return NS_OK; } @@ -172,9 +144,7 @@ nsDOMWorkerTimeout::ExpressionCallback::ExpressionCallback(PRUint32 aArgc, jsval* aArgv, JSContext* aCx, nsresult* aRv) -: mExpression(nsnull), - mLineNumber(0), - mRuntime(NULL) +: mLineNumber(0) { MOZ_COUNT_CTOR(nsDOMWorkerTimeout::ExpressionCallback); @@ -182,20 +152,20 @@ nsDOMWorkerTimeout::ExpressionCallback::ExpressionCallback(PRUint32 aArgc, *aRv = expr ? NS_OK : NS_ERROR_FAILURE; NS_ENSURE_SUCCESS(*aRv,); - *aRv = nsDOMThreadService::JSRuntimeService()->GetRuntime(&mRuntime); + JSRuntime* rt; + *aRv = nsDOMThreadService::JSRuntimeService()->GetRuntime(&rt); NS_ENSURE_SUCCESS(*aRv,); - PRBool success = JS_AddNamedRootRT(mRuntime, &mExpression, - "nsDOMWorkerTimeout Expression"); - CONSTRUCTOR_ENSURE_TRUE(success, *aRv); + JSBool ok = mExpression.Hold(rt); + CONSTRUCTOR_ENSURE_TRUE(ok, *aRv); - mExpression = expr; + mExpression = aArgv[0]; // Get the calling location. const char* fileName; PRUint32 lineNumber; if (nsJSUtils::GetCallingLocation(aCx, &fileName, &lineNumber, nsnull)) { - CopyUTF8toUTF16(nsDependentCString(fileName), mFileName); + mFileName.Assign(fileName); mLineNumber = lineNumber; } @@ -205,18 +175,36 @@ nsDOMWorkerTimeout::ExpressionCallback::ExpressionCallback(PRUint32 aArgc, nsDOMWorkerTimeout::ExpressionCallback::~ExpressionCallback() { MOZ_COUNT_DTOR(nsDOMWorkerTimeout::ExpressionCallback); - - if (mExpression) { - JS_RemoveRootRT(mRuntime, &mExpression); - } } nsresult nsDOMWorkerTimeout::ExpressionCallback::Run(nsDOMWorkerTimeout* aTimeout, JSContext* aCx) { - NS_ERROR("Not yet implemented!"); - return NS_ERROR_NOT_IMPLEMENTED; + JSObject* global = JS_GetGlobalObject(aCx); + NS_ENSURE_TRUE(global, NS_ERROR_FAILURE); + + JSPrincipals* principal = nsDOMWorkerSecurityManager::WorkerPrincipal(); + NS_ENSURE_TRUE(principal, NS_ERROR_FAILURE); + + JSString* expression = JS_ValueToString(aCx, mExpression); + NS_ENSURE_TRUE(expression, NS_ERROR_FAILURE); + + jschar* string = JS_GetStringChars(expression); + NS_ENSURE_TRUE(string, NS_ERROR_FAILURE); + + size_t stringLength = JS_GetStringLength(expression); + + jsval rval; + PRBool success = JS_EvaluateUCScriptForPrincipals(aCx, global, principal, + string, stringLength, + mFileName.get(), + mLineNumber, &rval); + if (!success) { + return NS_ERROR_FAILURE; + } + + return NS_OK; } nsDOMWorkerTimeout::nsDOMWorkerTimeout(nsDOMWorker* aWorker, diff --git a/dom/src/threads/nsDOMWorkerTimeout.h b/dom/src/threads/nsDOMWorkerTimeout.h index 098d3975810..41aa9bd2f74 100644 --- a/dom/src/threads/nsDOMWorkerTimeout.h +++ b/dom/src/threads/nsDOMWorkerTimeout.h @@ -44,9 +44,11 @@ // Other includes #include "jsapi.h" +#include "nsAutoJSValHolder.h" #include "nsAutoPtr.h" #include "nsCOMPtr.h" #include "nsStringGlue.h" +#include "nsTArray.h" // DOMWorker includes #include "nsDOMWorker.h" @@ -145,10 +147,9 @@ private: virtual nsresult Run(nsDOMWorkerTimeout* aTimeout, JSContext* aCx); protected: - jsval mCallback; - jsval* mCallbackArgs; + nsAutoJSValHolder mCallback; + nsTArray mCallbackArgs; PRUint32 mCallbackArgsLength; - JSRuntime* mRuntime; }; class ExpressionCallback : public CallbackBase @@ -162,10 +163,9 @@ private: virtual nsresult Run(nsDOMWorkerTimeout* aTimeout, JSContext* aCx); protected: - JSString* mExpression; - nsString mFileName; + nsAutoJSValHolder mExpression; + nsCString mFileName; PRUint32 mLineNumber; - JSRuntime* mRuntime; }; // Hold this object alive! diff --git a/dom/src/threads/test/test_threadTimeouts.html b/dom/src/threads/test/test_threadTimeouts.html index a2cc6d13db8..944b72c85ee 100644 --- a/dom/src/threads/test/test_threadTimeouts.html +++ b/dom/src/threads/test/test_threadTimeouts.html @@ -30,6 +30,9 @@ Tests of DOM Worker Threads (Bug 437152) event.target.postMessage("cancelInterval"); break; case "intervalCanceled": + worker.postMessage("startExpression"); + break; + case "expressionFinished": SimpleTest.finish(); break; default: diff --git a/dom/src/threads/test/threadTimeouts_worker.js b/dom/src/threads/test/threadTimeouts_worker.js index fd0ca9ecef0..f3e4f014e1d 100644 --- a/dom/src/threads/test/threadTimeouts_worker.js +++ b/dom/src/threads/test/threadTimeouts_worker.js @@ -29,6 +29,9 @@ function messageListener(event) { clearInterval(gTimeoutId); postMessage("intervalCanceled"); break; + case "startExpression": + setTimeout("this.postMessage('expressionFinished');", 2000); + break; default: throw "Bad message: " + event.data; }