Web Inspector: JSContext inspection should report exceptions in the console
authorjoepeck@webkit.org <joepeck@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 21 Feb 2014 19:14:17 +0000 (19:14 +0000)
committerjoepeck@webkit.org <joepeck@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 21 Feb 2014 19:14:17 +0000 (19:14 +0000)
https://bugs.webkit.org/show_bug.cgi?id=128776

Source/JavaScriptCore:

Reviewed by Timothy Hatcher.

When JavaScript API functions have an exception, let the inspector
know so it can log the JavaScript and Native backtrace that caused
the exception.

Include some clean up of ConsoleMessage and ScriptCallStack construction.

* API/JSBase.cpp:
(JSEvaluateScript):
(JSCheckScriptSyntax):
* API/JSObjectRef.cpp:
(JSObjectMakeFunction):
(JSObjectMakeArray):
(JSObjectMakeDate):
(JSObjectMakeError):
(JSObjectMakeRegExp):
(JSObjectGetProperty):
(JSObjectSetProperty):
(JSObjectGetPropertyAtIndex):
(JSObjectSetPropertyAtIndex):
(JSObjectDeleteProperty):
(JSObjectCallAsFunction):
(JSObjectCallAsConstructor):
* API/JSValue.mm:
(reportExceptionToInspector):
(valueToArray):
(valueToDictionary):
* API/JSValueRef.cpp:
(JSValueIsEqual):
(JSValueIsInstanceOfConstructor):
(JSValueCreateJSONString):
(JSValueToNumber):
(JSValueToStringCopy):
(JSValueToObject):
When seeing an exception, let the inspector know there was an exception.

* inspector/JSGlobalObjectInspectorController.h:
* inspector/JSGlobalObjectInspectorController.cpp:
(Inspector::JSGlobalObjectInspectorController::JSGlobalObjectInspectorController):
(Inspector::JSGlobalObjectInspectorController::appendAPIBacktrace):
(Inspector::JSGlobalObjectInspectorController::reportAPIException):
Log API exceptions by also grabbing the native backtrace.

* inspector/ScriptCallStack.h:
* inspector/ScriptCallStack.cpp:
(Inspector::ScriptCallStack::firstNonNativeCallFrame):
(Inspector::ScriptCallStack::append):
Minor extensions to ScriptCallStack to make it easier to work with.

* inspector/ConsoleMessage.cpp:
(Inspector::ConsoleMessage::ConsoleMessage):
(Inspector::ConsoleMessage::autogenerateMetadata):
Provide better default information if the first call frame was native.

* inspector/ScriptCallStackFactory.cpp:
(Inspector::createScriptCallStack):
(Inspector::extractSourceInformationFromException):
(Inspector::createScriptCallStackFromException):
Perform the handling here of inserting a fake call frame for exceptions
if there was no call stack (e.g. a SyntaxError) or if the first call
frame had no information.

* inspector/ConsoleMessage.cpp:
(Inspector::ConsoleMessage::ConsoleMessage):
(Inspector::ConsoleMessage::autogenerateMetadata):
* inspector/ConsoleMessage.h:
* inspector/ScriptCallStackFactory.cpp:
(Inspector::createScriptCallStack):
(Inspector::createScriptCallStackForConsole):
* inspector/ScriptCallStackFactory.h:
* inspector/agents/InspectorConsoleAgent.cpp:
(Inspector::InspectorConsoleAgent::enable):
(Inspector::InspectorConsoleAgent::addMessageToConsole):
(Inspector::InspectorConsoleAgent::count):
* inspector/agents/JSGlobalObjectDebuggerAgent.cpp:
(Inspector::JSGlobalObjectDebuggerAgent::breakpointActionLog):
ConsoleMessage cleanup.

Source/WebCore:

Include some clean up of ConsoleMessage and ScriptCallStack construction.

Covered by existing tests.

* bindings/js/JSDOMBinding.cpp:
(WebCore::reportException):
Simplify code now that createStackTraceFromException handles it.

* page/ContentSecurityPolicy.cpp:
(WebCore::gatherSecurityPolicyViolationEventData):
(WebCore::ContentSecurityPolicy::reportViolation):
ScriptCallStack can give us the first non-native callframe.

* inspector/InspectorResourceAgent.cpp:
(WebCore::InspectorResourceAgent::buildInitiatorObject):
* inspector/PageDebuggerAgent.cpp:
(WebCore::PageDebuggerAgent::breakpointActionLog):
* inspector/TimelineRecordFactory.cpp:
(WebCore::TimelineRecordFactory::createGenericRecord):
* page/Console.cpp:
(WebCore::internalAddMessage):
(WebCore::Console::profile):
(WebCore::Console::profileEnd):
(WebCore::Console::timeEnd):
* page/ContentSecurityPolicy.cpp:
(WebCore::gatherSecurityPolicyViolationEventData):
(WebCore::ContentSecurityPolicy::reportViolation):
* page/DOMWindow.cpp:
(WebCore::DOMWindow::postMessage):

Source/WebInspectorUI:

Reviewed by Timothy Hatcher.

* UserInterface/ConsoleMessageImpl.js:
(WebInspector.ConsoleMessageImpl.prototype._formatMessage):
(WebInspector.ConsoleMessageImpl.prototype._shouldHideURL):
(WebInspector.ConsoleMessageImpl.prototype._firstNonNativeCallFrame):
(WebInspector.ConsoleMessageImpl.prototype._populateStackTraceTreeElement):
Provide better handling for "[native code]" and legacy "undefined"
call frame URLs. Never linkify these. Also, when showing a link
for an exception, always use the first non-native call frame as
the link location.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@164486 268f45cc-cd09-0410-ab3c-d52691b4dbfc

25 files changed:
Source/JavaScriptCore/API/JSBase.cpp
Source/JavaScriptCore/API/JSObjectRef.cpp
Source/JavaScriptCore/API/JSValue.mm
Source/JavaScriptCore/API/JSValueRef.cpp
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/inspector/ConsoleMessage.cpp
Source/JavaScriptCore/inspector/ConsoleMessage.h
Source/JavaScriptCore/inspector/JSGlobalObjectInspectorController.cpp
Source/JavaScriptCore/inspector/JSGlobalObjectInspectorController.h
Source/JavaScriptCore/inspector/ScriptCallStack.cpp
Source/JavaScriptCore/inspector/ScriptCallStack.h
Source/JavaScriptCore/inspector/ScriptCallStackFactory.cpp
Source/JavaScriptCore/inspector/ScriptCallStackFactory.h
Source/JavaScriptCore/inspector/agents/InspectorConsoleAgent.cpp
Source/JavaScriptCore/inspector/agents/JSGlobalObjectDebuggerAgent.cpp
Source/WebCore/ChangeLog
Source/WebCore/bindings/js/JSDOMBinding.cpp
Source/WebCore/inspector/InspectorResourceAgent.cpp
Source/WebCore/inspector/PageDebuggerAgent.cpp
Source/WebCore/inspector/TimelineRecordFactory.cpp
Source/WebCore/page/Console.cpp
Source/WebCore/page/ContentSecurityPolicy.cpp
Source/WebCore/page/DOMWindow.cpp
Source/WebInspectorUI/ChangeLog
Source/WebInspectorUI/UserInterface/ConsoleMessageImpl.js

index bc66835..d40784b 100644 (file)
 #include "SourceCode.h"
 #include <wtf/text/StringHash.h>
 
+#if ENABLE(REMOTE_INSPECTOR)
+#include "JSGlobalObjectInspectorController.h"
+#endif
+
 using namespace JSC;
 
 JSValueRef JSEvaluateScript(JSContextRef ctx, JSStringRef script, JSObjectRef thisObject, JSStringRef sourceURL, int startingLineNumber, JSValueRef* exception)
