Add 'error' parameter to 'window.onerror' handlers.

Source/core/dom/ScriptExecutionContext.cpp

Index: Source/core/dom/ScriptExecutionContext.cpp
diff --git a/Source/core/dom/ScriptExecutionContext.cpp b/Source/core/dom/ScriptExecutionContext.cpp
index ff466ba50c62691668ca73cb576396de04f1a26e..a69587fbe80759de6ef4f4ee05febbf7e8eac843 100644
--- a/Source/core/dom/ScriptExecutionContext.cpp
+++ b/Source/core/dom/ScriptExecutionContext.cpp
@@ -28,6 +28,7 @@
 #include "config.h"
 #include "core/dom/ScriptExecutionContext.h"
 
+#include "bindings/v8/SerializedScriptValue.h"
 #include "core/dom/ContextLifecycleNotifier.h"
 #include "core/dom/ErrorEvent.h"
 #include "core/dom/EventTarget.h"
@@ -194,7 +195,7 @@ void ScriptExecutionContext::closeMessagePorts() {
     }
 }
 
-bool ScriptExecutionContext::sanitizeScriptError(String& errorMessage, int& lineNumber, int& columnNumber, String& sourceURL)
+bool ScriptExecutionContext::sanitizeScriptError(String& errorMessage, int& lineNumber, int& columnNumber, String& sourceURL, ScriptValue& error)
 {
     KURL targetURL = completeURL(sourceURL);
     if (securityOrigin()->canRequest(targetURL))
@@ -203,10 +204,11 @@ bool ScriptExecutionContext::sanitizeScriptError(String& errorMessage, int& line
     sourceURL = String();
     lineNumber = 0;
     columnNumber = 0;
+    error.clear();
     return true;
 }
 
-void ScriptExecutionContext::reportException(const String& errorMessage, int lineNumber, int columnNumber, const String& sourceURL, PassRefPtr<ScriptCallStack> callStack)
+void ScriptExecutionContext::reportException(const String& errorMessage, int lineNumber, int columnNumber, const String& sourceURL, PassRefPtr<ScriptCallStack> callStack, const ScriptValue& error)
 {
     if (m_inDispatchErrorEvent) {
         if (!m_pendingExceptions)
@@ -216,7 +218,7 @@ void ScriptExecutionContext::reportException(const String& errorMessage, int lin
     }
 
     // First report the original exception and only then all the nested ones.
-    if (!dispatchErrorEvent(errorMessage, lineNumber, columnNumber, sourceURL))
+    if (!dispatchErrorEvent(errorMessage, lineNumber, columnNumber, sourceURL, error))
         logExceptionToConsole(errorMessage, sourceURL, lineNumber, columnNumber, callStack);
 
     if (!m_pendingExceptions)
@@ -234,7 +236,7 @@ void ScriptExecutionContext::addConsoleMessage(MessageSource source, MessageLeve
     addMessage(source, level, message, sourceURL, lineNumber, 0, state, requestIdentifier);
 }
 
-bool ScriptExecutionContext::dispatchErrorEvent(const String& errorMessage, int lineNumber, int columnNumber, const String& sourceURL)
+bool ScriptExecutionContext::dispatchErrorEvent(const String& errorMessage, int lineNumber, int columnNumber, const String& sourceURL, const ScriptValue& errorObject)
 {
     EventTarget* target = errorEventTarget();
     if (!target)
@@ -244,11 +246,12 @@ bool ScriptExecutionContext::dispatchErrorEvent(const String& errorMessage, int
     int line = lineNumber;
     int column = columnNumber;
     String sourceName = sourceURL;
-    sanitizeScriptError(message, line, column, sourceName);
+    ScriptValue error = errorObject;
+    sanitizeScriptError(message, line, column, sourceName, error);
 
     ASSERT(!m_inDispatchErrorEvent);
     m_inDispatchErrorEvent = true;
-    RefPtr<ErrorEvent> errorEvent = ErrorEvent::create(message, sourceName, line, column);
+    RefPtr<ErrorEvent> errorEvent = ErrorEvent::create(message, sourceName, line, column, error);

[adamk, 2013/07/30 15:46:41]

It seems like the thing holding you back from just storing the error on the
wrapper is a layering concern about calling toV8() here in
ScriptExecutionContext.cpp. But the amount of code would be tiny, something
like:

toV8(errorEvent)->SetHiddenValue(V8HiddenPropertyName::error(), error.v8Value())

I'm convinced sure hiding the V8 stuff behind ScriptValue is providing any value
here. The worldID workaround just seems weird.

[Mike West, 2013/07/30 15:53:37]

Yup. You're entirely correct. But there are only a handful of places in core
that use 'toV8', and it generally feels like a hack. This code shouldn't need to
know about worlds or wrappers. I'm not sure introducing those concepts is a good
idea.

[adamk, 2013/07/30 16:00:48]

No need to teach it about worlds (though note that DOMWrapperWorld is already
used in core), but hiding the existence of wrappers seems hardly worth it to me.
I guess this is just a difference of opinion about the aesthetics (as long as
your version doesn't leak memory, that is; where do you break the reference
cycle from script -> ErrorEvent -> script?).

     target->dispatchEvent(errorEvent);
     m_inDispatchErrorEvent = false;
     return errorEvent->defaultPrevented();