Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
290633009:
DevTools: Show detailed information for exceptions during snippet execution. (Closed)
|
Created:
6 years, 7 months ago by kozyatinskiy1 Modified:
6 years, 5 months ago CC:
blink-reviews, vsevik+blink_chromium.org, caseq+blink_chromium.org, arv+blink, eustas+blink_chromium.org, malch+blink_chromium.org, yurys+blink_chromium.org, lushnikov+blink_chromium.org, abarth-chromium, loislo+blink_chromium.org, pfeldman+blink_chromium.org, paulirish+reviews_chromium.org, blink-reviews-bindings_chromium.org, devtools-reviews_chromium.org, apavlov+blink_chromium.org, sergeyv+blink_chromium.org, aandrey+blink_chromium.org Base URL:
https://chromium.googlesource.com/chromium/blink.git@master Visibility:
Public. |
DescriptionAdd Debugger.ExceptionDetails to protocol. Use ExceptionDetails for pass error when exception in snippet (run or compile).
Console show the position of exception when exception happens in snippet.
BUG=346013
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=175794
Patch Set 1 : #
Total comments: 6
Patch Set 2 : #Patch Set 3 : #
Total comments: 3
Patch Set 4 : #
Total comments: 4
Patch Set 5 : #Patch Set 6 : #
Total comments: 2
Patch Set 7 : #
Total comments: 5
Patch Set 8 : #Patch Set 9 : #Patch Set 10 : #
Total comments: 1
Messages
Total messages: 46 (0 generated)
https://chromiumcodereview.appspot.com/290633009/diff/10008/Source/bindings/v... File Source/bindings/v8/ScriptDebugServer.cpp (right): https://chromiumcodereview.appspot.com/290633009/diff/10008/Source/bindings/v... Source/bindings/v8/ScriptDebugServer.cpp:617: if (lineNumber) This won't be needed once default parameters are removed. We assume these should always be passed in here. https://chromiumcodereview.appspot.com/290633009/diff/10008/Source/bindings/v... Source/bindings/v8/ScriptDebugServer.cpp:618: *lineNumber = message->GetLineNumber(); extra space https://chromiumcodereview.appspot.com/290633009/diff/10008/Source/bindings/v... Source/bindings/v8/ScriptDebugServer.cpp:662: *lineNumber = message->GetLineNumber(); ditto https://chromiumcodereview.appspot.com/290633009/diff/10008/Source/bindings/v... File Source/bindings/v8/ScriptDebugServer.h (right): https://chromiumcodereview.appspot.com/290633009/diff/10008/Source/bindings/v... Source/bindings/v8/ScriptDebugServer.h:101: virtual void compileScript(ScriptState*, const String& expression, const String& sourceURL, String* scriptId, String* exceptionMessage, int* lineNumber = 0, int* columnNumber = 0, RefPtr<ScriptCallStack>* stackTrace = 0); Why do you need this default parameters values? https://chromiumcodereview.appspot.com/290633009/diff/10008/Source/core/inspe... File Source/core/inspector/InspectorDebuggerAgent.cpp (right): https://chromiumcodereview.appspot.com/290633009/diff/10008/Source/core/inspe... Source/core/inspector/InspectorDebuggerAgent.cpp:958: exceptionMessage = ExceptionMessage::create().setText(exceptionMessageText); We should only do this when scriptIdValue is not set. It is optional after all. if (scriptIdValue) return; https://chromiumcodereview.appspot.com/290633009/diff/10008/Source/devtools/f... File Source/devtools/front_end/sdk/ScriptSnippetModel.js (right): https://chromiumcodereview.appspot.com/290633009/diff/10008/Source/devtools/f... Source/devtools/front_end/sdk/ScriptSnippetModel.js:299: var consoleMessage = new WebInspector.ConsoleMessage( This is hard to read. Let's split this into two if-branches: if (wasThrown) ... Also we could probably reuse the same ConsoleMessage constructor call for both compile and run failures.
Overall looks good, let's add a test and we can land it.
https://chromiumcodereview.appspot.com/290633009/diff/50001/Source/devtools/p... File Source/devtools/protocol.json (right): https://chromiumcodereview.appspot.com/290633009/diff/50001/Source/devtools/p... Source/devtools/protocol.json:2912: "id": "ExceptionMessage", Exception details https://chromiumcodereview.appspot.com/290633009/diff/50001/Source/devtools/p... Source/devtools/protocol.json:2914: "description": "Exception message.", Detailed information on exception (or error) that was thrown during script compilation or execution. https://chromiumcodereview.appspot.com/290633009/diff/50001/Source/devtools/p... Source/devtools/protocol.json:3138: { "name": "exceptionMessage", "$ref": "ExceptionMessage", "optional": true, "description": "Exception message."} exceptionDetails here and in other places.
Also please give it a more meaningful name, e.g. DevTools: Show detailed information for exceptions during snippet execution.
On 2014/05/22 05:40:28, vsevik wrote: > Also please give it a more meaningful name, e.g. > DevTools: Show detailed information for exceptions during snippet execution. Done. On 2014/05/21 13:16:32, vsevik wrote: > https://chromiumcodereview.appspot.com/290633009/diff/50001/Source/devtools/p... > File Source/devtools/protocol.json (right): > > https://chromiumcodereview.appspot.com/290633009/diff/50001/Source/devtools/p... > Source/devtools/protocol.json:2912: "id": "ExceptionMessage", > Exception details Done. > https://chromiumcodereview.appspot.com/290633009/diff/50001/Source/devtools/p... > Source/devtools/protocol.json:2914: "description": "Exception message.", > Detailed information on exception (or error) that was thrown during script > compilation or execution. Done. > https://chromiumcodereview.appspot.com/290633009/diff/50001/Source/devtools/p... > Source/devtools/protocol.json:3138: { "name": "exceptionMessage", "$ref": > "ExceptionMessage", "optional": true, "description": "Exception message."} > exceptionDetails here and in other places. Done. On 2014/05/21 13:16:24, vsevik wrote: > Overall looks good, let's add a test and we can land it. Update existing test debugger-compile-and-run.html.
https://chromiumcodereview.appspot.com/290633009/diff/70001/LayoutTests/inspe... File LayoutTests/inspector/sources/debugger/debugger-compile-and-run.html (right): https://chromiumcodereview.appspot.com/290633009/diff/70001/LayoutTests/inspe... LayoutTests/inspector/sources/debugger/debugger-compile-and-run.html:19: for (var i = 0; i < stack.length && i < 100; ++i) Please put { on the same line here (we only put { on the next line for functions in blink) https://chromiumcodereview.appspot.com/290633009/diff/70001/LayoutTests/inspe... LayoutTests/inspector/sources/debugger/debugger-compile-and-run.html:33: var contextId = undefined; = undefined is redundant.
https://chromiumcodereview.appspot.com/290633009/diff/70001/LayoutTests/inspe... File LayoutTests/inspector/sources/debugger/debugger-compile-and-run.html (right): https://chromiumcodereview.appspot.com/290633009/diff/70001/LayoutTests/inspe... LayoutTests/inspector/sources/debugger/debugger-compile-and-run.html:33: var contextId = undefined; On 2014/05/22 15:40:46, vsevik wrote: > = undefined is redundant. Actually passing this variable is also redundant. Optional protocol parameters could be skipped.
https://chromiumcodereview.appspot.com/290633009/diff/70001/Source/core/inspe... File Source/core/inspector/InspectorDebuggerAgent.cpp (right): https://chromiumcodereview.appspot.com/290633009/diff/70001/Source/core/inspe... Source/core/inspector/InspectorDebuggerAgent.cpp:958: if (!!scriptIdValue) "if (scriptIdValue)" is well enough.
On 2014/05/22 15:56:15, vsevik wrote: > https://chromiumcodereview.appspot.com/290633009/diff/70001/Source/core/inspe... > File Source/core/inspector/InspectorDebuggerAgent.cpp (right): > > https://chromiumcodereview.appspot.com/290633009/diff/70001/Source/core/inspe... > Source/core/inspector/InspectorDebuggerAgent.cpp:958: if (!!scriptIdValue) > "if (scriptIdValue)" is well enough. Changed to if (!scriptIdValue.isEmpty()): https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit... On 2014/05/22 15:42:01, vsevik wrote: > https://chromiumcodereview.appspot.com/290633009/diff/70001/LayoutTests/inspe... > File LayoutTests/inspector/sources/debugger/debugger-compile-and-run.html > (right): > > https://chromiumcodereview.appspot.com/290633009/diff/70001/LayoutTests/inspe... > LayoutTests/inspector/sources/debugger/debugger-compile-and-run.html:33: var > contextId = undefined; > On 2014/05/22 15:40:46, vsevik wrote: > > = undefined is redundant. > > Actually passing this variable is also redundant. Optional protocol parameters > could be skipped. Done. On 2014/05/22 15:40:46, vsevik wrote: > https://chromiumcodereview.appspot.com/290633009/diff/70001/LayoutTests/inspe... > File LayoutTests/inspector/sources/debugger/debugger-compile-and-run.html > (right): > > https://chromiumcodereview.appspot.com/290633009/diff/70001/LayoutTests/inspe... > LayoutTests/inspector/sources/debugger/debugger-compile-and-run.html:19: for > (var i = 0; i < stack.length && i < 100; ++i) > Please put { on the same line here (we only put { on the next line for functions > in blink) > > https://chromiumcodereview.appspot.com/290633009/diff/70001/LayoutTests/inspe... > LayoutTests/inspector/sources/debugger/debugger-compile-and-run.html:33: var > contextId = undefined; > = undefined is redundant. Done.
lgtm
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/kozyatinskiy@google.com/290633009/100001
The CQ bit was unchecked by commit-bot@chromium.org
Failed to apply patch for Source/bindings/v8/ScriptDebugServer.cpp:
While running patch -p1 --forward --force --no-backup-if-mismatch;
patching file Source/bindings/v8/ScriptDebugServer.cpp
Hunk #1 succeeded at 34 with fuzz 2.
Hunk #2 FAILED at 601.
Hunk #3 FAILED at 613.
Hunk #4 FAILED at 633.
Hunk #5 FAILED at 655.
4 out of 5 hunks FAILED -- saving rejects to file
Source/bindings/v8/ScriptDebugServer.cpp.rej
Patch: Source/bindings/v8/ScriptDebugServer.cpp
Index: Source/bindings/v8/ScriptDebugServer.cpp
diff --git a/Source/bindings/v8/ScriptDebugServer.cpp
b/Source/bindings/v8/ScriptDebugServer.cpp
index
7db4eb02cd996383b0388890019af6ea2843f27a..ba28f7372024919fda2f06fe8e9f1385bba5e037
100644
--- a/Source/bindings/v8/ScriptDebugServer.cpp
+++ b/Source/bindings/v8/ScriptDebugServer.cpp
@@ -34,6 +34,7 @@
#include "DebuggerScriptSource.h"
#include "V8JavaScriptCallFrame.h"
#include "bindings/v8/ScopedPersistent.h"
+#include "bindings/v8/ScriptCallStackFactory.h"
#include "bindings/v8/ScriptController.h"
#include "bindings/v8/ScriptObject.h"
#include "bindings/v8/ScriptSourceCode.h"
@@ -600,7 +601,7 @@ bool ScriptDebugServer::isPaused()
return !m_executionState.isEmpty();
}
-void ScriptDebugServer::compileScript(ScriptState* scriptState, const String&
expression, const String& sourceURL, String* scriptId, String* exceptionMessage,
int* lineNumber, int* columnNumber)
+void ScriptDebugServer::compileScript(ScriptState* scriptState, const String&
expression, const String& sourceURL, String* scriptId, String*
exceptionDetailsText, int* lineNumber, int* columnNumber,
RefPtr<ScriptCallStack>* stackTrace)
{
if (scriptState->contextIsEmpty())
return;
@@ -612,11 +613,10 @@ void ScriptDebugServer::compileScript(ScriptState*
scriptState, const String& ex
if (tryCatch.HasCaught()) {
v8::Local<v8::Message> message = tryCatch.Message();
if (!message.IsEmpty()) {
- *exceptionMessage =
toCoreStringWithUndefinedOrNullCheck(message->Get());
- if (lineNumber)
- *lineNumber = message->GetLineNumber();
- if (columnNumber)
- *columnNumber = message->GetStartColumn();
+ *exceptionDetailsText =
toCoreStringWithUndefinedOrNullCheck(message->Get());
+ *lineNumber = message->GetLineNumber();
+ *columnNumber = message->GetStartColumn();
+ *stackTrace = createScriptCallStack(message->GetStackTrace(),
ScriptCallStack::maxCallStackSizeToCapture, m_isolate);
}
return;
}
@@ -632,7 +632,7 @@ void ScriptDebugServer::clearCompiledScripts()
m_compiledScripts.clear();
}
-void ScriptDebugServer::runScript(ScriptState* scriptState, const String&
scriptId, ScriptValue* result, bool* wasThrown, String* exceptionMessage, int*
lineNumber, int* columnNumber)
+void ScriptDebugServer::runScript(ScriptState* scriptState, const String&
scriptId, ScriptValue* result, bool* wasThrown, String* exceptionDetailsText,
int* lineNumber, int* columnNumber, RefPtr<ScriptCallStack>* stackTrace)
{
if (!m_compiledScripts.contains(scriptId))
return;
@@ -654,11 +654,10 @@ void ScriptDebugServer::runScript(ScriptState*
scriptState, const String& script
*result = ScriptValue(scriptState, tryCatch.Exception());
v8::Local<v8::Message> message = tryCatch.Message();
if (!message.IsEmpty()) {
- *exceptionMessage =
toCoreStringWithUndefinedOrNullCheck(message->Get());
- if (lineNumber)
- *lineNumber = message->GetLineNumber();
- if (columnNumber)
- *columnNumber = message->GetStartColumn();
+ *exceptionDetailsText =
toCoreStringWithUndefinedOrNullCheck(message->Get());
+ *lineNumber = message->GetLineNumber();
+ *columnNumber = message->GetStartColumn();
+ *stackTrace = createScriptCallStack(message->GetStackTrace(),
ScriptCallStack::maxCallStackSizeToCapture, m_isolate);
}
} else {
*result = ScriptValue(scriptState, value);
On 2014/05/22 16:43:03, I haz the power (commit-bot) wrote:
> Failed to apply patch for Source/bindings/v8/ScriptDebugServer.cpp:
> While running patch -p1 --forward --force --no-backup-if-mismatch;
> patching file Source/bindings/v8/ScriptDebugServer.cpp
> Hunk #1 succeeded at 34 with fuzz 2.
> Hunk #2 FAILED at 601.
> Hunk #3 FAILED at 613.
> Hunk #4 FAILED at 633.
> Hunk #5 FAILED at 655.
> 4 out of 5 hunks FAILED -- saving rejects to file
> Source/bindings/v8/ScriptDebugServer.cpp.rej
>
> Patch: Source/bindings/v8/ScriptDebugServer.cpp
> Index: Source/bindings/v8/ScriptDebugServer.cpp
> diff --git a/Source/bindings/v8/ScriptDebugServer.cpp
> b/Source/bindings/v8/ScriptDebugServer.cpp
> index
>
7db4eb02cd996383b0388890019af6ea2843f27a..ba28f7372024919fda2f06fe8e9f1385bba5e037
> 100644
> --- a/Source/bindings/v8/ScriptDebugServer.cpp
> +++ b/Source/bindings/v8/ScriptDebugServer.cpp
> @@ -34,6 +34,7 @@
> #include "DebuggerScriptSource.h"
> #include "V8JavaScriptCallFrame.h"
> #include "bindings/v8/ScopedPersistent.h"
> +#include "bindings/v8/ScriptCallStackFactory.h"
> #include "bindings/v8/ScriptController.h"
> #include "bindings/v8/ScriptObject.h"
> #include "bindings/v8/ScriptSourceCode.h"
> @@ -600,7 +601,7 @@ bool ScriptDebugServer::isPaused()
> return !m_executionState.isEmpty();
> }
>
> -void ScriptDebugServer::compileScript(ScriptState* scriptState, const String&
> expression, const String& sourceURL, String* scriptId, String*
exceptionMessage,
> int* lineNumber, int* columnNumber)
> +void ScriptDebugServer::compileScript(ScriptState* scriptState, const String&
> expression, const String& sourceURL, String* scriptId, String*
> exceptionDetailsText, int* lineNumber, int* columnNumber,
> RefPtr<ScriptCallStack>* stackTrace)
> {
> if (scriptState->contextIsEmpty())
> return;
> @@ -612,11 +613,10 @@ void ScriptDebugServer::compileScript(ScriptState*
> scriptState, const String& ex
> if (tryCatch.HasCaught()) {
> v8::Local<v8::Message> message = tryCatch.Message();
> if (!message.IsEmpty()) {
> - *exceptionMessage =
> toCoreStringWithUndefinedOrNullCheck(message->Get());
> - if (lineNumber)
> - *lineNumber = message->GetLineNumber();
> - if (columnNumber)
> - *columnNumber = message->GetStartColumn();
> + *exceptionDetailsText =
> toCoreStringWithUndefinedOrNullCheck(message->Get());
> + *lineNumber = message->GetLineNumber();
> + *columnNumber = message->GetStartColumn();
> + *stackTrace = createScriptCallStack(message->GetStackTrace(),
> ScriptCallStack::maxCallStackSizeToCapture, m_isolate);
> }
> return;
> }
> @@ -632,7 +632,7 @@ void ScriptDebugServer::clearCompiledScripts()
> m_compiledScripts.clear();
> }
>
> -void ScriptDebugServer::runScript(ScriptState* scriptState, const String&
> scriptId, ScriptValue* result, bool* wasThrown, String* exceptionMessage, int*
> lineNumber, int* columnNumber)
> +void ScriptDebugServer::runScript(ScriptState* scriptState, const String&
> scriptId, ScriptValue* result, bool* wasThrown, String* exceptionDetailsText,
> int* lineNumber, int* columnNumber, RefPtr<ScriptCallStack>* stackTrace)
> {
> if (!m_compiledScripts.contains(scriptId))
> return;
> @@ -654,11 +654,10 @@ void ScriptDebugServer::runScript(ScriptState*
> scriptState, const String& script
> *result = ScriptValue(scriptState, tryCatch.Exception());
> v8::Local<v8::Message> message = tryCatch.Message();
> if (!message.IsEmpty()) {
> - *exceptionMessage =
> toCoreStringWithUndefinedOrNullCheck(message->Get());
> - if (lineNumber)
> - *lineNumber = message->GetLineNumber();
> - if (columnNumber)
> - *columnNumber = message->GetStartColumn();
> + *exceptionDetailsText =
> toCoreStringWithUndefinedOrNullCheck(message->Get());
> + *lineNumber = message->GetLineNumber();
> + *columnNumber = message->GetStartColumn();
> + *stackTrace = createScriptCallStack(message->GetStackTrace(),
> ScriptCallStack::maxCallStackSizeToCapture, m_isolate);
> }
> } else {
> *result = ScriptValue(scriptState, value);
Patch fixed.
The CQ bit was checked by vsevik@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/kozyatinskiy@google.com/290633009/120001
FYI, CQ is re-trying this CL (attempt #1). Please consider checking whether the failures are real, and report flakes to chrome-troopers@google.com. The failing builders are: blink_presubmit on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/blink_presubmit/builds/6163)
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: blink_presubmit on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/blink_presubmit/builds/6164)
pfeldmn@, yurys@ Could you please OWNERS lgtm this (bindings/v8)?
pfeldman@, yurys@ Could you please OWNERS lgtm this (bindings/v8)?
lgtm https://codereview.chromium.org/290633009/diff/120001/Source/bindings/v8/Scri... File Source/bindings/v8/ScriptDebugServer.cpp (right): https://codereview.chromium.org/290633009/diff/120001/Source/bindings/v8/Scri... Source/bindings/v8/ScriptDebugServer.cpp:619: *stackTrace = createScriptCallStack(message->GetStackTrace(), ScriptCallStack::maxCallStackSizeToCapture, m_isolate); We should pass message->GetStackTrace()->GetFrameCount(); or MAX_INT instead of ScriptCallStack::maxCallStackSizeToCapture as we want to wrap full stack here. https://codereview.chromium.org/290633009/diff/120001/Source/bindings/v8/Scri... Source/bindings/v8/ScriptDebugServer.cpp:660: *stackTrace = createScriptCallStack(message->GetStackTrace(), ScriptCallStack::maxCallStackSizeToCapture, m_isolate); ditto
> https://codereview.chromium.org/290633009/diff/120001/Source/bindings/v8/Scri... > File Source/bindings/v8/ScriptDebugServer.cpp (right): > > https://codereview.chromium.org/290633009/diff/120001/Source/bindings/v8/Scri... > Source/bindings/v8/ScriptDebugServer.cpp:619: *stackTrace = > createScriptCallStack(message->GetStackTrace(), > ScriptCallStack::maxCallStackSizeToCapture, m_isolate); > We should pass message->GetStackTrace()->GetFrameCount(); or MAX_INT instead of > ScriptCallStack::maxCallStackSizeToCapture as we want to wrap full stack here. Done. > https://codereview.chromium.org/290633009/diff/120001/Source/bindings/v8/Scri... > Source/bindings/v8/ScriptDebugServer.cpp:660: *stackTrace = > createScriptCallStack(message->GetStackTrace(), > ScriptCallStack::maxCallStackSizeToCapture, m_isolate); > ditto Done.
FYI https://codereview.chromium.org/290633009/diff/140001/LayoutTests/inspector/s... File LayoutTests/inspector/sources/debugger/debugger-compile-and-run.html (right): https://codereview.chromium.org/290633009/diff/140001/LayoutTests/inspector/s... LayoutTests/inspector/sources/debugger/debugger-compile-and-run.html:10: InspectorTest.addResult('exceptionDetails:') nit: use "" instead of '' https://codereview.chromium.org/290633009/diff/140001/LayoutTests/inspector/s... LayoutTests/inspector/sources/debugger/debugger-compile-and-run.html:16: InspectorTest.addResult(" no stack trace attached to exceptionDetails"); nit: "FAIL: no stack trace..." to indicate explicitly of the test failure
lgtm https://codereview.chromium.org/290633009/diff/140001/Source/core/inspector/I... File Source/core/inspector/InspectorDebuggerAgent.cpp (right): https://codereview.chromium.org/290633009/diff/140001/Source/core/inspector/I... Source/core/inspector/InspectorDebuggerAgent.cpp:939: void InspectorDebuggerAgent::compileScript(ErrorString* errorString, const String& expression, const String& sourceURL, const int* executionContextId, TypeBuilder::OptOutput<ScriptId>* scriptId, RefPtr<TypeBuilder::Debugger::ExceptionDetails>& exceptionDetails) TypeBuilder::Debugger::ExceptionDetails -> ExceptionDetails https://codereview.chromium.org/290633009/diff/140001/Source/core/inspector/I... Source/core/inspector/InspectorDebuggerAgent.cpp:968: void InspectorDebuggerAgent::runScript(ErrorString* errorString, const ScriptId& scriptId, const int* executionContextId, const String* const objectGroup, const bool* const doNotPauseOnExceptionsAndMuteConsole, RefPtr<RemoteObject>& result, RefPtr<TypeBuilder::Debugger::ExceptionDetails>& exceptionDetails) ditto https://codereview.chromium.org/290633009/diff/140001/Source/devtools/front_e... File Source/devtools/front_end/sdk/ScriptSnippetModel.js (right): https://codereview.chromium.org/290633009/diff/140001/Source/devtools/front_e... Source/devtools/front_end/sdk/ScriptSnippetModel.js:273: var wasThrown = !!exceptionDetails; remove this var?
> https://codereview.chromium.org/290633009/diff/140001/LayoutTests/inspector/s... > File LayoutTests/inspector/sources/debugger/debugger-compile-and-run.html > (right): > > https://codereview.chromium.org/290633009/diff/140001/LayoutTests/inspector/s... > LayoutTests/inspector/sources/debugger/debugger-compile-and-run.html:10: > InspectorTest.addResult('exceptionDetails:') > nit: use "" instead of '' Done. > https://codereview.chromium.org/290633009/diff/140001/LayoutTests/inspector/s... > LayoutTests/inspector/sources/debugger/debugger-compile-and-run.html:16: > InspectorTest.addResult(" no stack trace attached to exceptionDetails"); > nit: "FAIL: no stack trace..." to indicate explicitly of the test failure It is not a failed test if there was a compile error. Second part of test is about this. > https://codereview.chromium.org/290633009/diff/140001/Source/core/inspector/I... > File Source/core/inspector/InspectorDebuggerAgent.cpp (right): > > https://codereview.chromium.org/290633009/diff/140001/Source/core/inspector/I... > Source/core/inspector/InspectorDebuggerAgent.cpp:939: void > InspectorDebuggerAgent::compileScript(ErrorString* errorString, const String& > expression, const String& sourceURL, const int* executionContextId, > TypeBuilder::OptOutput<ScriptId>* scriptId, > RefPtr<TypeBuilder::Debugger::ExceptionDetails>& exceptionDetails) > TypeBuilder::Debugger::ExceptionDetails -> ExceptionDetails Done. > https://codereview.chromium.org/290633009/diff/140001/Source/core/inspector/I... > Source/core/inspector/InspectorDebuggerAgent.cpp:968: void > InspectorDebuggerAgent::runScript(ErrorString* errorString, const ScriptId& > scriptId, const int* executionContextId, const String* const objectGroup, const > bool* const doNotPauseOnExceptionsAndMuteConsole, RefPtr<RemoteObject>& result, > RefPtr<TypeBuilder::Debugger::ExceptionDetails>& exceptionDetails) > ditto Done. > https://codereview.chromium.org/290633009/diff/140001/Source/devtools/front_e... > File Source/devtools/front_end/sdk/ScriptSnippetModel.js (right): > > https://codereview.chromium.org/290633009/diff/140001/Source/devtools/front_e... > Source/devtools/front_end/sdk/ScriptSnippetModel.js:273: var wasThrown = > !!exceptionDetails; > remove this var? Done.
The CQ bit was checked by vsevik@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/kozyatinskiy@google.com/290633009/180001
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: blink_presubmit on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/blink_presubmit/builds/7052) linux_blink_dbg on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/linux_blink_dbg/builds/1...) linux_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/1...) mac_blink_compile_dbg on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/mac_blink_compile_dbg/bu...) mac_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/10439)
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: mac_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/10447)
This one needs a rebaseline.
On 2014/06/05 15:57:31, vsevik wrote: > This one needs a rebaseline. Done.
The CQ bit was checked by vsevik@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/kozyatinskiy@google.com/290633009/190001
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: blink_presubmit on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/blink_presubmit/builds/7166) linux_blink_dbg on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/linux_blink_dbg/builds/1...) linux_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/1...) mac_blink_compile_dbg on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/mac_blink_compile_dbg/bu...) mac_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/10611)
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: mac_blink_compile_dbg on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/mac_blink_compile_dbg/bu...) mac_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/10621)
The CQ bit was checked by kozyatinskiy@google.com
The CQ bit was unchecked by kozyatinskiy@google.com
The CQ bit was checked by vsevik@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/kozyatinskiy@google.com/290633009/230001
Message was sent while issue was closed.
Change committed as 175794
Message was sent while issue was closed.
FYI https://codereview.chromium.org/290633009/diff/230001/Source/devtools/front_e... File Source/devtools/front_end/sdk/ScriptSnippetModel.js (right): https://codereview.chromium.org/290633009/diff/230001/Source/devtools/front_e... Source/devtools/front_end/sdk/ScriptSnippetModel.js:345: exceptionDetails.stackTrace); We also need to pass Console.AsyncStackTrace with the exceptionDetails. I can do this unless you'd like to.
Message was sent while issue was closed.
On 2014/07/19 13:15:59, aandrey wrote: > FYI > > https://codereview.chromium.org/290633009/diff/230001/Source/devtools/front_e... > File Source/devtools/front_end/sdk/ScriptSnippetModel.js (right): > > https://codereview.chromium.org/290633009/diff/230001/Source/devtools/front_e... > Source/devtools/front_end/sdk/ScriptSnippetModel.js:345: > exceptionDetails.stackTrace); > We also need to pass Console.AsyncStackTrace with the exceptionDetails. I can do > this unless you'd like to. I am not sure there exists a scenario where AsyncStackTrace would make sense for snippets execution / console evaluation. We are making a protocol call here that returns synchronously. Could you please clarify what you mean? |
