Bug 41094 - Web Inspector: Inspector cleanup + better DevTools alignment with Inspector.
Summary: Web Inspector: Inspector cleanup + better DevTools alignment with Inspector.
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: Pavel Feldman
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-06-23 12:48 PDT by Pavel Feldman
Modified: 2010-06-24 11:07 PDT (History)
6 users (show)

See Also:


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

Note You need to log in before you can comment on or make changes to this bug.
Description Pavel Feldman 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!
Comment 1 Pavel Feldman 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.
Comment 2 Pavel Feldman 2010-06-23 12:55:30 PDT
Created attachment 59554 [details]
[PATCH] Same with InspectorClientImpl files actually removed.
Comment 3 Yury Semikhatsky 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.
Comment 4 Pavel Feldman 2010-06-24 04:51:25 PDT
Created attachment 59641 [details]
[PATCH] Review comments addressed.
Comment 5 Yury Semikhatsky 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?
Comment 6 Yury Semikhatsky 2010-06-24 05:04:20 PDT
Comment on attachment 59641 [details]
[PATCH] Review comments addressed.

r+ given interactive and layout tests pass.
Comment 7 WebKit Review Bot 2010-06-24 09:10:33 PDT
http://trac.webkit.org/changeset/61766 might have broken Chromium Linux Release
Comment 8 Pavel Feldman 2010-06-24 11:07:50 PDT
Landed as r61772