Bug 903212 - Remove ScriptEvaluated, and simplify callers. r=mrbkap

content/xul/document/src/XULDocument.cpp

@@ -3653,7 +3653,6 @@ XULDocument::ExecuteScript(nsIScriptContext * aContext,
     JS::Rooted<JS::Value> unused(cx);
     if (!JS_ExecuteScript(cx, global, aScriptObject, unused.address()))
         nsJSUtils::ReportPendingException(cx);
-    aContext->ScriptEvaluated(true);
     return NS_OK;
 }
 

dom/base/nsIScriptContext.h

@@ -28,8 +28,8 @@ class nsIDOMWindow;
 class nsIURI;
 
 #define NS_ISCRIPTCONTEXT_IID \
-{ 0xfd05ba99, 0x2906, 0x4c51, \
+{ 0x6219173f, 0x4a61, 0x4c99, \
-  { 0x89, 0xb3, 0xbc, 0xdf, 0xf6, 0x3b, 0xf2, 0xde } }
+  { 0xb1, 0xfd, 0x8e, 0x7a, 0xf0, 0xdc, 0xe0, 0x56 } }
 
 /* This MUST match JSVERSION_DEFAULT.  This version stuff if we don't
    know what language we have is a little silly... */
@@ -130,19 +130,6 @@ public:
    */
   virtual void GC(JS::gcreason::Reason aReason) = 0;
 
-  /**
-   * Inform the context that a script was evaluated.
-   * A GC may be done if "necessary."
-   * This call is necessary if script evaluation is done
-   * without using the EvaluateScript method.
-   * @param aTerminated If true then do script termination handling. Within DOM
-   *     this will always be true, but outside  callers (such as xpconnect) who
-   *     may do script evaluations nested inside inside DOM script evaluations
-   *     can pass false to avoid premature termination handling.
-   * @return NS_OK if the method is successful
-   */
-  virtual void ScriptEvaluated(bool aTerminated) = 0;
-
   virtual nsresult Serialize(nsIObjectOutputStream* aStream,
                              JS::Handle<JSScript*> aScriptObject) = 0;
   

dom/base/nsJSEnvironment.cpp

@@ -960,22 +960,15 @@ nsJSContext::EvaluateString(const nsAString& aScript,
                             JS::Value* aRetValue)
 {
   NS_ENSURE_TRUE(mIsInitialized, NS_ERROR_NOT_INITIALIZED);
-  nsresult rv;
   if (!mScriptsEnabled) {
     return NS_OK;
   }
 
-  {
+  AutoCxPusher pusher(mContext);
-    AutoCxPusher pusher(mContext);
+  nsJSUtils::EvaluateOptions evalOptions;
-    nsJSUtils::EvaluateOptions evalOptions;
+  evalOptions.setCoerceToString(aCoerceToString);
-    evalOptions.setCoerceToString(aCoerceToString);
+  return nsJSUtils::EvaluateString(mContext, aScript, aScopeObject,
-    rv = nsJSUtils::EvaluateString(mContext, aScript, aScopeObject,
                                    aCompileOptions, evalOptions, aRetValue);
-  }
-
-  // ScriptEvaluated needs to come after we pop the stack
-  ScriptEvaluated(true);
-  return rv;
 }
 
 #ifdef DEBUG
@@ -1895,15 +1888,6 @@ nsJSContext::IsContextInitialized()
   return mIsInitialized;
 }
 
-void
-nsJSContext::ScriptEvaluated(bool aTerminated)
-{
-  if (GetNativeGlobal()) {
-    JSAutoCompartment ac(mContext, GetNativeGlobal());
-    JS_MaybeGC(mContext);
-  }
-}
-
 bool
 nsJSContext::GetScriptsEnabled()
 {
@@ -1940,8 +1924,6 @@ nsJSContext::SetProcessingScriptTag(bool aFlag)
 NS_IMETHODIMP
 nsJSContext::ScriptExecuted()
 {
-  ScriptEvaluated(!::JS_IsRunning(mContext));
-
   return NS_OK;
 }
 

dom/base/nsJSEnvironment.h

@@ -63,7 +63,6 @@ public:
   virtual nsresult InitContext() MOZ_OVERRIDE;
   virtual bool IsContextInitialized() MOZ_OVERRIDE;
 
-  virtual void ScriptEvaluated(bool aTerminated) MOZ_OVERRIDE;
   virtual bool GetScriptsEnabled() MOZ_OVERRIDE;
   virtual void SetScriptsEnabled(bool aEnabled, bool aFireTimeouts) MOZ_OVERRIDE;
 

dom/bindings/CallbackObject.cpp

@@ -104,13 +104,6 @@ CallbackObject::CallSetup::CallSetup(JS::Handle<JSObject*> aCallback,
   xpc_UnmarkGrayObject(aCallback);
   mRootedCallable.construct(cx, aCallback);
 
-  // After this point we guarantee calling ScriptEvaluated() if we
-  // have an nsIScriptContext.
-  // XXXbz Why, if, say CheckFunctionAccess fails?  I know that's how
-  // nsJSContext::CallEventHandler used to work, but is it required?
-  // FIXME: Bug 807369.
-  mCtx = ctx;
-
   // Check that it's ok to run this callback at all.
   // FIXME: Bug 807371: we want a less silly check here.
   // Make sure to unwrap aCallback before passing it in, because
@@ -164,10 +157,8 @@ CallbackObject::CallSetup::~CallSetup()
     }
   }
 
