Bug 810644 part 1. Switch setTimeout and setInterval to using WebIDL callback functions. r=smaug, sr=peterv
authorBoris Zbarsky <bzbarsky@mit.edu>
Thu, 03 Jan 2013 14:02:36 -0500
changeset 117473 39b595bc7d78995e15f82e444d354a2b944ca92e
parent 117472 53e402890d35b9d0147e9c03fb32a9f05c7b937f
child 117474 c80717e72675f0f8698e3d58e682fa5a6425d341
push id1
push userroot
push dateMon, 20 Oct 2014 17:29:22 +0000
reviewerssmaug, peterv
bugs810644
milestone20.0a1
Bug 810644 part 1. Switch setTimeout and setInterval to using WebIDL callback functions. r=smaug, sr=peterv
dom/base/nsGlobalWindow.cpp
dom/base/nsIScriptTimeoutHandler.h
dom/base/nsJSTimeoutHandler.cpp
layout/xul/base/test/window_resizer_element.xul
--- a/dom/base/nsGlobalWindow.cpp
+++ b/dom/base/nsGlobalWindow.cpp
@@ -221,16 +221,17 @@
 #include "nsWrapperCacheInlines.h"
 #include "nsDOMEventTargetHelper.h"
 #include "nsIAppsService.h"
 #include "prrng.h"
 #include "nsSandboxFlags.h"
 #include "TimeChangeObserver.h"
 #include "nsPISocketTransportService.h"
 #include "mozilla/dom/AudioContext.h"
+#include "mozilla/dom/FunctionBinding.h"
 
 // Apple system headers seem to have a check() macro.  <sigh>
 #ifdef check
 #undef check
 #endif // check
 #include "AccessCheck.h"
 
 #ifdef ANDROID
@@ -1381,18 +1382,22 @@ nsGlobalWindow::IsBlackForCC()
 
 void
 nsGlobalWindow::UnmarkGrayTimers()
 {
   for (nsTimeout* timeout = mTimeouts.getFirst();
        timeout;
        timeout = timeout->getNext()) {
     if (timeout->mScriptHandler) {
-      JSObject* o = timeout->mScriptHandler->GetScriptObject();
-      xpc_UnmarkGrayObject(o);
+      Function* f = timeout->mScriptHandler->GetCallback();
+      if (f) {
+        // Callable() already does xpc_UnmarkGrayObject.
+        JSObject* o = f->Callable();
+        MOZ_ASSERT(!xpc_IsGrayGCThing(o), "Should have been unmarked");
+      }
     }
   }
 }
 
 //*****************************************************************************
 // nsGlobalWindow::nsIScriptGlobalObject
 //*****************************************************************************
 
