From 1a974d67e515c025a6173ebe1ab3be42240d2ef3 Mon Sep 17 00:00:00 2001 From: "igor@mir2.org" Date: Wed, 9 Jan 2008 07:31:13 -0800 Subject: [PATCH] Bug 409109: backing the checking as the tree was closed. --- dom/src/base/nsJSEnvironment.cpp | 80 ++++++++++++-------------- dom/src/base/nsJSEnvironment.h | 7 ++- js/src/jsapi.h | 13 ++--- js/src/jscntxt.h | 4 +- js/src/xpconnect/src/xpccomponents.cpp | 39 +++++-------- 5 files changed, 63 insertions(+), 80 deletions(-) diff --git a/dom/src/base/nsJSEnvironment.cpp b/dom/src/base/nsJSEnvironment.cpp index 55491b6faa4..a59ecbfb51b 100644 --- a/dom/src/base/nsJSEnvironment.cpp +++ b/dom/src/base/nsJSEnvironment.cpp @@ -782,19 +782,19 @@ PrintWinCodebase(nsGlobalWindow *win) } #endif -// The number of operation callbacks between calls to JS_MaybeGC -#define MAYBE_GC_OPERATION_COUNT_MASK 0x00000fff // 4095 +// The number of branch callbacks between calls to JS_MaybeGC +#define MAYBE_GC_BRANCH_COUNT_MASK 0x00000fff // 4095 -// The number of operation callbacks before we even check if our start +// The number of branch callbacks before we even check if our start // timestamp is initialized. This is a fairly low number as we want to // initialize the timestamp early enough to not waste much time before // we get there, but we don't want to bother doing this too early as // it's not generally necessary. -#define INITIALIZE_TIME_OPERATION_COUNT_MASK 0x000000ff // 255 +#define INITIALIZE_TIME_BRANCH_COUNT_MASK 0x000000ff // 255 // This function is called after each JS branch execution JSBool JS_DLL_CALLBACK -nsJSContext::DOMOperationCallback(JSContext *cx) +nsJSContext::DOMBranchCallback(JSContext *cx, JSScript *script) { // Get the native context nsJSContext *ctx = static_cast(::JS_GetContextPrivate(cx)); @@ -804,17 +804,17 @@ nsJSContext::DOMOperationCallback(JSContext *cx) return JS_TRUE; } - PRUint32 callbackCount = ++ctx->mOperationCallbackCount; + PRUint32 callbackCount = ++ctx->mBranchCallbackCount; - if (callbackCount & INITIALIZE_TIME_OPERATION_COUNT_MASK) { + if (callbackCount & INITIALIZE_TIME_BRANCH_COUNT_MASK) { return JS_TRUE; } - if (callbackCount == INITIALIZE_TIME_OPERATION_COUNT_MASK + 1 && - LL_IS_ZERO(ctx->mOperationCallbackTime)) { - // Initialize mOperationCallbackTime to start timing how long the + if (callbackCount == INITIALIZE_TIME_BRANCH_COUNT_MASK + 1 && + LL_IS_ZERO(ctx->mBranchCallbackTime)) { + // Initialize mBranchCallbackTime to start timing how long the // script has run - ctx->mOperationCallbackTime = PR_Now(); + ctx->mBranchCallbackTime = PR_Now(); ctx->mIsTrackingChromeCodeTime = ::JS_IsSystemObject(cx, ::JS_GetGlobalObject(cx)); @@ -822,22 +822,22 @@ nsJSContext::DOMOperationCallback(JSContext *cx) return JS_TRUE; } - if (callbackCount & MAYBE_GC_OPERATION_COUNT_MASK) { + if (callbackCount & MAYBE_GC_BRANCH_COUNT_MASK) { return JS_TRUE; } - // XXX Save the operation callback time so we can restore it after the GC, + // XXX Save the branch callback time so we can restore it after the GC, // because GCing can cause JS to run on our context, causing our - // ScriptEvaluated to be called, and clearing our operation callback time - // and count. See bug 302333. - PRTime callbackTime = ctx->mOperationCallbackTime; + // ScriptEvaluated to be called, and clearing our branch callback time and + // count. See bug 302333. + PRTime callbackTime = ctx->mBranchCallbackTime; // Run the GC if we get this far. JS_MaybeGC(cx); // Now restore the callback time and count, in case they got reset. - ctx->mOperationCallbackTime = callbackTime; - ctx->mOperationCallbackCount = callbackCount; + ctx->mBranchCallbackTime = callbackTime; + ctx->mBranchCallbackCount = callbackCount; PRTime now = PR_Now(); @@ -871,9 +871,7 @@ nsJSContext::DOMOperationCallback(JSContext *cx) nsresult rv; // Check if we should offer the option to debug - JSStackFrame* fp = ::JS_GetScriptedCaller(cx, NULL); - PRBool debugPossible = (fp != nsnull && - cx->debugHooks->debuggerHandler != nsnull); + PRBool debugPossible = (cx->debugHooks->debuggerHandler != nsnull); #ifdef MOZ_JSDEBUGGER // Get the debugger service if necessary. if (debugPossible) { @@ -941,7 +939,6 @@ nsJSContext::DOMOperationCallback(JSContext *cx) } // Append file and line number information, if available - JSScript *script = fp ? ::JS_GetFrameScript(cx, fp) : nsnull; if (script) { const char *filename = ::JS_GetScriptFilename(cx, script); if (filename) { @@ -997,18 +994,17 @@ nsJSContext::DOMOperationCallback(JSContext *cx) } } - ctx->mOperationCallbackTime = PR_Now(); + ctx->mBranchCallbackTime = PR_Now(); return JS_TRUE; } else if ((buttonPressed == 2) && debugPossible) { // Debug the script jsval rval; - switch(cx->debugHooks->debuggerHandler(cx, script, ::JS_GetFramePC(cx, fp), - &rval, + switch(cx->debugHooks->debuggerHandler(cx, script, cx->fp->pc, &rval, cx->debugHooks-> debuggerHandlerData)) { case JSTRAP_RETURN: - fp->rval = rval; + cx->fp->rval = rval; return JS_TRUE; case JSTRAP_ERROR: cx->throwing = JS_FALSE; @@ -1096,7 +1092,9 @@ nsJSContext::nsJSContext(JSRuntime *aRuntime) : mGCOnDestruction(PR_TRUE) ++sContextCount; - mDefaultJSOptions = JSOPTION_PRIVATE_IS_NSISUPPORTS | JSOPTION_ANONFUNFIX; + mDefaultJSOptions = JSOPTION_PRIVATE_IS_NSISUPPORTS + | JSOPTION_NATIVE_BRANCH_CALLBACK + | JSOPTION_ANONFUNFIX; // Let xpconnect resync its JSContext tracker. We do this before creating // a new JSContext just in case the heap manager recycles the JSContext @@ -1115,8 +1113,7 @@ nsJSContext::nsJSContext(JSRuntime *aRuntime) : mGCOnDestruction(PR_TRUE) JSOptionChangedCallback, this); - ::JS_SetOperationCallback(mContext, DOMOperationCallback, - JS_OPERATION_WEIGHT_BASE); + ::JS_SetBranchCallback(mContext, DOMBranchCallback); static JSLocaleCallbacks localeCallbacks = { @@ -1132,8 +1129,8 @@ nsJSContext::nsJSContext(JSRuntime *aRuntime) : mGCOnDestruction(PR_TRUE) mNumEvaluations = 0; mTerminations = nsnull; mScriptsEnabled = PR_TRUE; - mOperationCallbackCount = 0; - mOperationCallbackTime = LL_ZERO; + mBranchCallbackCount = 0; + mBranchCallbackTime = LL_ZERO; mProcessingScriptTag = PR_FALSE; mIsTrackingChromeCodeTime = PR_FALSE; } @@ -1170,8 +1167,8 @@ nsJSContext::Unlink() // Clear our entry in the JSContext, bugzilla bug 66413 ::JS_SetContextPrivate(mContext, nsnull); - // Clear the operation callback, bugzilla bug 238218 - ::JS_ClearOperationCallback(mContext); + // Clear the branch callback, bugzilla bug 238218 + ::JS_SetBranchCallback(mContext, nsnull); // Unregister our "javascript.options.*" pref-changed callback. nsContentUtils::UnregisterPrefCallback(js_options_dot_str, @@ -1479,11 +1476,9 @@ nsJSContext::EvaluateString(const nsAString& aScript, } // The result of evaluation, used only if there were no errors. This need - // not be a GC root currently, provided we run the GC only from the - // operation callback or from ScriptEvaluated. - // - // TODO: use JS_Begin/EndRequest to keep the GC from racing with JS - // execution on any thread. + // not be a GC root currently, provided we run the GC only from the branch + // callback or from ScriptEvaluated. TODO: use JS_Begin/EndRequest to keep + // the GC from racing with JS execution on any thread. jsval val; nsJSContext::TerminationFuncHolder holder(this); @@ -1689,8 +1684,9 @@ nsJSContext::ExecuteScript(void *aScriptObject, } // The result of evaluation, used only if there were no errors. This need - // not be a GC root currently, provided we run the GC only from the - // operation callback or from ScriptEvaluated. + // not be a GC root currently, provided we run the GC only from the branch + // callback or from ScriptEvaluated. TODO: use JS_Begin/EndRequest to keep + // the GC from racing with JS execution on any thread. jsval val; JSBool ok; @@ -3229,8 +3225,8 @@ nsJSContext::ScriptEvaluated(PRBool aTerminated) } #endif - mOperationCallbackCount = 0; - mOperationCallbackTime = LL_ZERO; + mBranchCallbackCount = 0; + mBranchCallbackTime = LL_ZERO; } nsresult diff --git a/dom/src/base/nsJSEnvironment.h b/dom/src/base/nsJSEnvironment.h index 7502708c356..eb0ff5fa23e 100644 --- a/dom/src/base/nsJSEnvironment.h +++ b/dom/src/base/nsJSEnvironment.h @@ -279,8 +279,8 @@ private: PRPackedBool mProcessingScriptTag; PRPackedBool mIsTrackingChromeCodeTime; - PRUint32 mOperationCallbackCount; - PRTime mOperationCallbackTime; + PRUint32 mBranchCallbackCount; + PRTime mBranchCallbackTime; PRUint32 mDefaultJSOptions; // mGlobalWrapperRef is used only to hold a strong reference to the @@ -292,7 +292,8 @@ private: static int PR_CALLBACK JSOptionChangedCallback(const char *pref, void *data); - static JSBool JS_DLL_CALLBACK DOMOperationCallback(JSContext *cx); + static JSBool JS_DLL_CALLBACK DOMBranchCallback(JSContext *cx, + JSScript *script); }; class nsIJSRuntimeService; diff --git a/js/src/jsapi.h b/js/src/jsapi.h index 2376b4be518..33e6d6b37c1 100644 --- a/js/src/jsapi.h +++ b/js/src/jsapi.h @@ -2105,19 +2105,14 @@ JS_CallFunctionValue(JSContext *cx, JSObject *obj, jsval fval, uintN argc, */ #define JS_MAX_OPERATION_LIMIT ((uint32) 0x7FFFFFFF) -#define JS_OPERATION_WEIGHT_BASE 4096 - /* * Set the operation callback that the engine calls periodically after * the internal operation count reaches the specified limit. * - * When operationLimit is JS_OPERATION_WEIGHT_BASE, the callback will be - * called at least after each backward jump in the interpreter. To minimize - * the overhead of the callback invocation we suggest at least - * - * 100 * JS_OPERATION_WEIGHT_BASE - * - * as a value for operationLimit. + * When operationLimit is 4096, the callback will be called at least after + * each backward jump in the interpreter. To minimize the overhead of the + * callback invocation we suggest at least 100*4096 as a value for + * operationLimit. */ extern JS_PUBLIC_API(void) JS_SetOperationCallback(JSContext *cx, JSOperationCallback callback, diff --git a/js/src/jscntxt.h b/js/src/jscntxt.h index b9031de2135..693e2ff48e8 100644 --- a/js/src/jscntxt.h +++ b/js/src/jscntxt.h @@ -1075,8 +1075,8 @@ extern JSErrorFormatString js_ErrorFormatString[JSErr_Limit]; #define JSOW_SET_PROPERTY 20 #define JSOW_NEW_PROPERTY 200 #define JSOW_DELETE_PROPERTY 30 -#define JSOW_ENTER_SHARP JS_OPERATION_WEIGHT_BASE -#define JSOW_SCRIPT_JUMP JS_OPERATION_WEIGHT_BASE +#define JSOW_ENTER_SHARP 4096 +#define JSOW_SCRIPT_JUMP 4096 /* * Reset the operation count and call the operation callback assuming that the diff --git a/js/src/xpconnect/src/xpccomponents.cpp b/js/src/xpconnect/src/xpccomponents.cpp index 1dfcd4b83fe..332cd356f70 100644 --- a/js/src/xpconnect/src/xpccomponents.cpp +++ b/js/src/xpconnect/src/xpccomponents.cpp @@ -3354,9 +3354,11 @@ public: NS_DECL_ISUPPORTS private: - static JSBool JS_DLL_CALLBACK ContextHolderOperationCallback(JSContext *cx); + static JSBool JS_DLL_CALLBACK ContextHolderBranchCallback(JSContext *cx, + JSScript *script); XPCAutoJSContext mJSContext; + JSBranchCallback mOrigBranchCallback; JSContext* mOrigCx; }; @@ -3364,49 +3366,38 @@ NS_IMPL_ISUPPORTS0(ContextHolder) ContextHolder::ContextHolder(JSContext *aOuterCx, JSObject *aSandbox) : mJSContext(JS_NewContext(JS_GetRuntime(aOuterCx), 1024), JS_FALSE), + mOrigBranchCallback(nsnull), mOrigCx(aOuterCx) { - if(mJSContext) - { + if (mJSContext) { JS_SetOptions(mJSContext, JSOPTION_DONT_REPORT_UNCAUGHT | JSOPTION_PRIVATE_IS_NSISUPPORTS); JS_SetGlobalObject(mJSContext, aSandbox); JS_SetContextPrivate(mJSContext, this); - if(JS_GetOperationCallback(aOuterCx)) - { - JS_SetOperationCallback(mJSContext, ContextHolderOperationCallback, - JS_GetOperationLimit(aOuterCx)); + // Now cache the original branch callback + mOrigBranchCallback = JS_SetBranchCallback(aOuterCx, nsnull); + JS_SetBranchCallback(aOuterCx, mOrigBranchCallback); + + if (mOrigBranchCallback) { + JS_SetBranchCallback(mJSContext, ContextHolderBranchCallback); } } } JSBool JS_DLL_CALLBACK -ContextHolder::ContextHolderOperationCallback(JSContext *cx) +ContextHolder::ContextHolderBranchCallback(JSContext *cx, JSScript *script) { ContextHolder* thisObject = static_cast(JS_GetContextPrivate(cx)); NS_ASSERTION(thisObject, "How did that happen?"); - JSContext *origCx = thisObject->mOrigCx; - JSOperationCallback callback = JS_GetOperationCallback(origCx); - JSBool ok = JS_TRUE; - if(callback) - { - ok = callback(origCx); - callback = JS_GetOperationCallback(origCx); - if(callback) - { - // If the callback is still set in the original context, reflect - // a possibly updated operation limit into cx. - JS_SetOperationLimit(cx, JS_GetOperationLimit(origCx)); - return ok; - } + if (thisObject->mOrigBranchCallback) { + return (thisObject->mOrigBranchCallback)(thisObject->mOrigCx, script); } - JS_ClearOperationCallback(cx); - return ok; + return JS_TRUE; } /***************************************************************************/