Bug 46724 - Web Inspector: move pauseOnExceptionState under control of InspectorState
Summary: Web Inspector: move pauseOnExceptionState under control of InspectorState
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Ilya Tikhonovsky
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-09-28 08:08 PDT by Ilya Tikhonovsky
Modified: 2010-09-29 01:00 PDT (History)
9 users (show)

See Also:


Attachments
[patch] initial version. (7.49 KB, patch)
2010-09-28 08:15 PDT, Ilya Tikhonovsky
yurys: review-
yurys: commit-queue-
Details | Formatted Diff | Diff
[patch] second iteration (7.74 KB, patch)
2010-09-28 13:20 PDT, Ilya Tikhonovsky
yurys: review+
yurys: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ilya Tikhonovsky 2010-09-28 08:08:36 PDT
%subj%
Comment 1 Ilya Tikhonovsky 2010-09-28 08:15:16 PDT
Created attachment 69053 [details]
[patch] initial version.
Comment 2 Yury Semikhatsky 2010-09-28 10:51:25 PDT
Comment on attachment 69053 [details]
[patch] initial version.

View in context: https://bugs.webkit.org/attachment.cgi?id=69053&action=review

> WebCore/inspector/InspectorController.cpp:263
> +    (*state)->setNumber(pauseOnExceptionsStateStateName, ScriptDebugServer::shared().pauseOnExceptionsState());

This is the only usage of ScriptDebugServer left in the IC, I should have removed when I was extracting InspectorDebuggerAgent. Could you move this accessor into Inspector  debugger agent and check here if the agent is present, it should cover both cases of debugger always enabled and debugger enabled manually.

> WebCore/inspector/front-end/inspector.js:595
> +        if ("updatePauseOnExceptionsState" in inspectorState)

you check for presence of one name but then access another field
Comment 3 Ilya Tikhonovsky 2010-09-28 13:20:37 PDT
Created attachment 69093 [details]
[patch] second iteration

comments addressed
Comment 4 Yury Semikhatsky 2010-09-28 23:47:35 PDT
Comment on attachment 69093 [details]
[patch] second iteration

View in context: https://bugs.webkit.org/attachment.cgi?id=69093&action=review

> WebCore/inspector/InspectorController.cpp:264
> +        (*state)->setNumber(pauseOnExceptionsStateStateName, m_debuggerAgent->pauseOnExceptionsState());

Now we can remove #include "ScriptDebugServer.h". Please do this before landing.
Comment 5 Ilya Tikhonovsky 2010-09-29 01:00:43 PDT
Committed r68635
	M	WebCore/ChangeLog
	M	WebCore/inspector/InspectorDebuggerAgent.h
	M	WebCore/inspector/InspectorController.cpp
	M	WebCore/inspector/InspectorDebuggerAgent.cpp
	M	WebCore/inspector/Inspector.idl
	M	WebCore/inspector/front-end/ScriptsPanel.js
	M	WebCore/inspector/front-end/inspector.js
r68635 = b05398cb12412b6f9b65099ae5f5526aab37c5a0 (refs/remotes/trunk)