Bug 41094

Summary:

Web Inspector: Inspector cleanup + better DevTools alignment with Inspector.

Product:

WebKit

Reporter:

Pavel Feldman <pfeldman>

Component:

Web Inspector (Deprecated)

Assignee:

Pavel Feldman <pfeldman>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

abarth, eric, joepeck, timothy, webkit.review.bot, yurys

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
[PATCH] Proposed change. I still need to test it, but 99.9% of changes are ready for review.	none
[PATCH] Same with InspectorClientImpl files actually removed.	yurys: review-
[PATCH] Review comments addressed.	yurys: review+

Pavel Feldman

Reported 2010-06-23 12:48:57 PDT

WebCore: - Removed a bunch of unused methods from all over the place - Added client callbacks for states surviving navigation - Implemented more user-friendly stub for InspectorFrontendHost.platform WebKit/chromium - Merged InspectorClient implementation into WebDevToolsAgent - Changed the way WebDevToolsAgent is initialized (will break chromium bot, needs coordinated landing) - Removed a lot of legacy code from WebDevToolsAgentImpl - Removed a lot of overrides from DevTools.js - Simplified devtools stubs - Got rid of all logic from within InjectDispatch.js We are now much cleaner and much more ready for remote debugging!

Attachments
[PATCH] Proposed change. I still need to test it, but 99.9% of changes are ready for review. (47.73 KB, patch) 2010-06-23 12:52 PDT, Pavel Feldman	no flags	Details Formatted Diff Diff
[PATCH] Same with InspectorClientImpl files actually removed. (54.83 KB, patch) 2010-06-23 12:55 PDT, Pavel Feldman	yurys: review-	Details Formatted Diff Diff
[PATCH] Review comments addressed. (54.78 KB, patch) 2010-06-24 04:51 PDT, Pavel Feldman	yurys: review+	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Pavel Feldman

Comment 1 2010-06-23 12:52:01 PDT

Created attachment 59553 [details] [PATCH] Proposed change. I still need to test it, but 99.9% of changes are ready for review.

Pavel Feldman

Comment 2 2010-06-23 12:55:30 PDT

Created attachment 59554 [details] [PATCH] Same with InspectorClientImpl files actually removed.

Yury Semikhatsky

Comment 3 2010-06-24 01:43:57 PDT

Comment on attachment 59554 [details] [PATCH] Same with InspectorClientImpl files actually removed. WebKit/chromium/src/WebDevToolsAgentImpl.cpp:486 + if (!agent->m_apuAgentEnabled) At first check if there is ApuAgent and dispatch the message just to it. WebKit/chromium/src/WebViewImpl.cpp: + void WebViewImpl::setDevToolsAgent(WebDevToolsAgent* devToolsAgent) It is likely to break layout tests. Make sure they are fine. WebKit/chromium/src/js/InjectDispatch.js: + function close() { Pleas make sure tests pass.

Pavel Feldman

Comment 4 2010-06-24 04:51:25 PDT

Created attachment 59641 [details] [PATCH] Review comments addressed.

Yury Semikhatsky

Comment 5 2010-06-24 04:58:33 PDT

Comment on attachment 59641 [details] [PATCH] Review comments addressed. WebKit/chromium/src/WebDevToolsAgentImpl.cpp:491 + if (method.isEmpty() || exceptionCatcher.HasCaught()) do you need to check for jsDispatchOnClient.HasCaught here and above?

Yury Semikhatsky

Comment 6 2010-06-24 05:04:20 PDT

Comment on attachment 59641 [details] [PATCH] Review comments addressed. r+ given interactive and layout tests pass.

WebKit Review Bot

Comment 7 2010-06-24 09:10:33 PDT

http://trac.webkit.org/changeset/61766 might have broken Chromium Linux Release

Pavel Feldman

Comment 8 2010-06-24 11:07:50 PDT

Landed as r61772

Note You need to log in before you can comment on or make changes to this bug.