Extension Error Piping - Blink: WebKit Side

Source/core/page/Console.cpp

 /*
  * Copyright (C) 2007 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  *
  * 1.  Redistributions of source code must retain the above copyright
  *     notice, this list of conditions and the following disclaimer.
  * 2.  Redistributions in binary form must reproduce the above copyright
@@ skipping 51 matching lines @@
 static void internalAddMessage(Page* page, MessageType type, MessageLevel level, ScriptState* state, PassRefPtr<ScriptArguments> prpArguments, bool acceptNoArguments = false, bool printTrace = false)
 {
     RefPtr<ScriptArguments> arguments = prpArguments;
 
     if (!page)
         return;
 
     if (!acceptNoArguments && !arguments->argumentCount())
         return;
 
-    size_t stackSize = printTrace ? ScriptCallStack::maxCallStackSizeToCapture : 1;
-    RefPtr<ScriptCallStack> callStack(createScriptCallStack(state, stackSize));
-    const ScriptCallFrame& lastCaller = callStack->at(0);
-
     String message;
     bool gotMessage = arguments->getFirstArgumentAsString(message);
     InspectorInstrumentation::addMessageToConsole(page, ConsoleAPIMessageSource, type, level, message, state, arguments);
 
-    if (gotMessage)
-        page->chrome().client()->addMessageToConsole(ConsoleAPIMessageSource, type, level, message, lastCaller.lineNumber(), lastCaller.sourceURL());
+    if (gotMessage) {
+        RefPtr<ScriptCallStack> callStack(createScriptCallStack(state, 1));
+        if (page->chrome().client()->shouldReportDetailedMessageForSource(callStack->at(0).sourceURL())) {

[pfeldman, 2013/08/08 17:13:52]

Is it true that you are now collecting stack traces for all evals (empty URLs)
twice unconditionally?

[Devlin, 2013/08/12 16:09:41]

Short answer: Nope.

Sorry, I didn't upload the corresponding chrome-side update - I was more worried
about nailing down the fundamentals of the Blink half, since we've gone through
so many iterations.

shouldReportDetailedMessageForSource() now takes in a URL and returns true if
and only if the url matches the extension scheme (i.e., anything with
chrome-extension://).  We get the stack trace of one frame in order to find the
last caller (which we need in either case) to check this URL, and only if it's
an extension URL do we go back and get the rest of the trace.

As for evals, experimenting with it a bit has shown that it's not really viable
to check for errors with them.  The ScriptState (and as such, the context) is
inaccessible in the case of a crippling error - the kind we're really interested
in.  So even though we could potentially report things like console.log, it's
not useful enough to warrant the added complexity (since then you could also
just to console.log(new Error().stack)).

For things other than evals (like event/background pages, content scripts,
browser actions/event callbacks, etc), we can determine whether or not it's an
extension through the URL alone.

Also, this addresses Adam's concern about exposing v8 to the WebViewClient
interface.

[pfeldman, 2013/08/12 16:15:34]

Ok!

+            callStack = createScriptCallStack(ScriptCallStack::maxCallStackSizeToCapture);
+            for (size_t i = 0; i < callStack->size(); ++i) {
+                const ScriptCallFrame& frame = callStack->at(i);
+                message.append("\n    at " + (frame.functionName().length() ? frame.functionName() : String("(anonymous function)")) + " (" + frame.sourceURL() + ":" + String::number(frame.lineNumber()) + ":" + String::number(frame.columnNumber()) + ")");
+            }
+        }
+        page->chrome().client()->addMessageToConsole(ConsoleAPIMessageSource, type, level, message, callStack->at(0).lineNumber(), callStack->at(0).sourceURL());
+    }
 }
 
 void Console::debug(ScriptState* state, PassRefPtr<ScriptArguments> arguments)
 {
     internalAddMessage(page(), LogMessageType, DebugMessageLevel, state, arguments);
 }
 
 void Console::error(ScriptState* state, PassRefPtr<ScriptArguments> arguments)
 {
     internalAddMessage(page(), LogMessageType, ErrorMessageLevel, state, arguments);
@@ skipping 138 matching lines @@
 }
 
 Page* Console::page() const
 {
     if (!m_frame)
         return 0;
     return m_frame->page();
 }
 
 } // namespace WebCore