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 126575 39b595bc7d78995e15f82e444d354a2b944ca92e
parent 126574 53e402890d35b9d0147e9c03fb32a9f05c7b937f
child 126576 c80717e72675f0f8698e3d58e682fa5a6425d341
push id2151
push userlsblakk@mozilla.com
push dateTue, 19 Feb 2013 18:06:57 +0000
treeherdermozilla-beta@4952e88741ec [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug, peterv
bugs810644
milestone20.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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)
 {