@@ -65,6 +69,14 @@ JSValueRef JSEvaluateScript(JSContextRef ctx, JSStringRef script, JSObjectRef th
     if (evaluationException) {
         if (exception)
             *exception = toRef(exec, evaluationException);
+#if ENABLE(REMOTE_INSPECTOR)
+        // FIXME: If we have a debugger attached we could learn about ParseError exceptions through
+        // ScriptDebugServer::sourceParsed and this path could produce a duplicate warning. The
+        // Debugger path is currently ignored by inspector.
+        // NOTE: If we don't have a debugger, this SourceCode will be forever lost to the inspector.
+        // We could stash it in the inspector in case an inspector is ever opened.
+        globalObject->inspectorController().reportAPIException(exec, evaluationException);
+#endif
         return 0;
     }
 
@@ -94,6 +106,9 @@ bool JSCheckScriptSyntax(JSContextRef ctx, JSStringRef script, JSStringRef sourc
     if (!isValidSyntax) {
         if (exception)
             *exception = toRef(exec, syntaxException);
+#if ENABLE(REMOTE_INSPECTOR)
+        exec->vmEntryGlobalObject()->inspectorController().reportAPIException(exec, syntaxException);
+#endif
         return false;
     }
 
index 980eec3..870fd36 100644 (file)
 #include "PropertyNameArray.h"
 #include "RegExpConstructor.h"
 
+#if ENABLE(REMOTE_INSPECTOR)
+#include "JSGlobalObjectInspectorController.h"
+#endif
+
 using namespace JSC;
 
 JSClassRef JSClassCreate(const JSClassDefinition* definition)
@@ -145,9 +149,13 @@ JSObjectRef JSObjectMakeFunction(JSContextRef ctx, JSStringRef name, unsigned pa
 
     JSObject* result = constructFunction(exec, exec->lexicalGlobalObject(), args, nameID, sourceURL->string(), TextPosition(OrdinalNumber::fromOneBasedInt(startingLineNumber), OrdinalNumber::first()));
     if (exec->hadException()) {
+        JSValue exceptionValue = exec->exception();
         if (exception)
-            *exception = toRef(exec, exec->exception());
+            *exception = toRef(exec, exceptionValue);
         exec->clearException();
+#if ENABLE(REMOTE_INSPECTOR)
+        exec->vmEntryGlobalObject()->inspectorController().reportAPIException(exec, exceptionValue);
+#endif
         result = 0;
     }
     return toRef(result);
@@ -173,9 +181,13 @@ JSObjectRef JSObjectMakeArray(JSContextRef ctx, size_t argumentCount, const JSVa
         result = constructEmptyArray(exec, 0);
 
     if (exec->hadException()) {
+        JSValue exceptionValue = exec->exception();
         if (exception)
-            *exception = toRef(exec, exec->exception());
+            *exception = toRef(exec, exceptionValue);
         exec->clearException();
+#if ENABLE(REMOTE_INSPECTOR)
+        exec->vmEntryGlobalObject()->inspectorController().reportAPIException(exec, exceptionValue);
+#endif
         result = 0;
     }
 
@@ -197,9 +209,13 @@ JSObjectRef JSObjectMakeDate(JSContextRef ctx, size_t argumentCount, const JSVal
 
     JSObject* result = constructDate(exec, exec->lexicalGlobalObject(), argList);
     if (exec->hadException()) {
+        JSValue exceptionValue = exec->exception();
         if (exception)
-            *exception = toRef(exec, exec->exception());
+            *exception = toRef(exec, exceptionValue);
         exec->clearException();
+#if ENABLE(REMOTE_INSPECTOR)
+        exec->vmEntryGlobalObject()->inspectorController().reportAPIException(exec, exceptionValue);
+#endif
         result = 0;
     }
 
@@ -220,9 +236,13 @@ JSObjectRef JSObjectMakeError(JSContextRef ctx, size_t argumentCount, const JSVa
     JSObject* result = ErrorInstance::create(exec, errorStructure, message);
 
     if (exec->hadException()) {
+        JSValue exceptionValue = exec->exception();
         if (exception)
-            *exception = toRef(exec, exec->exception());
+            *exception = toRef(exec, exceptionValue);
         exec->clearException();
+#if ENABLE(REMOTE_INSPECTOR)
+        exec->vmEntryGlobalObject()->inspectorController().reportAPIException(exec, exceptionValue);
+#endif
         result = 0;
     }
 
@@ -244,9 +264,13 @@ JSObjectRef JSObjectMakeRegExp(JSContextRef ctx, size_t argumentCount, const JSV
 
     JSObject* result = constructRegExp(exec, exec->lexicalGlobalObject(),  argList);
     if (exec->hadException()) {
+        JSValue exceptionValue = exec->exception();
         if (exception)
-            *exception = toRef(exec, exec->exception());
+            *exception = toRef(exec, exceptionValue);
         exec->clearException();
+#if ENABLE(REMOTE_INSPECTOR)
+        exec->vmEntryGlobalObject()->inspectorController().reportAPIException(exec, exceptionValue);
+#endif
         result = 0;
     }
     
@@ -308,9 +332,13 @@ JSValueRef JSObjectGetProperty(JSContextRef ctx, JSObjectRef object, JSStringRef
 
     JSValue jsValue = jsObject->get(exec, propertyName->identifier(&exec->vm()));
     if (exec->hadException()) {
+        JSValue exceptionValue = exec->exception();
         if (exception)
-            *exception = toRef(exec, exec->exception());
+            *exception = toRef(exec, exceptionValue);
         exec->clearException();
+#if ENABLE(REMOTE_INSPECTOR)
+        exec->vmEntryGlobalObject()->inspectorController().reportAPIException(exec, exceptionValue);
+#endif
     }
     return toRef(exec, jsValue);
 }
@@ -337,9 +365,13 @@ void JSObjectSetProperty(JSContextRef ctx, JSObjectRef object, JSStringRef prope
     }
 
     if (exec->hadException()) {
+        JSValue exceptionValue = exec->exception();
         if (exception)
-            *exception = toRef(exec, exec->exception());
+            *exception = toRef(exec, exceptionValue);
         exec->clearException();
+#if ENABLE(REMOTE_INSPECTOR)
+        exec->vmEntryGlobalObject()->inspectorController().reportAPIException(exec, exceptionValue);
+#endif
     }
 }
 
@@ -356,9 +388,13 @@ JSValueRef JSObjectGetPropertyAtIndex(JSContextRef ctx, JSObjectRef object, unsi
 
     JSValue jsValue = jsObject->get(exec, propertyIndex);
     if (exec->hadException()) {
+        JSValue exceptionValue = exec->exception();
         if (exception)
-            *exception = toRef(exec, exec->exception());
+            *exception = toRef(exec, exceptionValue);
         exec->clearException();
+#if ENABLE(REMOTE_INSPECTOR)
+        exec->vmEntryGlobalObject()->inspectorController().reportAPIException(exec, exceptionValue);
+#endif
     }
     return toRef(exec, jsValue);
 }
@@ -378,9 +414,13 @@ void JSObjectSetPropertyAtIndex(JSContextRef ctx, JSObjectRef object, unsigned p
     
     jsObject->methodTable()->putByIndex(jsObject, exec, propertyIndex, jsValue, false);
     if (exec->hadException()) {
+        JSValue exceptionValue = exec->exception();
         if (exception)
-            *exception = toRef(exec, exec->exception());
+            *exception = toRef(exec, exceptionValue);
         exec->clearException();
+#if ENABLE(REMOTE_INSPECTOR)
+        exec->vmEntryGlobalObject()->inspectorController().reportAPIException(exec, exceptionValue);
+#endif
     }
 }
 
@@ -397,9 +437,13 @@ bool JSObjectDeleteProperty(JSContextRef ctx, JSObjectRef object, JSStringRef pr
 
     bool result = jsObject->methodTable()->deleteProperty(jsObject, exec, propertyName->identifier(&exec->vm()));
     if (exec->hadException()) {
+        JSValue exceptionValue = exec->exception();
         if (exception)
-            *exception = toRef(exec, exec->exception());
+            *exception = toRef(exec, exceptionValue);
         exec->clearException();
+#if ENABLE(REMOTE_INSPECTOR)
+        exec->vmEntryGlobalObject()->inspectorController().reportAPIException(exec, exceptionValue);
+#endif
     }
     return result;
 }
@@ -541,9 +585,13 @@ JSValueRef JSObjectCallAsFunction(JSContextRef ctx, JSObjectRef object, JSObject
 
     JSValueRef result = toRef(exec, call(exec, jsObject, callType, callData, jsThisObject, argList));
     if (exec->hadException()) {
+        JSValue exceptionValue = exec->exception();
         if (exception)
-            *exception = toRef(exec, exec->exception());
+            *exception = toRef(exec, exceptionValue);
         exec->clearException();
+#if ENABLE(REMOTE_INSPECTOR)
+        exec->vmEntryGlobalObject()->inspectorController().reportAPIException(exec, exceptionValue);
+#endif
         result = 0;
     }
     return result;
@@ -578,9 +626,13 @@ JSObjectRef JSObjectCallAsConstructor(JSContextRef ctx, JSObjectRef object, size
         argList.append(toJS(exec, arguments[i]));
     JSObjectRef result = toRef(construct(exec, jsObject, constructType, constructData, argList));
     if (exec->hadException()) {
+        JSValue exceptionValue = exec->exception();
         if (exception)
-            *exception = toRef(exec, exec->exception());
+            *exception = toRef(exec, exceptionValue);
         exec->clearException();
+#if ENABLE(REMOTE_INSPECTOR)
+        exec->vmEntryGlobalObject()->inspectorController().reportAPIException(exec, exceptionValue);
+#endif
         result = 0;
     }
     return result;
index 66ea7d8..1a9c373 100644 (file)
@@ -20,7 +20,7 @@
  * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
  * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
  * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
- * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. 
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
 #include "config.h"
 #import <wtf/text/WTFString.h>
 #import <wtf/text/StringHash.h>
 
+#if ENABLE(REMOTE_INSPECTOR)
+#import "CallFrame.h"
+#import "JSGlobalObject.h"
+#import "JSGlobalObjectInspectorController.h"
+#endif
+
 #if JSC_OBJC_API_ENABLED
 
 NSString * const JSPropertyDescriptorWritableKey = @"writable";
@@ -629,6 +635,14 @@ JSContainerConvertor::Task JSContainerConvertor::take()
     return last;
 }
 
+#if ENABLE(REMOTE_INSPECTOR)
+static void reportExceptionToInspector(JSGlobalContextRef context, JSC::JSValue exception)
+{
+    JSC::ExecState* exec = toJS(context);
+    exec->vmEntryGlobalObject()->inspectorController().reportAPIException(exec, exception);
+}
+#endif
+
 static JSContainerConvertor::Task valueToObjectWithoutCopy(JSGlobalContextRef context, JSValueRef value)
 {
     if (!JSValueIsObject(context, value)) {
@@ -781,8 +795,13 @@ id valueToArray(JSGlobalContextRef context, JSValueRef value, JSValueRef* except
         return containerValueToObject(context, (JSContainerConvertor::Task){ value, [NSMutableArray array], ContainerArray});
 
     JSC::APIEntryShim shim(toJS(context));
-    if (!(JSValueIsNull(context, value) || JSValueIsUndefined(context, value)))
-        *exception = toRef(JSC::createTypeError(toJS(context), ASCIILiteral("Cannot convert primitive to NSArray")));
+    if (!(JSValueIsNull(context, value) || JSValueIsUndefined(context, value))) {
+        JSC::JSObject* exceptionObject = JSC::createTypeError(toJS(context), ASCIILiteral("Cannot convert primitive to NSArray"));
+        *exception = toRef(exceptionObject);
+#if ENABLE(REMOTE_INSPECTOR)
+        reportExceptionToInspector(context, exceptionObject);
+#endif
+    }
     return nil;
 }
 
@@ -798,8 +817,13 @@ id valueToDictionary(JSGlobalContextRef context, JSValueRef value, JSValueRef* e
         return containerValueToObject(context, (JSContainerConvertor::Task){ value, [NSMutableDictionary dictionary], ContainerDictionary});
 
     JSC::APIEntryShim shim(toJS(context));
-    if (!(JSValueIsNull(context, value) || JSValueIsUndefined(context, value)))
-        *exception = toRef(JSC::createTypeError(toJS(context), ASCIILiteral("Cannot convert primitive to NSDictionary")));
+    if (!(JSValueIsNull(context, value) || JSValueIsUndefined(context, value))) {
+        JSC::JSObject* exceptionObject = JSC::createTypeError(toJS(context), ASCIILiteral("Cannot convert primitive to NSDictionary"));
+        *exception = toRef(exceptionObject);
+#if ENABLE(REMOTE_INSPECTOR)
+        reportExceptionToInspector(context, exceptionObject);
+#endif
+    }
     return nil;
 }
 
index 6f6f896..a7a4ac6 100644 (file)
@@ -20,7 +20,7 @@
  * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
  * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
  * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
- * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. 
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
 #include "config.h"
 #include <mach-o/dyld.h>
 #endif
 
+#if ENABLE(REMOTE_INSPECTOR)
+#include "JSGlobalObjectInspectorController.h"
+#endif
+
 using namespace JSC;
 
 #if PLATFORM(MAC)
@@ -202,9 +206,13 @@ bool JSValueIsEqual(JSContextRef ctx, JSValueRef a, JSValueRef b, JSValueRef* ex
 
     bool result = JSValue::equal(exec, jsA, jsB); // false if an exception is thrown
     if (exec->hadException()) {
+        JSValue exceptionValue = exec->exception();
         if (exception)
-            *exception = toRef(exec, exec->exception());
+            *exception = toRef(exec, exceptionValue);
         exec->clearException();
+#if ENABLE(REMOTE_INSPECTOR)
+        exec->vmEntryGlobalObject()->inspectorController().reportAPIException(exec, exceptionValue);
+#endif
     }
     return result;
 }
@@ -240,9 +248,13 @@ bool JSValueIsInstanceOfConstructor(JSContextRef ctx, JSValueRef value, JSObject
         return false;
     bool result = jsConstructor->hasInstance(exec, jsValue); // false if an exception is thrown
     if (exec->hadException()) {
+        JSValue exceptionValue = exec->exception();
         if (exception)
-            *exception = toRef(exec, exec->exception());
+            *exception = toRef(exec, exceptionValue);
         exec->clearException();
+#if ENABLE(REMOTE_INSPECTOR)
+        exec->vmEntryGlobalObject()->inspectorController().reportAPIException(exec, exceptionValue);
+#endif
     }
     return result;
 }
@@ -344,9 +356,13 @@ JSStringRef JSValueCreateJSONString(JSContextRef ctx, JSValueRef apiValue, unsig
     if (exception)
         *exception = 0;
     if (exec->hadException()) {
+        JSValue exceptionValue = exec->exception();
         if (exception)
-            *exception = toRef(exec, exec->exception());
+            *exception = toRef(exec, exceptionValue);
         exec->clearException();
+#if ENABLE(REMOTE_INSPECTOR)
+        exec->vmEntryGlobalObject()->inspectorController().reportAPIException(exec, exceptionValue);
+#endif
         return 0;
     }
     return OpaqueJSString::create(result).leakRef();
@@ -378,9 +394,13 @@ double JSValueToNumber(JSContextRef ctx, JSValueRef value, JSValueRef* exception
 
     double number = jsValue.toNumber(exec);
     if (exec->hadException()) {
+        JSValue exceptionValue = exec->exception();
         if (exception)
-            *exception = toRef(exec, exec->exception());
+            *exception = toRef(exec, exceptionValue);
         exec->clearException();
+#if ENABLE(REMOTE_INSPECTOR)
+        exec->vmEntryGlobalObject()->inspectorController().reportAPIException(exec, exceptionValue);
+#endif
         number = QNaN;
     }
     return number;
@@ -399,9 +419,13 @@ JSStringRef JSValueToStringCopy(JSContextRef ctx, JSValueRef value, JSValueRef*
     
     RefPtr<OpaqueJSString> stringRef(OpaqueJSString::create(jsValue.toString(exec)->value(exec)));
     if (exec->hadException()) {
+        JSValue exceptionValue = exec->exception();
         if (exception)
-            *exception = toRef(exec, exec->exception());
+            *exception = toRef(exec, exceptionValue);
         exec->clearException();
+#if ENABLE(REMOTE_INSPECTOR)
+        exec->vmEntryGlobalObject()->inspectorController().reportAPIException(exec, exceptionValue);
+#endif
         stringRef.clear();
     }
     return stringRef.release().leakRef();
@@ -420,13 +444,17 @@ JSObjectRef JSValueToObject(JSContextRef ctx, JSValueRef value, JSValueRef* exce
     
     JSObjectRef objectRef = toRef(jsValue.toObject(exec));
     if (exec->hadException()) {
+        JSValue exceptionValue = exec->exception();
         if (exception)
-            *exception = toRef(exec, exec->exception());
+            *exception = toRef(exec, exceptionValue);
         exec->clearException();
+#if ENABLE(REMOTE_INSPECTOR)
+        exec->vmEntryGlobalObject()->inspectorController().reportAPIException(exec, exceptionValue);
+#endif
         objectRef = 0;
     }
     return objectRef;
-}    
+}
 
 void JSValueProtect(JSContextRef ctx, JSValueRef value)
 {
index 49d06c2..76fab56 100644 (file)
@@ -1,3 +1,87 @@
+2014-02-21  Joseph Pecoraro  <pecoraro@apple.com>
+
+        Web Inspector: JSContext inspection should report exceptions in the console
+        https://bugs.webkit.org/show_bug.cgi?id=128776
+
+        Reviewed by Timothy Hatcher.
+
+        When JavaScript API functions have an exception, let the inspector
+        know so it can log the JavaScript and Native backtrace that caused
+        the exception.
+
+        Include some clean up of ConsoleMessage and ScriptCallStack construction.
+
+        * API/JSBase.cpp:
+        (JSEvaluateScript):
+        (JSCheckScriptSyntax):
+        * API/JSObjectRef.cpp:
+        (JSObjectMakeFunction):
+        (JSObjectMakeArray):
+        (JSObjectMakeDate):
+        (JSObjectMakeError):
+        (JSObjectMakeRegExp):
+        (JSObjectGetProperty):
+        (JSObjectSetProperty):
+        (JSObjectGetPropertyAtIndex):
+        (JSObjectSetPropertyAtIndex):
+        (JSObjectDeleteProperty):
+        (JSObjectCallAsFunction):
+        (JSObjectCallAsConstructor):
+        * API/JSValue.mm:
+        (reportExceptionToInspector):
+        (valueToArray):
+        (valueToDictionary):
+        * API/JSValueRef.cpp:
+        (JSValueIsEqual):
+        (JSValueIsInstanceOfConstructor):
+        (JSValueCreateJSONString):
+        (JSValueToNumber):
+        (JSValueToStringCopy):
+        (JSValueToObject):
+        When seeing an exception, let the inspector know there was an exception.
+
+        * inspector/JSGlobalObjectInspectorController.h:
+        * inspector/JSGlobalObjectInspectorController.cpp:
+        (Inspector::JSGlobalObjectInspectorController::JSGlobalObjectInspectorController):
+        (Inspector::JSGlobalObjectInspectorController::appendAPIBacktrace):
+        (Inspector::JSGlobalObjectInspectorController::reportAPIException):
+        Log API exceptions by also grabbing the native backtrace.
+
+        * inspector/ScriptCallStack.h:
+        * inspector/ScriptCallStack.cpp:
+        (Inspector::ScriptCallStack::firstNonNativeCallFrame):
+        (Inspector::ScriptCallStack::append):
+        Minor extensions to ScriptCallStack to make it easier to work with.
+
+        * inspector/ConsoleMessage.cpp:
+        (Inspector::ConsoleMessage::ConsoleMessage):
+        (Inspector::ConsoleMessage::autogenerateMetadata):
+        Provide better default information if the first call frame was native.
+
+        * inspector/ScriptCallStackFactory.cpp:
+        (Inspector::createScriptCallStack):
+        (Inspector::extractSourceInformationFromException):
+        (Inspector::createScriptCallStackFromException):
+        Perform the handling here of inserting a fake call frame for exceptions
+        if there was no call stack (e.g. a SyntaxError) or if the first call
+        frame had no information.
+
+        * inspector/ConsoleMessage.cpp:
+        (Inspector::ConsoleMessage::ConsoleMessage):
+        (Inspector::ConsoleMessage::autogenerateMetadata):
+        * inspector/ConsoleMessage.h:
+        * inspector/ScriptCallStackFactory.cpp:
+        (Inspector::createScriptCallStack):
+        (Inspector::createScriptCallStackForConsole):
+        * inspector/ScriptCallStackFactory.h:
+        * inspector/agents/InspectorConsoleAgent.cpp:
+        (Inspector::InspectorConsoleAgent::enable):
+        (Inspector::InspectorConsoleAgent::addMessageToConsole):
+        (Inspector::InspectorConsoleAgent::count):
+        * inspector/agents/JSGlobalObjectDebuggerAgent.cpp:
+        (Inspector::JSGlobalObjectDebuggerAgent::breakpointActionLog):
+        ConsoleMessage cleanup.
+
 2014-02-20  Anders Carlsson  <andersca@apple.com>
 
         Modernize JSGlobalLock and JSLockHolder
index b3a752a..336e554 100644 (file)
@@ -45,7 +45,7 @@
 
 namespace Inspector {
 
-ConsoleMessage::ConsoleMessage(bool canGenerateCallStack, MessageSource source, MessageType type, MessageLevel level, const String& message, unsigned long requestIdentifier)
+ConsoleMessage::ConsoleMessage(MessageSource source, MessageType type, MessageLevel level, const String& message, unsigned long requestIdentifier)
     : m_source(source)
     , m_type(type)
     , m_level(level)
@@ -56,10 +56,9 @@ ConsoleMessage::ConsoleMessage(bool canGenerateCallStack, MessageSource source,
     , m_repeatCount(1)
     , m_requestId(IdentifiersFactory::requestId(requestIdentifier))
 {
-    autogenerateMetadata(canGenerateCallStack);
 }
 
-ConsoleMessage::ConsoleMessage(bool canGenerateCallStack, MessageSource source, MessageType type, MessageLevel level, const String& message, const String& url, unsigned line, unsigned column, JSC::ExecState* state, unsigned long requestIdentifier)
+ConsoleMessage::ConsoleMessage(MessageSource source, MessageType type, MessageLevel level, const String& message, const String& url, unsigned line, unsigned column, JSC::ExecState* state, unsigned long requestIdentifier)
     : m_source(source)
     , m_type(type)
     , m_level(level)
@@ -70,30 +69,31 @@ ConsoleMessage::ConsoleMessage(bool canGenerateCallStack, MessageSource source,
     , m_repeatCount(1)
     , m_requestId(IdentifiersFactory::requestId(requestIdentifier))
 {
-    autogenerateMetadata(canGenerateCallStack, state);
+    autogenerateMetadata(state);
 }
 
-ConsoleMessage::ConsoleMessage(bool, MessageSource source, MessageType type, MessageLevel level, const String& message, PassRefPtr<ScriptCallStack> callStack, unsigned long requestIdentifier)
+ConsoleMessage::ConsoleMessage(MessageSource source, MessageType type, MessageLevel level, const String& message, PassRefPtr<ScriptCallStack> callStack, unsigned long requestIdentifier)
     : m_source(source)
     , m_type(type)
     , m_level(level)
     , m_message(message)
-    , m_arguments(nullptr)
+    , m_url()
     , m_line(0)
     , m_column(0)
     , m_repeatCount(1)
     , m_requestId(IdentifiersFactory::requestId(requestIdentifier))
 {
-    if (callStack && callStack->size()) {
-        const ScriptCallFrame& frame = callStack->at(0);
-        m_url = frame.sourceURL();
-        m_line = frame.lineNumber();
-        m_column = frame.columnNumber();
-    }
     m_callStack = callStack;
+
+    const ScriptCallFrame* frame = m_callStack ? m_callStack->firstNonNativeCallFrame() : nullptr;
+    if (frame) {
+        m_url = frame->sourceURL();
+        m_line = frame->lineNumber();
+        m_column = frame->columnNumber();
+    }
 }
 
-ConsoleMessage::ConsoleMessage(bool canGenerateCallStack, MessageSource source, MessageType type, MessageLevel level, const String& message, PassRefPtr<ScriptArguments> arguments, JSC::ExecState* state, unsigned long requestIdentifier)
+ConsoleMessage::ConsoleMessage(MessageSource source, MessageType type, MessageLevel level, const String& message, PassRefPtr<ScriptArguments> arguments, JSC::ExecState* state, unsigned long requestIdentifier)
     : m_source(source)
     , m_type(type)
     , m_level(level)
@@ -105,35 +105,30 @@ ConsoleMessage::ConsoleMessage(bool canGenerateCallStack, MessageSource source,
     , m_repeatCount(1)
     , m_requestId(IdentifiersFactory::requestId(requestIdentifier))
 {
-    autogenerateMetadata(canGenerateCallStack, state);
+    autogenerateMetadata(state);
 }
 
 ConsoleMessage::~ConsoleMessage()
 {
 }
 
-// FIXME: Remove the generate without ExecState path. The caller should always provide an ExecState.
-void ConsoleMessage::autogenerateMetadata(bool /*canGenerateCallStack*/, JSC::ExecState* state)
+void ConsoleMessage::autogenerateMetadata(JSC::ExecState* state)
 {
-    if (m_type == MessageType::EndGroup)
+    if (!state)
         return;
 
-    if (state)
-        m_callStack = createScriptCallStackForConsole(state);
-    // else if (canGenerateCallStack)
-    //     m_callStack = createScriptCallStack(ScriptCallStack::maxCallStackSizeToCapture, true);
-    else
+    if (m_type == MessageType::EndGroup)
         return;
 
-    if (m_callStack && m_callStack->size()) {
-        const ScriptCallFrame& frame = m_callStack->at(0);
-        m_url = frame.sourceURL();
-        m_line = frame.lineNumber();
-        m_column = frame.columnNumber();
+    // FIXME: Should this really be using "for console" in the generic ConsoleMessage autogeneration? This can skip the first frame.
+    m_callStack = createScriptCallStackForConsole(state, ScriptCallStack::maxCallStackSizeToCapture);
+
+    if (const ScriptCallFrame* frame = m_callStack->firstNonNativeCallFrame()) {
+        m_url = frame->sourceURL();
+        m_line = frame->lineNumber();
+        m_column = frame->columnNumber();
         return;
     }
-
-    m_callStack.clear();
 }
 
 static Inspector::TypeBuilder::Console::ConsoleMessage::Source::Enum messageSourceValue(MessageSource source)
index 498eb1f..a54aa8d 100644 (file)
@@ -51,10 +51,10 @@ class JS_EXPORT_PRIVATE ConsoleMessage {
     WTF_MAKE_NONCOPYABLE(ConsoleMessage);
     WTF_MAKE_FAST_ALLOCATED;
 public:
-    ConsoleMessage(bool canGenerateCallStack, MessageSource, MessageType, MessageLevel, const String& message, unsigned long requestIdentifier = 0);
-    ConsoleMessage(bool canGenerateCallStack, MessageSource, MessageType, MessageLevel, const String& message, const String& url, unsigned line, unsigned column, JSC::ExecState* = nullptr, unsigned long requestIdentifier = 0);
-    ConsoleMessage(bool canGenerateCallStack, MessageSource, MessageType, MessageLevel, const String& message, PassRefPtr<ScriptCallStack>, unsigned long requestIdentifier = 0);
-    ConsoleMessage(bool canGenerateCallStack, MessageSource, MessageType, MessageLevel, const String& message, PassRefPtr<ScriptArguments>, JSC::ExecState*, unsigned long requestIdentifier = 0);
+    ConsoleMessage(MessageSource, MessageType, MessageLevel, const String& message, unsigned long requestIdentifier = 0);
+    ConsoleMessage(MessageSource, MessageType, MessageLevel, const String& message, const String& url, unsigned line, unsigned column, JSC::ExecState* = nullptr, unsigned long requestIdentifier = 0);
+    ConsoleMessage(MessageSource, MessageType, MessageLevel, const String& message, PassRefPtr<ScriptCallStack>, unsigned long requestIdentifier = 0);
+    ConsoleMessage(MessageSource, MessageType, MessageLevel, const String& message, PassRefPtr<ScriptArguments>, JSC::ExecState*, unsigned long requestIdentifier = 0);
     ~ConsoleMessage();
 
     void addToFrontend(InspectorConsoleFrontendDispatcher*, InjectedScriptManager*, bool generatePreview);
@@ -74,7 +74,7 @@ public:
     void clear();
 
 private:
-    void autogenerateMetadata(bool canGenerateCallStack, JSC::ExecState* = nullptr);
+    void autogenerateMetadata(JSC::ExecState* = nullptr);
 
     MessageSource m_source;
     MessageType m_type;
index 4dec996..0288bcf 100644 (file)
@@ -29,6 +29,7 @@
 #if ENABLE(INSPECTOR)
 
 #include "Completion.h"
+#include "ErrorHandlingScope.h"
 #include "InjectedScriptHost.h"
 #include "InjectedScriptManager.h"
 #include "InspectorAgent.h"
 #include "JSGlobalObjectConsoleAgent.h"
 #include "JSGlobalObjectDebuggerAgent.h"
 #include "JSGlobalObjectRuntimeAgent.h"
+#include "ScriptCallStack.h"
+#include "ScriptCallStackFactory.h"
+#include <cxxabi.h>
+#include <dlfcn.h>
+#include <execinfo.h>
 
 using namespace JSC;
 
@@ -52,6 +58,8 @@ JSGlobalObjectInspectorController::JSGlobalObjectInspectorController(JSGlobalObj
     auto consoleAgent = std::make_unique<JSGlobalObjectConsoleAgent>(m_injectedScriptManager.get());
     auto debuggerAgent = std::make_unique<JSGlobalObjectDebuggerAgent>(m_injectedScriptManager.get(), m_globalObject, consoleAgent.get());
 
+    m_consoleAgent = consoleAgent.get();
+
     runtimeAgent->setScriptDebugServer(&debuggerAgent->scriptDebugServer());
 
     m_agents.append(std::make_unique<InspectorAgent>());
@@ -101,6 +109,51 @@ void JSGlobalObjectInspectorController::dispatchMessageFromFrontend(const String
         m_inspectorBackendDispatcher->dispatch(message);
 }
 
+void JSGlobalObjectInspectorController::appendAPIBacktrace(ScriptCallStack* callStack)
+{
+    static const int framesToShow = 31;
+    static const int framesToSkip = 3; // WTFGetBacktrace, appendAPIBacktrace, reportAPIException.
+
+    void* samples[framesToShow + framesToSkip];
+    int frames = framesToShow + framesToSkip;
+    WTFGetBacktrace(samples, &frames);
+
+    void** stack = samples + framesToSkip;
+    int size = frames - framesToSkip;
+    for (int i = 0; i < size; ++i) {
+        const char* mangledName = nullptr;
+        char* cxaDemangled = nullptr;
+        Dl_info info;
+        if (dladdr(stack[i], &info) && info.dli_sname)
+            mangledName = info.dli_sname;
+        if (mangledName)
+            cxaDemangled = abi::__cxa_demangle(mangledName, nullptr, nullptr, nullptr);
+        if (mangledName || cxaDemangled)
+            callStack->append(ScriptCallFrame(cxaDemangled ? cxaDemangled : mangledName, ASCIILiteral("[native code]"), 0, 0));
+        else
+            callStack->append(ScriptCallFrame(ASCIILiteral("?"), ASCIILiteral("[native code]"), 0, 0));
+        free(cxaDemangled);
+    }
+}
+
+void JSGlobalObjectInspectorController::reportAPIException(ExecState* exec, JSValue exception)
+{
+    if (isTerminatedExecutionException(exception))
+        return;
+
+    ErrorHandlingScope errorScope(exec->vm());
+
+    RefPtr<ScriptCallStack> callStack = createScriptCallStackFromException(exec, exception, ScriptCallStack::maxCallStackSizeToCapture);
+    appendAPIBacktrace(callStack.get());
+
+    // FIXME: <http://webkit.org/b/115087> Web Inspector: Should not evaluate JavaScript handling exceptions
+    // If this is a custom exception object, call toString on it to try and get a nice string representation for the exception.
+    String errorMessage = exception.toString(exec)->value(exec);
+    exec->clearException();
+
+    m_consoleAgent->addMessageToConsole(MessageSource::JS, MessageType::Log, MessageLevel::Error, errorMessage, callStack);
+}
+
 InspectorFunctionCallHandler JSGlobalObjectInspectorController::functionCallHandler() const
 {
     return JSC::call;
index 377f40e..8266e59 100644 (file)
 namespace JSC {
 class ExecState;
 class JSGlobalObject;
+class JSValue;
 }
 
 namespace Inspector {
 
 class InjectedScriptManager;
 class InspectorBackendDispatcher;
+class InspectorConsoleAgent;
 class InspectorFrontendChannel;
+class ScriptCallStack;
 
 class JSGlobalObjectInspectorController final : public InspectorEnvironment {
     WTF_MAKE_NONCOPYABLE(JSGlobalObjectInspectorController);
@@ -58,6 +61,8 @@ public:
 
     void globalObjectDestroyed();
 
+    void reportAPIException(JSC::ExecState*, JSC::JSValue exception);
+
     virtual bool developerExtrasEnabled() const override { return true; }
     virtual bool canAccessInspectedScriptState(JSC::ExecState*) const override { return true; }
     virtual InspectorFunctionCallHandler functionCallHandler() const override;
@@ -66,8 +71,11 @@ public:
     virtual void didCallInjectedScriptFunction(JSC::ExecState*) override { }
 
 private:
+    void appendAPIBacktrace(ScriptCallStack* callStack);
+
     JSC::JSGlobalObject& m_globalObject;
     std::unique_ptr<InjectedScriptManager> m_injectedScriptManager;
+    InspectorConsoleAgent* m_consoleAgent;
     InspectorAgentRegistry m_agents;
     InspectorFrontendChannel* m_inspectorFrontendChannel;
     RefPtr<InspectorBackendDispatcher> m_inspectorBackendDispatcher;
index 2dd285f..cb70c21 100644 (file)
@@ -61,6 +61,25 @@ size_t ScriptCallStack::size() const
     return m_frames.size();
 }
 
+const ScriptCallFrame* ScriptCallStack::firstNonNativeCallFrame() const
+{
+    if (!m_frames.size())
+        return nullptr;
+
+    for (size_t i = 0; i < m_frames.size(); ++i) {
+        const ScriptCallFrame& frame = m_frames[i];
+        if (frame.sourceURL() != "[native code]")
+            return &frame;
+    }
+
+    return nullptr;
+}
+
+void ScriptCallStack::append(const ScriptCallFrame& frame)
+{
+    m_frames.append(frame);
+}
+
 bool ScriptCallStack::isEqual(ScriptCallStack* o) const
 {
     if (!o)
index ddaa2bf..85836d1 100644 (file)
@@ -54,6 +54,10 @@ public:
     const ScriptCallFrame& at(size_t) const;
     size_t size() const;
 
+    const ScriptCallFrame* firstNonNativeCallFrame() const;
+
+    void append(const ScriptCallFrame&);
+
     bool isEqual(ScriptCallStack*) const;
 
 #if ENABLE(INSPECTOR)
@@ -61,6 +65,7 @@ public:
 #endif
 
 private:
+    ScriptCallStack();
     ScriptCallStack(Vector<ScriptCallFrame>&);
 
     Vector<ScriptCallFrame> m_frames;
index 5637b4f..e4329b2 100644 (file)
@@ -83,27 +83,18 @@ private:
     size_t m_remainingCapacityForFrameCapture;
 };
 
-PassRefPtr<ScriptCallStack> createScriptCallStack(JSC::ExecState* exec, size_t maxStackSize, bool emptyIsAllowed)
+PassRefPtr<ScriptCallStack> createScriptCallStack(JSC::ExecState* exec, size_t maxStackSize)
 {
     Vector<ScriptCallFrame> frames;
 
-    if (exec) {
-        CallFrame* frame = exec->vm().topCallFrame;
-        CreateScriptCallStackFunctor functor(false, frames, maxStackSize);
-        frame->iterate(functor);
-    }
-
-    if (frames.isEmpty() && !emptyIsAllowed) {
-        // No frames found. It may happen in the case where
-        // a bound function is called from native code for example.
-        // Fallback to setting lineNumber to 0, and source and function name to "undefined".
-        frames.append(ScriptCallFrame(ASCIILiteral("undefined"), ASCIILiteral("undefined"), 0, 0));
-    }
+    CallFrame* frame = exec->vm().topCallFrame;
+    CreateScriptCallStackFunctor functor(false, frames, maxStackSize);
+    frame->iterate(functor);
 
     return ScriptCallStack::create(frames);
 }
 
-PassRefPtr<ScriptCallStack> createScriptCallStack(JSC::ExecState* exec, size_t maxStackSize)
+PassRefPtr<ScriptCallStack> createScriptCallStackForConsole(JSC::ExecState* exec, size_t maxStackSize)
 {
     Vector<ScriptCallFrame> frames;
 
@@ -119,10 +110,16 @@ PassRefPtr<ScriptCallStack> createScriptCallStack(JSC::ExecState* exec, size_t m
     return ScriptCallStack::create(frames);
 }
 
-PassRefPtr<ScriptCallStack> createScriptCallStackForConsole(JSC::ExecState* exec)
+static void extractSourceInformationFromException(JSC::ExecState* exec, JSObject* exceptionObject, int* lineNumber, int* columnNumber, String* sourceURL)
 {
-    // FIXME: Caller should use createScriptCallStack alternative with the exec and appropriate max.
-    return createScriptCallStack(exec, ScriptCallStack::maxCallStackSizeToCapture);
+    // FIXME: <http://webkit.org/b/115087> Web Inspector: Should not need to evaluate JavaScript handling exceptions
+    JSValue lineValue = exceptionObject->getDirect(exec->vm(), Identifier(exec, "line"));
+    *lineNumber = lineValue && lineValue.isNumber() ? int(lineValue.toNumber(exec)) : 0;
+    JSValue columnValue = exceptionObject->getDirect(exec->vm(), Identifier(exec, "column"));
+    *columnNumber = columnValue && columnValue.isNumber() ? int(columnValue.toNumber(exec)) : 0;
+    JSValue sourceURLValue = exceptionObject->getDirect(exec->vm(), Identifier(exec, "sourceURL"));
+    *sourceURL = sourceURLValue && sourceURLValue.isString() ? sourceURLValue.toString(exec)->value(exec) : String();
+    exec->clearException();
 }
 
 PassRefPtr<ScriptCallStack> createScriptCallStackFromException(JSC::ExecState* exec, JSC::JSValue& exception, size_t maxStackSize)
@@ -137,22 +134,24 @@ PassRefPtr<ScriptCallStack> createScriptCallStackFromException(JSC::ExecState* e
         unsigned column;
         stackTrace[i].computeLineAndColumn(line, column);
         String functionName = stackTrace[i].friendlyFunctionName(exec);
-        frames.append(ScriptCallFrame(functionName, stackTrace[i].sourceURL, line, column));
+        frames.append(ScriptCallFrame(functionName, stackTrace[i].friendlySourceURL(), line, column));
     }
 
-    // FIXME: <http://webkit.org/b/115087> Web Inspector: WebCore::reportException should not evaluate JavaScript handling exceptions
-    // Fallback to getting at least the line and sourceURL from the exception if it has values and the exceptionStack doesn't.
-    if (frames.size() > 0) {
-        const ScriptCallFrame& firstCallFrame = frames.first();
-        JSObject* exceptionObject = exception.toObject(exec);
-        if (exception.isObject() && firstCallFrame.sourceURL().isEmpty()) {
-            JSValue lineValue = exceptionObject->getDirect(exec->vm(), Identifier(exec, "line"));
-            int lineNumber = lineValue && lineValue.isNumber() ? int(lineValue.toNumber(exec)) : 0;
-            JSValue columnValue = exceptionObject->getDirect(exec->vm(), Identifier(exec, "column"));
-            int columnNumber = columnValue && columnValue.isNumber() ? int(columnValue.toNumber(exec)) : 0;
-            JSValue sourceURLValue = exceptionObject->getDirect(exec->vm(), Identifier(exec, "sourceURL"));
-            String exceptionSourceURL = sourceURLValue && sourceURLValue.isString() ? sourceURLValue.toString(exec)->value(exec) : ASCIILiteral("undefined");
-            frames[0] = ScriptCallFrame(firstCallFrame.functionName(), exceptionSourceURL, lineNumber, columnNumber);
+    // Fallback to getting at least the line and sourceURL from the exception object if it has values and the exceptionStack doesn't.
+    JSObject* exceptionObject = exception.toObject(exec);
+    if (exception.isObject()) {
+        int lineNumber;
+        int columnNumber;
+        String exceptionSourceURL;
+        if (!frames.size()) {
+            extractSourceInformationFromException(exec, exceptionObject, &lineNumber, &columnNumber, &exceptionSourceURL);
+            frames.append(ScriptCallFrame(String(), exceptionSourceURL, lineNumber, columnNumber));
+        } else {
+            const ScriptCallFrame& firstCallFrame = frames.first();
+            if (firstCallFrame.sourceURL().isEmpty()) {
+                extractSourceInformationFromException(exec, exceptionObject, &lineNumber, &columnNumber, &exceptionSourceURL);
+                frames[0] = ScriptCallFrame(firstCallFrame.functionName(), exceptionSourceURL, lineNumber, columnNumber);
+            }
         }
     }
 
index 7ce9c90..07c1586 100644 (file)
@@ -45,9 +45,8 @@ class ScriptArguments;
 class ScriptCallStack;
 
 // FIXME: The subtle differences between these should be eliminated.
-JS_EXPORT_PRIVATE PassRefPtr<ScriptCallStack> createScriptCallStack(JSC::ExecState*, size_t maxStackSize, bool emptyIsAllowed);
 JS_EXPORT_PRIVATE PassRefPtr<ScriptCallStack> createScriptCallStack(JSC::ExecState*, size_t maxStackSize);
-JS_EXPORT_PRIVATE PassRefPtr<ScriptCallStack> createScriptCallStackForConsole(JSC::ExecState*);
+JS_EXPORT_PRIVATE PassRefPtr<ScriptCallStack> createScriptCallStackForConsole(JSC::ExecState*, size_t maxStackSize);
 JS_EXPORT_PRIVATE PassRefPtr<ScriptCallStack> createScriptCallStackFromException(JSC::ExecState*, JSC::JSValue& exception, size_t maxStackSize);
 JS_EXPORT_PRIVATE PassRefPtr<ScriptArguments> createScriptArguments(JSC::ExecState*, unsigned skipArgumentCount);
 
index 72f60c1..37b2227 100644 (file)
@@ -80,7 +80,7 @@ void InspectorConsoleAgent::enable(ErrorString*)
     m_enabled = true;
 
     if (m_expiredConsoleMessageCount) {
-        ConsoleMessage expiredMessage(!isWorkerAgent(), MessageSource::Other, MessageType::Log, MessageLevel::Warning, String::format("%d console messages are not shown.", m_expiredConsoleMessageCount));
+        ConsoleMessage expiredMessage(MessageSource::Other, MessageType::Log, MessageLevel::Warning, String::format("%d console messages are not shown.", m_expiredConsoleMessageCount));
         expiredMessage.addToFrontend(m_frontendDispatcher.get(), m_injectedScriptManager, false);
     }
 
@@ -128,7 +128,7 @@ void InspectorConsoleAgent::addMessageToConsole(MessageSource source, MessageTyp
         clearMessages(&error);
     }
 
-    addConsoleMessage(std::make_unique<ConsoleMessage>(!isWorkerAgent(), source, type, level, message, callStack, requestIdentifier));
+    addConsoleMessage(std::make_unique<ConsoleMessage>(source, type, level, message, callStack, requestIdentifier));
 }
 
 void InspectorConsoleAgent::addMessageToConsole(MessageSource source, MessageType type, MessageLevel level, const String& message, JSC::ExecState* state, PassRefPtr<ScriptArguments> arguments, unsigned long requestIdentifier)
@@ -141,7 +141,7 @@ void InspectorConsoleAgent::addMessageToConsole(MessageSource source, MessageTyp
         clearMessages(&error);
     }
 
-    addConsoleMessage(std::make_unique<ConsoleMessage>(!isWorkerAgent(), source, type, level, message, arguments, state, requestIdentifier));
+    addConsoleMessage(std::make_unique<ConsoleMessage>(source, type, level, message, arguments, state, requestIdentifier));
 }
 
 void InspectorConsoleAgent::addMessageToConsole(MessageSource source, MessageType type, MessageLevel level, const String& message, const String& scriptID, unsigned lineNumber, unsigned columnNumber, JSC::ExecState* state, unsigned long requestIdentifier)
@@ -154,8 +154,7 @@ void InspectorConsoleAgent::addMessageToConsole(MessageSource source, MessageTyp
         clearMessages(&error);
     }
 
-    bool canGenerateCallStack = !isWorkerAgent() && m_frontendDispatcher;
-    addConsoleMessage(std::make_unique<ConsoleMessage>(canGenerateCallStack, source, type, level, message, scriptID, lineNumber, columnNumber, state, requestIdentifier));
+    addConsoleMessage(std::make_unique<ConsoleMessage>(source, type, level, message, scriptID, lineNumber, columnNumber, state, requestIdentifier));
 }
 
 Vector<unsigned> InspectorConsoleAgent::consoleMessageArgumentCounts() const
@@ -197,7 +196,7 @@ void InspectorConsoleAgent::stopTiming(const String& title, PassRefPtr<ScriptCal
 
 void InspectorConsoleAgent::count(JSC::ExecState* state, PassRefPtr<ScriptArguments> arguments)
 {
-    RefPtr<ScriptCallStack> callStack(createScriptCallStackForConsole(state));
+    RefPtr<ScriptCallStack> callStack(createScriptCallStackForConsole(state, ScriptCallStack::maxCallStackSizeToCapture));
     const ScriptCallFrame& lastCaller = callStack->at(0);
     // Follow Firebug's behavior of counting with null and undefined title in
     // the same bucket as no argument
index 2e1750b..c087e81 100644 (file)
@@ -69,7 +69,7 @@ InjectedScript JSGlobalObjectDebuggerAgent::injectedScriptForEval(ErrorString* e
 
 void JSGlobalObjectDebuggerAgent::breakpointActionLog(JSC::ExecState* exec, const String& message)
 {
-    m_consoleAgent->addMessageToConsole(MessageSource::JS, MessageType::Log, MessageLevel::Log, message, createScriptCallStack(exec, ScriptCallStack::maxCallStackSizeToCapture, true), 0);
+    m_consoleAgent->addMessageToConsole(MessageSource::JS, MessageType::Log, MessageLevel::Log, message, createScriptCallStack(exec, ScriptCallStack::maxCallStackSizeToCapture), 0);
 }
 
 } // namespace Inspector
index 0968a99..9a4eb60 100644 (file)
@@ -1,3 +1,40 @@
+2014-02-21  Joseph Pecoraro  <pecoraro@apple.com>
+
+        Web Inspector: JSContext inspection should report exceptions in the console
+        https://bugs.webkit.org/show_bug.cgi?id=128776
+
+        Reviewed by Timothy Hatcher.
+
+        Include some clean up of ConsoleMessage and ScriptCallStack construction.
+
+        Covered by existing tests.
+
+        * bindings/js/JSDOMBinding.cpp:
+        (WebCore::reportException):
+        Simplify code now that createStackTraceFromException handles it.
+
+        * page/ContentSecurityPolicy.cpp:
+        (WebCore::gatherSecurityPolicyViolationEventData):
+        (WebCore::ContentSecurityPolicy::reportViolation):
+        ScriptCallStack can give us the first non-native callframe.
+
+        * inspector/InspectorResourceAgent.cpp:
+        (WebCore::InspectorResourceAgent::buildInitiatorObject):
+        * inspector/PageDebuggerAgent.cpp:
+        (WebCore::PageDebuggerAgent::breakpointActionLog):
+        * inspector/TimelineRecordFactory.cpp:
+        (WebCore::TimelineRecordFactory::createGenericRecord):
+        * page/Console.cpp:
+        (WebCore::internalAddMessage):
+        (WebCore::Console::profile):
+        (WebCore::Console::profileEnd):
+        (WebCore::Console::timeEnd):
+        * page/ContentSecurityPolicy.cpp:
+        (WebCore::gatherSecurityPolicyViolationEventData):
+        (WebCore::ContentSecurityPolicy::reportViolation):
+        * page/DOMWindow.cpp:
+        (WebCore::DOMWindow::postMessage):
+
 2014-02-21  Martin Hodovan  <mhodovan@inf.u-szeged.hu>
 
         Fixing the !ENABLE(SVG_FONTS) build
index 58092ea..2b2f402 100644 (file)
@@ -171,20 +171,10 @@ void reportException(ExecState* exec, JSValue exception, CachedScript* cachedScr
     int lineNumber = 0;
     int columnNumber = 0;
     String exceptionSourceURL;
-    if (callStack->size()) {
-        const ScriptCallFrame& frame = callStack->at(0);
-        lineNumber = frame.lineNumber();
-        columnNumber = frame.columnNumber();
-        exceptionSourceURL = frame.sourceURL();
-    } else {
-        // There may not be an exceptionStack for a <script> SyntaxError. Fallback to getting at least the line and sourceURL from the exception.
-        JSObject* exceptionObject = exception.toObject(exec);
-        JSValue lineValue = exceptionObject->getDirect(exec->vm(), Identifier(exec, "line"));
-        lineNumber = lineValue && lineValue.isNumber() ? int(lineValue.toNumber(exec)) : 0;
-        JSValue columnValue = exceptionObject->getDirect(exec->vm(), Identifier(exec, "column"));
-        columnNumber = columnValue && columnValue.isNumber() ? int(columnValue.toNumber(exec)) : 0;
-        JSValue sourceURLValue = exceptionObject->getDirect(exec->vm(), Identifier(exec, "sourceURL"));
-        exceptionSourceURL = sourceURLValue && sourceURLValue.isString() ? sourceURLValue.toString(exec)->value(exec) : ASCIILiteral("undefined");
+    if (const ScriptCallFrame* callFrame = callStack->firstNonNativeCallFrame()) {
+        lineNumber = callFrame->lineNumber();
+        columnNumber = callFrame->columnNumber();
+        exceptionSourceURL = callFrame->sourceURL();
     }
 
     String errorMessage;
index 89e5cf4..a6548b6 100644 (file)
@@ -439,7 +439,7 @@ void InspectorResourceAgent::didScheduleStyleRecalculation(Document* document)
 
 PassRefPtr<Inspector::TypeBuilder::Network::Initiator> InspectorResourceAgent::buildInitiatorObject(Document* document)
 {
-    RefPtr<ScriptCallStack> stackTrace = createScriptCallStack(JSMainThreadExecState::currentState(), ScriptCallStack::maxCallStackSizeToCapture, true);
+    RefPtr<ScriptCallStack> stackTrace = createScriptCallStack(JSMainThreadExecState::currentState(), ScriptCallStack::maxCallStackSizeToCapture);
     if (stackTrace && stackTrace->size() > 0) {
         RefPtr<Inspector::TypeBuilder::Network::Initiator> initiatorObject = Inspector::TypeBuilder::Network::Initiator::create()
             .setType(Inspector::TypeBuilder::Network::Initiator::Type::Script);
index e4670cd..b536226 100644 (file)
@@ -117,7 +117,7 @@ void PageDebuggerAgent::unmuteConsole()
 
 void PageDebuggerAgent::breakpointActionLog(JSC::ExecState* exec, const String& message)
 {
-    m_pageAgent->page()->console().addMessage(MessageSource::JS, MessageLevel::Log, message, createScriptCallStack(exec, ScriptCallStack::maxCallStackSizeToCapture, true));
+    m_pageAgent->page()->console().addMessage(MessageSource::JS, MessageLevel::Log, message, createScriptCallStack(exec, ScriptCallStack::maxCallStackSizeToCapture));
 }
 
 InjectedScript PageDebuggerAgent::injectedScriptForEval(ErrorString* errorString, const int* executionContextId)
index d9da743..2d62276 100644 (file)
@@ -57,7 +57,7 @@ PassRefPtr<InspectorObject> TimelineRecordFactory::createGenericRecord(double st
     record->setNumber("startTime", startTime);
 
     if (maxCallStackDepth) {
-        RefPtr<ScriptCallStack> stackTrace = createScriptCallStack(JSMainThreadExecState::currentState(), maxCallStackDepth, true);
+        RefPtr<ScriptCallStack> stackTrace = createScriptCallStack(JSMainThreadExecState::currentState(), maxCallStackDepth);
         if (stackTrace && stackTrace->size())
             record->setValue("stackTrace", stackTrace->buildInspectorArray());
     }
index e73a6ed..790d703 100644 (file)
@@ -77,7 +77,7 @@ static void internalAddMessage(Page* page, MessageType type, MessageLevel level,
         return;
 
     size_t stackSize = printTrace ? ScriptCallStack::maxCallStackSizeToCapture : 1;
-    RefPtr<ScriptCallStack> callStack(createScriptCallStack(state, stackSize));
+    RefPtr<ScriptCallStack> callStack(createScriptCallStackForConsole(state, stackSize));
     const ScriptCallFrame& lastCaller = callStack->at(0);
 
     String message;
@@ -203,7 +203,7 @@ void Console::profile(JSC::ExecState* state, const String& title)
 
     ScriptProfiler::start(state, resolvedTitle);
 
-    RefPtr<ScriptCallStack> callStack(createScriptCallStack(state, 1));
+    RefPtr<ScriptCallStack> callStack(createScriptCallStackForConsole(state, 1));
     const ScriptCallFrame& lastCaller = callStack->at(0);
     InspectorInstrumentation::addStartProfilingMessageToConsole(page, resolvedTitle, lastCaller.lineNumber(), lastCaller.columnNumber(), lastCaller.sourceURL());
 }
@@ -222,7 +222,7 @@ void Console::profileEnd(JSC::ExecState* state, const String& title)
         return;
 
     m_profiles.append(profile);
-    RefPtr<ScriptCallStack> callStack(createScriptCallStack(state, 1));
+    RefPtr<ScriptCallStack> callStack(createScriptCallStackForConsole(state, 1));
     InspectorInstrumentation::addProfile(page, profile, callStack);
 }
 
@@ -233,7 +233,7 @@ void Console::time(const String& title)
 
 void Console::timeEnd(JSC::ExecState* state, const String& title)
 {
-    RefPtr<ScriptCallStack> callStack(createScriptCallStackForConsole(state));
+    RefPtr<ScriptCallStack> callStack(createScriptCallStackForConsole(state, 1));
     InspectorInstrumentation::stopConsoleTiming(m_frame, title, callStack.release());
 }
 
index 1fa112a..62365df 100644 (file)
@@ -171,15 +171,6 @@ FeatureObserver::Feature getFeatureObserverType(ContentSecurityPolicy::HeaderTyp
     return FeatureObserver::NumberOfFeatures;
 }
 
-const ScriptCallFrame& getFirstNonNativeFrame(PassRefPtr<ScriptCallStack> stack)
-{
-    int frameNumber = 0;
-    if (!stack->at(0).lineNumber() && stack->size() > 1 && stack->at(1).lineNumber())
-        frameNumber = 1;
-
-    return stack->at(frameNumber);
-}
-
 } // namespace
 
 static bool skipExactly(const UChar*& position, const UChar* end, UChar delimiter)
@@ -1727,16 +1718,12 @@ static void gatherSecurityPolicyViolationEventData(SecurityPolicyViolationEventI
     init.sourceFile = String();
     init.lineNumber = 0;
 
-    RefPtr<ScriptCallStack> stack = createScriptCallStack(JSMainThreadExecState::currentState(), 2, false);
-    if (!stack)
-        return;
-
-    const ScriptCallFrame& callFrame = getFirstNonNativeFrame(stack);
-
-    if (callFrame.lineNumber()) {
-        URL source = URL(ParsedURLString, callFrame.sourceURL());
+    RefPtr<ScriptCallStack> stack = createScriptCallStack(JSMainThreadExecState::currentState(), 2);
+    const ScriptCallFrame* callFrame = stack->firstNonNativeCallFrame();
+    if (callFrame && callFrame->lineNumber()) {
+        URL source = URL(URL(), callFrame->sourceURL());
         init.sourceFile = stripURLForUseInReport(document, source);
-        init.lineNumber = callFrame.lineNumber();
+        init.lineNumber = callFrame->lineNumber();
     }
 }
 #endif
@@ -1777,31 +1764,28 @@ void ContentSecurityPolicy::reportViolation(const String& directiveText, const S
     // harmless information.
 
     RefPtr<InspectorObject> cspReport = InspectorObject::create();
-    cspReport->setString("document-uri", document->url().strippedForUseAsReferrer());
-    cspReport->setString("referrer", document->referrer());
-    cspReport->setString("violated-directive", directiveText);
+    cspReport->setString(ASCIILiteral("document-uri"), document->url().strippedForUseAsReferrer());
+    cspReport->setString(ASCIILiteral("referrer"), document->referrer());
+    cspReport->setString(ASCIILiteral("violated-directive"), directiveText);
 #if ENABLE(CSP_NEXT)
     if (experimentalFeaturesEnabled())
-        cspReport->setString("effective-directive", effectiveDirective);
+        cspReport->setString(ASCIILiteral("effective-directive"), effectiveDirective);
 #else
     UNUSED_PARAM(effectiveDirective);
 #endif
-    cspReport->setString("original-policy", header);
-    cspReport->setString("blocked-uri", stripURLForUseInReport(document, blockedURL));
-
-    RefPtr<ScriptCallStack> stack = createScriptCallStack(JSMainThreadExecState::currentState(), 2, false);
-    if (stack) {
-        const ScriptCallFrame& callFrame = getFirstNonNativeFrame(stack);
-
-        if (callFrame.lineNumber()) {
-            URL source = URL(ParsedURLString, callFrame.sourceURL());
-            cspReport->setString("source-file", stripURLForUseInReport(document, source));
-            cspReport->setNumber("line-number", callFrame.lineNumber());
-        }
+    cspReport->setString(ASCIILiteral("original-policy"), header);
+    cspReport->setString(ASCIILiteral("blocked-uri"), stripURLForUseInReport(document, blockedURL));
+
+    RefPtr<ScriptCallStack> stack = createScriptCallStack(JSMainThreadExecState::currentState(), 2);
+    const ScriptCallFrame* callFrame = stack->firstNonNativeCallFrame();
+    if (callFrame && callFrame->lineNumber()) {
+        URL source = URL(URL(), callFrame->sourceURL());
+        cspReport->setString(ASCIILiteral("source-file"), stripURLForUseInReport(document, source));
+        cspReport->setNumber(ASCIILiteral("line-number"), callFrame->lineNumber());
     }
 
     RefPtr<InspectorObject> reportObject = InspectorObject::create();
-    reportObject->setObject("csp-report", cspReport.release());
+    reportObject->setObject(ASCIILiteral("csp-report"), cspReport.release());
 
     RefPtr<FormData> report = FormData::create(reportObject->toJSONString().utf8());
 
index 4fa4330..e730b08 100644 (file)
@@ -846,7 +846,7 @@ void DOMWindow::postMessage(PassRefPtr<SerializedScriptValue> message, const Mes
     // Capture stack trace only when inspector front-end is loaded as it may be time consuming.
     RefPtr<ScriptCallStack> stackTrace;
     if (InspectorInstrumentation::consoleAgentEnabled(sourceDocument))
-        stackTrace = createScriptCallStack(JSMainThreadExecState::currentState(), ScriptCallStack::maxCallStackSizeToCapture, true);
+        stackTrace = createScriptCallStack(JSMainThreadExecState::currentState(), ScriptCallStack::maxCallStackSizeToCapture);
 
     // Schedule the message.
     PostMessageTimer* timer = new PostMessageTimer(this, message, sourceOrigin, &source, std::move(channels), target.get(), stackTrace.release());
index 1a51178..24db090 100644 (file)
@@ -1,3 +1,20 @@
+2014-02-21  Joseph Pecoraro  <pecoraro@apple.com>
+
+        Web Inspector: JSContext inspection should report exceptions in the console
+        https://bugs.webkit.org/show_bug.cgi?id=128776
+
+        Reviewed by Timothy Hatcher.
+
+        * UserInterface/ConsoleMessageImpl.js:
+        (WebInspector.ConsoleMessageImpl.prototype._formatMessage):
+        (WebInspector.ConsoleMessageImpl.prototype._shouldHideURL):
+        (WebInspector.ConsoleMessageImpl.prototype._firstNonNativeCallFrame):
+        (WebInspector.ConsoleMessageImpl.prototype._populateStackTraceTreeElement):
+        Provide better handling for "[native code]" and legacy "undefined"
+        call frame URLs. Never linkify these. Also, when showing a link
+        for an exception, always use the first non-native call frame as
+        the link location.
+
 2014-02-21  Antoine Quint  <graouts@webkit.org>
 
         Web Inspector: scrollbar may appear when selecting a stop in gradient editor
index a61ffc2..a25a045 100644 (file)
@@ -109,10 +109,11 @@ WebInspector.ConsoleMessageImpl.prototype = {
         }
 
         if (this.source !== WebInspector.ConsoleMessage.MessageSource.Network || this._request) {
-            if (this._stackTrace && this._stackTrace.length && this._stackTrace[0].url) {
-                var urlElement = this._linkifyCallFrame(this._stackTrace[0]);
+            var firstNonNativeCallFrame = this._firstNonNativeCallFrame();
+            if (firstNonNativeCallFrame) {
+                var urlElement = this._linkifyCallFrame(firstNonNativeCallFrame);
                 this._formattedMessage.appendChild(urlElement);
-            } else if (this.url && this.url !== "undefined") {
+            } else if (this.url && !this._shouldHideURL(this.url)) {
                 var urlElement = this._linkifyLocation(this.url, this.line, this.column);
                 this._formattedMessage.appendChild(urlElement);
             }
@@ -145,6 +146,26 @@ WebInspector.ConsoleMessageImpl.prototype = {
         return !!this._stackTrace && this._stackTrace.length && (this.source === WebInspector.ConsoleMessage.MessageSource.Network || this.level === WebInspector.ConsoleMessage.MessageLevel.Error || this.type === WebInspector.ConsoleMessage.MessageType.Trace);
     },
 
+    _shouldHideURL: function(url)
+    {
+        return url === "undefined" || url === "[native code]";
+    },
+
+    _firstNonNativeCallFrame: function()
+    {
+        if (!this._stackTrace)
+            return null;
+
+        for (var i = 0; i < this._stackTrace.length; i++) {
+            var frame = this._stackTrace[i];
+            if (!frame.url || frame.url === "[native code]")
+                continue;
+            return frame;
+        }
+
+        return null;
+    },
+
     get message()
     {
         // force message formatting
@@ -526,7 +547,7 @@ WebInspector.ConsoleMessageImpl.prototype = {
             messageTextElement.appendChild(document.createTextNode(functionName));
             content.appendChild(messageTextElement);
 
-            if (frame.url) {
+            if (frame.url && !this._shouldHideURL(frame.url)) {
                 var urlElement = this._linkifyCallFrame(frame);
                 content.appendChild(urlElement);
             }