-  // If we have an mCtx, we need to call ScriptEvaluated() on it.  But we have
+  // To get our nesting right we have to destroy our JSAutoCompartment first.
-  // to do that after we pop the JSContext stack (see bug 295983).  And to get
+  // But be careful: it might not have been constructed at all!
-  // our nesting right we have to destroy our JSAutoCompartment first.  But be
-  // careful: it might not have been constructed at all!
   mAc.destroyIfConstructed();
 
   // XXXbz For that matter why do we need to manually call ScriptEvaluated at
@@ -178,10 +169,6 @@ CallbackObject::CallSetup::~CallSetup()
 
   // Popping an nsCxPusher is safe even if it never got pushed.
   mCxPusher.Pop();
-
-  if (mCtx) {
-    mCtx->ScriptEvaluated(true);
-  }
 }
 
 already_AddRefed<nsISupports>

dom/bindings/CallbackObject.h

@@ -135,7 +135,6 @@ protected:
 
     // Members which can go away whenever
     JSContext* mCx;
-    nsCOMPtr<nsIScriptContext> mCtx;
 
     // And now members whose construction/destruction order we need to control.
 

js/xpconnect/src/nsCxPusher.cpp

@@ -102,7 +102,7 @@ nsCxPusher::Pop()
 
 namespace mozilla {
 
-AutoCxPusher::AutoCxPusher(JSContext* cx, bool allowNull) : mScriptIsRunning(false)
+AutoCxPusher::AutoCxPusher(JSContext* cx, bool allowNull)
 {
   MOZ_ASSERT_IF(!allowNull, cx);
 
@@ -112,15 +112,7 @@ AutoCxPusher::AutoCxPusher(JSContext* cx, bool allowNull) : mScriptIsRunning(fal
   if (cx)
     mScx = GetScriptContextFromJSContext(cx);
 
-  // NB: The GetDynamicScriptContext is historical and might not be sane.
   XPCJSContextStack *stack = XPCJSRuntime::Get()->GetJSContextStack();
-  if (cx && nsJSUtils::GetDynamicScriptContext(cx) && stack->HasJSContext(cx))
-  {
-    // If the context is on the stack, that means that a script
-    // is running at the moment in the context.
-    mScriptIsRunning = true;
-  }
-
   if (!stack->Push(cx)) {
     MOZ_CRASH();
   }
@@ -169,16 +161,7 @@ AutoCxPusher::~AutoCxPusher()
   DebugOnly<JSContext*> stackTop;
   MOZ_ASSERT(mPushedContext == nsXPConnect::XPConnect()->GetCurrentJSContext());
   XPCJSRuntime::Get()->GetJSContextStack()->Pop();
-
-  if (!mScriptIsRunning && mScx) {
-    // No JS is running in the context, but executing the event handler might have
-    // caused some JS to run. Tell the script context that it's done.
-
-    mScx->ScriptEvaluated(true);
-  }
-
   mScx = nullptr;
-  mScriptIsRunning = false;
 }
 
 AutoJSContext::AutoJSContext(MOZ_GUARD_OBJECT_NOTIFIER_ONLY_PARAM_IN_IMPL)

js/xpconnect/src/nsCxPusher.h

@@ -35,7 +35,6 @@ private:
   mozilla::Maybe<JSAutoRequest> mAutoRequest;
   mozilla::Maybe<JSAutoCompartment> mAutoCompartment;
   nsCOMPtr<nsIScriptContext> mScx;
-  bool mScriptIsRunning;
 #ifdef DEBUG
   JSContext* mPushedContext;
   unsigned mCompartmentDepthOnEntry;