Bug 124065 - Web Inspector: It should be possible to debug the Inspector code
Summary: Web Inspector: It should be possible to debug the Inspector code
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Alexandru Chiculita
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2013-11-08 13:15 PST by Alexandru Chiculita
Modified: 2013-11-11 08:11 PST (History)
8 users (show)

See Also:


Attachments
Patch V1 (10.79 KB, patch)
2013-11-08 13:27 PST, Alexandru Chiculita
no flags Details | Formatted Diff | Diff
Patch V2 (10.78 KB, patch)
2013-11-08 13:30 PST, Alexandru Chiculita
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2 (502.60 KB, application/zip)
2013-11-08 13:58 PST, Build Bot
no flags Details
Patch V3 (11.02 KB, patch)
2013-11-08 15:48 PST, Alexandru Chiculita
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexandru Chiculita 2013-11-08 13:15:01 PST
Make it possible to debug the inspector code using another instance of the inspector.
Comment 1 Radar WebKit Bug Importer 2013-11-08 13:15:26 PST
<rdar://problem/15427415>
Comment 2 Alexandru Chiculita 2013-11-08 13:27:14 PST
Created attachment 216430 [details]
Patch V1
Comment 3 WebKit Commit Bot 2013-11-08 13:29:07 PST
Attachment 216430 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/inspector-protocol/debugger/nested-inspectors-expected.txt', u'LayoutTests/inspector-protocol/debugger/nested-inspectors.html', u'Source/WebKit2/ChangeLog', u'Source/WebKit2/UIProcess/WebInspectorProxy.cpp', u'Source/WebKit2/UIProcess/WebInspectorProxy.h']" exit_code: 1
Source/WebKit2/UIProcess/WebInspectorProxy.cpp:71:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebKit2/UIProcess/WebInspectorProxy.cpp:87:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 2 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Alexandru Chiculita 2013-11-08 13:30:57 PST
Created attachment 216433 [details]
Patch V2
Comment 5 Build Bot 2013-11-08 13:58:40 PST
Comment on attachment 216433 [details]
Patch V2

Attachment 216433 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/22728545

New failing tests:
inspector-protocol/debugger/setBreakpoint.html
Comment 6 Build Bot 2013-11-08 13:58:41 PST
Created attachment 216435 [details]
Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-09  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 7 Alexandru Chiculita 2013-11-08 14:09:26 PST
(In reply to comment #5)
> (From update of attachment 216433 [details])
> Attachment 216433 [details] did not pass mac-wk2-ews (mac-wk2):
> Output: http://webkit-queues.appspot.com/results/22728545
> 
> New failing tests:
> inspector-protocol/debugger/setBreakpoint.html

This should be unrelated to my C++ change as the test runner will not touch my code. However, the new test has an impact on the others, so I will have to debug that.
Comment 8 Alexandru Chiculita 2013-11-08 15:33:31 PST
(In reply to comment #7)
> (In reply to comment #5)
> > (From update of attachment 216433 [details] [details])
> > Attachment 216433 [details] [details] did not pass mac-wk2-ews (mac-wk2):
> > Output: http://webkit-queues.appspot.com/results/22728545
> > 
> > New failing tests:
> > inspector-protocol/debugger/setBreakpoint.html
> 
> This should be unrelated to my C++ change as the test runner will not touch my code. However, the new test has an impact on the others, so I will have to debug that.

Seems like I've uncovered a bug.

The test in this patch is changing the m_pauseOnExceptionsState on the ScriptDebugServer. The state is not reset before the following tests are run.

1. The failing test sets a breakpoint with a condition.
2. The JS in the conditon throws an exception.
3. m_pauseOnExceptionsState that was not reset, so the ScriptDebugServer will notify the inspector about the exception. This will run an event loop and wait.
4. This is unexpected in the test, so it will try to fail early by detaching the inspector window.
5. ScriptDebugServer.m_currentCallFrame is set to 0 and the event loop unrolls.
6. ScriptDebugServer.hasBreakpoint tries to report the exception to the console using ScriptDebugServer.m_currentCallFrame which is 0. <Crash>

I will revert the m_pauseOnExceptionsState in the new test and also add/fix the other bug. I guess that one could reproduce in the inspector by adding a condition that throws an exception. Wait until the debugger stops on the condition. Close the inspector.
Comment 9 Alexandru Chiculita 2013-11-08 15:48:47 PST
Created attachment 216454 [details]
Patch V3
Comment 10 WebKit Commit Bot 2013-11-08 16:37:30 PST
Comment on attachment 216454 [details]
Patch V3

Clearing flags on attachment: 216454

Committed r158976: <http://trac.webkit.org/changeset/158976>
Comment 11 WebKit Commit Bot 2013-11-08 16:37:32 PST
All reviewed patches have been landed.  Closing bug.
Comment 12 Gustavo Noronha (kov) 2013-11-11 08:11:25 PST
This broke attaching the inspector, see https://bugs.webkit.org/show_bug.cgi?id=124148