@@ -9741,44 +9746,43 @@ nsGlobalWindow::RunTimeoutHandler(nsTime
   bool trackNestingLevel = !timeout->mIsInterval;
   uint32_t nestingLevel;
   if (trackNestingLevel) {
     nestingLevel = sNestingLevel;
     sNestingLevel = timeout->mNestingLevel;
   }
 
   nsCOMPtr<nsIScriptTimeoutHandler> handler(timeout->mScriptHandler);
-  JSObject* scriptObject = handler->GetScriptObject();
-  if (!scriptObject) {
+  nsRefPtr<Function> callback = handler->GetCallback();
+  if (!callback) {
     // Evaluate the timeout expression.
     const PRUnichar* script = handler->GetHandlerText();
     NS_ASSERTION(script, "timeout has no script nor handler text!");
 
     const char* filename = nullptr;
     uint32_t lineNo = 0;
     handler->GetLocation(&filename, &lineNo);
 
     bool is_undefined;
     aScx->EvaluateString(nsDependentString(script), FastGetGlobalJSObject(),
                          timeout->mPrincipal, timeout->mPrincipal,
                          filename, lineNo, JSVERSION_DEFAULT, nullptr,
                          &is_undefined);
   } else {
-    nsCOMPtr<nsIVariant> dummy;
+    // Hold strong ref to ourselves while we call the callback.
     nsCOMPtr<nsISupports> me(static_cast<nsIDOMWindow *>(this));
-    aScx->CallEventHandler(me, FastGetGlobalJSObject(),
-                           scriptObject, handler->GetArgv(),
-                           // XXXmarkh - consider allowing CallEventHandler to
-                           // accept nullptr?
-                           getter_AddRefs(dummy));
-
-  }
-
-  // We ignore any failures from calling EvaluateString() or
-  // CallEventHandler() on the context here since we're in a loop
+    ErrorResult ignored;
+    // Need the .get() because the first argument is a template, and C++ can't
+    // decide between it being a ParentObject& and a void* if we pass in the
+    // nsCOMPtr.
+    callback->Call(me.get(), handler->GetArgs(), ignored);
+  }
+
+  // We ignore any failures from calling EvaluateString() on the context or
+  // Call() on a Function here since we're in a loop
   // where we're likely to be running timeouts whose OS timers
   // didn't fire in time and we don't want to not fire those timers
   // now just because execution of one timer failed. We can't
   // propagate the error to anyone who cares about it from this
   // point anyway, and the script context should have already reported
   // the script error in the usual way - so we just drop it.
 
   if (trackNestingLevel) {
--- a/dom/base/nsIScriptTimeoutHandler.h
+++ b/dom/base/nsIScriptTimeoutHandler.h
@@ -1,46 +1,53 @@
 /* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* vim: set ts=2 sw=2 et tw=80: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 #ifndef nsIScriptTimeoutHandler_h___
 #define nsIScriptTimeoutHandler_h___
 
-class nsIArray;
+#include "nsTArray.h"
+
+namespace JS {
+class Value;
+} // namespace JS
+namespace mozilla {
+namespace dom {
+class Function;
+} // namespace dom
+} // namespace mozilla
 
 #define NS_ISCRIPTTIMEOUTHANDLER_IID \
-{ 0xcaf520a5, 0x8078, 0x4cba, \
-  { 0x8a, 0xb9, 0xb6, 0x8a, 0x12, 0x43, 0x4f, 0x05 } }
+{ 0x53c8e80e, 0xcc78, 0x48bc, \
+ { 0xba, 0x63, 0x0c, 0xb9, 0xdb, 0xf7, 0x06, 0x34 } }
 
 /**
  * Abstraction of the script objects etc required to do timeouts in a
  * language agnostic way.
  */
 
 class nsIScriptTimeoutHandler : public nsISupports
 {
 public:
   NS_DECLARE_STATIC_IID_ACCESSOR(NS_ISCRIPTTIMEOUTHANDLER_IID)
 
-  // Get a script object for the language suitable for passing back to
-  // the language's context as an event handler.  If this returns nullptr,
-  // GetHandlerText() will be called to get the string.
-  virtual JSObject *GetScriptObject() = 0;
+  // Get the Function to call.  If this returns nullptr, GetHandlerText() will
+  // be called to get the string.
+  virtual mozilla::dom::Function *GetCallback() = 0;
 
   // Get the handler text of not a compiled object.
   virtual const PRUnichar *GetHandlerText() = 0;
 
   // Get the location of the script.
   // Note: The memory pointed to by aFileName is owned by the
   // nsIScriptTimeoutHandler and should not be freed by the caller.
   virtual void GetLocation(const char **aFileName, uint32_t *aLineNo) = 0;
 
-  // If a script object, get the argv suitable for passing back to the
-  // script context.
-  virtual nsIArray *GetArgv() = 0;
+  // If we have a Function, get the arguments for passing to it.
+  virtual const nsTArray<JS::Value>& GetArgs() = 0;
 };
 
 NS_DEFINE_STATIC_IID_ACCESSOR(nsIScriptTimeoutHandler,
                               NS_ISCRIPTTIMEOUTHANDLER_IID)
 
 #endif // nsIScriptTimeoutHandler_h___
--- a/dom/base/nsJSTimeoutHandler.cpp
+++ b/dom/base/nsJSTimeoutHandler.cpp
@@ -16,62 +16,65 @@
 #include "nsJSEnvironment.h"
 #include "nsServiceManagerUtils.h"
 #include "nsError.h"
 #include "nsGlobalWindow.h"
 #include "nsIContentSecurityPolicy.h"
 #include "nsAlgorithm.h"
 #include "mozilla/Attributes.h"
 #include "mozilla/Likely.h"
+#include "mozilla/dom/FunctionBinding.h"
 
 static const char kSetIntervalStr[] = "setInterval";
 static const char kSetTimeoutStr[] = "setTimeout";
 
+using namespace mozilla::dom;
+
 // Our JS nsIScriptTimeoutHandler implementation.
 class nsJSScriptTimeoutHandler MOZ_FINAL : public nsIScriptTimeoutHandler
 {
 public:
   // nsISupports
   NS_DECL_CYCLE_COLLECTING_ISUPPORTS
   NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_CLASS(nsJSScriptTimeoutHandler)
 
   nsJSScriptTimeoutHandler();
   ~nsJSScriptTimeoutHandler();
 
   virtual const PRUnichar *GetHandlerText();
-  virtual JSObject *GetScriptObject() {
-    return mFunObj;
+  virtual Function* GetCallback()
+  {
+    return mFunction;
   }
-  virtual void GetLocation(const char **aFileName, uint32_t *aLineNo) {
+  virtual void GetLocation(const char **aFileName, uint32_t *aLineNo)
+  {
     *aFileName = mFileName.get();
     *aLineNo = mLineNo;
   }
 
-  virtual nsIArray *GetArgv() {
-    return mArgv;
+  virtual const nsTArray<JS::Value>& GetArgs()
+  {
+    return mArgs;
   }
 
   nsresult Init(nsGlobalWindow *aWindow, bool *aIsInterval,
                 int32_t *aInterval);
 
   void ReleaseJSObjects();
 
 private:
-
-  nsCOMPtr<nsIScriptContext> mContext;
-
   // filename, line number and JS language version string of the
   // caller of setTimeout()
   nsCString mFileName;
   uint32_t mLineNo;
-  nsCOMPtr<nsIJSArgArray> mArgv;
+  nsTArray<JS::Value> mArgs;
 
   // The JS expression to evaluate or function to call, if !mExpr
   JSFlatString *mExpr;
-  JSObject *mFunObj;
+  nsRefPtr<Function> mFunction;
 };
 
 
 // nsJSScriptTimeoutHandler
 // QueryInterface implementation for nsJSScriptTimeoutHandler
 NS_IMPL_CYCLE_COLLECTION_CLASS(nsJSScriptTimeoutHandler)
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(nsJSScriptTimeoutHandler)
   tmp->ReleaseJSObjects();
@@ -81,18 +84,19 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_
     nsAutoCString name("nsJSScriptTimeoutHandler");
     if (tmp->mExpr) {
       name.AppendLiteral(" [");
       name.Append(tmp->mFileName);
       name.AppendLiteral(":");
       name.AppendInt(tmp->mLineNo);
       name.AppendLiteral("]");
     }
-    else if (tmp->mFunObj) {
-      JSFunction* fun = JS_GetObjectFunction(tmp->mFunObj);
+    else if (tmp->mFunction) {
+      JSFunction* fun =
+        JS_GetObjectFunction(js::UnwrapObject(tmp->mFunction->Callable()));
       if (fun && JS_GetFunctionId(fun)) {
         JSFlatString *funId = JS_ASSERT_STRING_IS_FLAT(JS_GetFunctionId(fun));
         size_t size = 1 + JS_PutEscapedFlatString(NULL, 0, funId, 0);
         char *funIdName = new char[size];
         if (funIdName) {
           JS_PutEscapedFlatString(funIdName, size, funId, 0);
           name.AppendLiteral(" [");
           name.Append(funIdName);
@@ -103,63 +107,63 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_
     }
     cb.DescribeRefCountedNode(tmp->mRefCnt.get(), name.get());
   }
   else {
     NS_IMPL_CYCLE_COLLECTION_DESCRIBE(nsJSScriptTimeoutHandler,
                                       tmp->mRefCnt.get())
   }
 
-  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mContext)
-  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mArgv)
+  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mFunction)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE_SCRIPT_OBJECTS
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 
 NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN(nsJSScriptTimeoutHandler)
   NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mExpr)
-  NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mFunObj)
+  for (uint32_t i = 0; i < tmp->mArgs.Length(); ++i) {
+    NS_IMPL_CYCLE_COLLECTION_TRACE_JSVAL_MEMBER_CALLBACK(mArgs[i])
+  }
 NS_IMPL_CYCLE_COLLECTION_TRACE_END
 
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(nsJSScriptTimeoutHandler)
   NS_INTERFACE_MAP_ENTRY(nsIScriptTimeoutHandler)
   NS_INTERFACE_MAP_ENTRY(nsISupports)
 NS_INTERFACE_MAP_END
 
 NS_IMPL_CYCLE_COLLECTING_ADDREF(nsJSScriptTimeoutHandler)
 NS_IMPL_CYCLE_COLLECTING_RELEASE(nsJSScriptTimeoutHandler)
 
 nsJSScriptTimeoutHandler::nsJSScriptTimeoutHandler() :
   mLineNo(0),
-  mExpr(nullptr),
-  mFunObj(nullptr)
+  mExpr(nullptr)
 {
 }
 
 nsJSScriptTimeoutHandler::~nsJSScriptTimeoutHandler()
 {
   ReleaseJSObjects();
 }
 
 void
 nsJSScriptTimeoutHandler::ReleaseJSObjects()
 {
   if (mExpr) {
     mExpr = nullptr;
   } else {
-    mFunObj = nullptr;
+    mFunction = nullptr;
+    mArgs.Clear();
   }
   NS_DROP_JS_OBJECTS(this, nsJSScriptTimeoutHandler);
 }
 
 nsresult
 nsJSScriptTimeoutHandler::Init(nsGlobalWindow *aWindow, bool *aIsInterval,
                                int32_t *aInterval)
 {
-  mContext = aWindow->GetContextInternal();
-  if (!mContext) {
+  if (!aWindow->GetContextInternal() || !aWindow->FastGetGlobalJSObject()) {
     // This window was already closed, or never properly initialized,
     // don't let a timer be scheduled on such a window.
 
     return NS_ERROR_NOT_INITIALIZED;
   }
 
   nsAXPCNativeCallContext *ncc = nullptr;
   nsresult rv = nsContentUtils::XPConnect()->
@@ -267,43 +271,38 @@ nsJSScriptTimeoutHandler::Init(nsGlobalW
     // Get the calling location.
     const char *filename;
     if (nsJSUtils::GetCallingLocation(cx, &filename, &mLineNo)) {
       mFileName.Assign(filename);
     }
   } else if (funobj) {
     NS_HOLD_JS_OBJECTS(this, nsJSScriptTimeoutHandler);
 
-    mFunObj = funobj;
+    bool ok;
+    mFunction = new Function(cx, aWindow->FastGetGlobalJSObject(), funobj, &ok);
+    if (!ok) {
+      NS_DROP_JS_OBJECTS(this, nsJSScriptTimeoutHandler);
+      return NS_ERROR_OUT_OF_MEMORY;
+    }
 
     // Create our arg array.  argc is the number of arguments passed
     // to setTimeout or setInterval; the first two are our callback
     // and the delay, so only arguments after that need to go in our
     // array.
-    nsCOMPtr<nsIJSArgArray> array;
     // NS_MAX(argc - 2, 0) wouldn't work right because argc is unsigned.
-    rv = NS_CreateJSArgv(cx, NS_MAX(argc, 2u) - 2, nullptr,
-                         getter_AddRefs(array));
-    if (NS_FAILED(rv)) {
+    uint32_t argCount = NS_MAX(argc, 2u) - 2;
+    FallibleTArray<JS::Value> args;
+    if (!args.SetCapacity(argCount)) {
+      // No need to drop here, since we already have a non-null mFunction
       return NS_ERROR_OUT_OF_MEMORY;
     }
-
-    uint32_t dummy;
-    jsval *jsargv = nullptr;
-    array->GetArgs(&dummy, reinterpret_cast<void **>(&jsargv));
-
-    // jsargv might be null if we have argc <= 2
-    if (jsargv) {
-      for (int32_t i = 2; (uint32_t)i < argc; ++i) {
-        jsargv[i - 2] = argv[i];
-      }
-    } else {
-      NS_ASSERTION(argc <= 2, "Why do we have no jsargv when we have arguments?");
+    for (uint32_t idx = 0; idx < argCount; ++idx) {
+      *args.AppendElement() = argv[idx + 2];
     }
-    mArgv = array;
+    args.SwapElements(mArgs);
   } else {
     NS_WARNING("No func and no expr - why are we here?");
   }
   *aInterval = interval;
   return NS_OK;
 }
 
 const PRUnichar *
@@ -314,22 +313,18 @@ nsJSScriptTimeoutHandler::GetHandlerText
 }
 
 nsresult NS_CreateJSTimeoutHandler(nsGlobalWindow *aWindow,
                                    bool *aIsInterval,
                                    int32_t *aInterval,
                                    nsIScriptTimeoutHandler **aRet)
 {
   *aRet = nullptr;
-  nsJSScriptTimeoutHandler *handler = new nsJSScriptTimeoutHandler();
-  if (!handler)
-    return NS_ERROR_OUT_OF_MEMORY;
-
+  nsRefPtr<nsJSScriptTimeoutHandler> handler = new nsJSScriptTimeoutHandler();
   nsresult rv = handler->Init(aWindow, aIsInterval, aInterval);
   if (NS_FAILED(rv)) {
-    delete handler;
     return rv;
   }
 
-  NS_ADDREF(*aRet = handler);
+  handler.forget(aRet);
 
   return NS_OK;
 }
--- a/layout/xul/base/test/window_resizer_element.xul
+++ b/layout/xul/base/test/window_resizer_element.xul
@@ -1,14 +1,15 @@
 <?xml-stylesheet href="chrome://global/skin" type="text/css"?>
 <window xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
         align="start">
 <script type="application/javascript" src="chrome://mochikit/content/tests/SimpleTest/EventUtils.js"></script>
 <script><![CDATA[
 var is = window.opener.SimpleTest.is;
+window.onerror = window.opener.onerror;
 
 const anchorPositions =
   [ "before_start", "before_end", "after_start", "after_end",
     "start_before", "start_after", "end_before", "end_after", "overlap", "screen"];
 var currentPosition;
 
 function testResizer(resizerid, noShrink, hResize, vResize, testid)
 {