Created attachment 42462 [details] A test page What steps will reproduce the problem? 1. Open the attached page. 2. Open devtools and make sure 'Pause on exception' is on(by default it is). 3. Click 'Test' button. What is the expected output? Execution is paused inside f(); What do you see instead? Exception is recorded in console but script execution is not paused in the Scripts panel and you cannot inspect the call stack. This is a regression introduced by http://trac.webkit.org/changeset/48610
I think we should change MessageCallback added to v8 by V8Proxy. Currently the callback is V8ConsoleMessage::handler but it seems that it just needs to dispatch the exception message to the right handler(page's document or worker context).
(In reply to comment #1) > I think we should change MessageCallback added to v8 by V8Proxy. Currently the > callback is V8ConsoleMessage::handler but it seems that it just needs to > dispatch the exception message to the right handler(page's document or worker > context). I'm not too familiar with how the devtools hook in here. Can you upload a patch that does the above so we can see what it looks like in code?
(In reply to comment #2) > (In reply to comment #1) > > I think we should change MessageCallback added to v8 by V8Proxy. Currently the > > callback is V8ConsoleMessage::handler but it seems that it just needs to > > dispatch the exception message to the right handler(page's document or worker > > context). > > I'm not too familiar with how the devtools hook in here. Can you upload a > patch that does the above so we can see what it looks like in code? We bisected regression to this change and it does not seem to justify well enough why it decided to temporarily set verbose to false in this very place of the code. The suggested fix is to basically revert trunk/WebCore/bindings/v8/V8AbstractEventListener.cpp change introduced in http://trac.webkit.org/changeset/48610. In fact that is why comment was initially a follow up to that bug. Note that there is jam's change that happened in between, but i think it should stay.
Looks like if jianli broke it, he should be the one fixing this, right?
Created attachment 42501 [details] Proposed Patch
Comment on attachment 42501 [details] Proposed Patch Thanks for fixing this! We should make "stop on exceptions" work now and worry about dupe logs after. Please submit a bug if you come across any.
This patch will lead to duplicated messages in console. If there is an exception in the handler it will be first reported to the console because the TryCatch is verbose and then it will be reported in reportException. (In reply to comment #5) > Created an attachment (id=42501) [details] > Proposed Patch
Yes, I started to work on the change but we need to prepare for GDD now. I'll send it later. (In reply to comment #2) > I'm not too familiar with how the devtools hook in here. Can you upload a > patch that does the above so we can see what it looks like in code?
Created attachment 42526 [details] Proposed Patch Fixed the problem that duplicate error messages could be shown in the console.
This patch seems to fix the issue with console duplicates. But please add a FIXME comment and file a bug for this. We should not discriminate between different ScriptExecutionContext types when reporting exceptions. (In reply to comment #9) > Created an attachment (id=42526) [details] > Proposed Patch > > Fixed the problem that duplicate error messages could be shown in the console.
Comment on attachment 42526 [details] Proposed Patch r+ given FIXME is added and bug is filed.
Bug filed: https://bugs.webkit.org/show_bug.cgi?id=31171 FIXME added and committed as http://trac.webkit.org/changeset/50568.