Bug 31888 - Web Inspector: Split InspectorBackend into three parts: backend, injected script host and frontend host.
Summary: Web Inspector: Split InspectorBackend into three parts: backend, injected scr...
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
Depends on:
Reported: 2009-11-25 13:44 PST by Pavel Feldman
Modified: 2009-12-01 00:07 PST (History)
6 users (show)

See Also:

[DIAGRAM] Proposed design. (81.43 KB, image/png)
2009-11-25 13:44 PST, Pavel Feldman
no flags Details
[PATCH] The patch (139.82 KB, patch)
2009-11-26 11:16 PST, Pavel Feldman
no flags Details | Formatted Diff | Diff
[PATCH] Now splits IDL into 3 parts as in design proposal. (213.26 KB, patch)
2009-11-27 07:09 PST, Pavel Feldman
timothy: 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 2009-11-25 13:44:59 PST
Created attachment 43869 [details]
[DIAGRAM] Proposed design.

This is going to be a large refactoring that would split InspectorBackend into three IDLs: one for the frontend, one for the injected script and one for the frontend host. See diagram attached:

Not changed:
- InspectorController exposes public API and wires others together
- InspectorFrontend determines the interface to the frontend. It is semi-serializes all the calls into JSON-stringifiable objects
- The calls made via InspectorFrontend are dispatched by the WebInspector and delegated to appropriate frontend objects
- InspectorController seeds InspectorBackend.idl-based wrapper into the fron-end

- InspectorController global object gets renamed to InspectorBackend in the front-end
- InspectorController seeds another InjectedScriptHost.idl-based wrapper into the InjectedScript context. This idl has methods that are used from within injected script only. Most of them are IDL [Custom] since they operate Nodes, Databases, etc. (this is new)
- InspectorController seeds yet another InspectorFrontendHost.idl-based wrapper into  the front-end that has methods not related to inspected page, but that are going straight to the inspector client (attach, detach, setheight, addSourceToFrame, searchSourceView, etc).
- All calls to InspectorBackend in front-end are going through the semi-serialization into JSON-stringifiable objects.
- Front-end's InspectorBackend object has methods that will get either to InspectorBackend wrapper or into InjectedScript on the inspected page. So that there is single interface to talk to backend. Clients decide whether they implement backend code as C++ in InspectorBackend or in JavaScript in InjectedScript depending on what fits better.

- This is a nice first step towards running InjectedScript in its context.
- InspectorBackend loses all the [Custom] methods, they get into InjectedScriptHost
- Interaction between back-end and front-end is clear and serializable. There is no problem in putting socket in between.
- Extracting InspectorFrontendHost allows running self-contained front-end that is capable of showing sources in frames, etc. on another machine
- And many nice internal cleanups such as highlightDOMNode that does not need to be [Custom] for a while already.
Comment 1 Timothy Hatcher 2009-11-25 18:04:28 PST
This looks good to me. Should WebInspector.InspectorBackend just be WebInspector.Backend?
Comment 2 Pavel Feldman 2009-11-26 11:16:06 PST
Created attachment 43927 [details]
[PATCH] The patch
Comment 3 Pavel Feldman 2009-11-27 07:09:19 PST
Created attachment 43952 [details]
[PATCH] Now splits IDL into 3 parts as in design proposal.
Comment 4 Pavel Feldman 2009-11-27 08:38:40 PST
Since the patch is fairly large, I uploaded it to http://codereview.appspot.com/162045.

Note that it won't show side-by-side diff for large files, so when you iterate over the files you sometimes need to get back to side-by-side explicitly via choosing it in top right corner. You can leave comments in place given you have google account.
Comment 5 Pavel Feldman 2009-12-01 00:07:43 PST
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	C	WebCore/inspector/front-end/TestController.js => WebCore/inspector/InjectedScriptHost.idl
	C	WebCore/inspector/front-end/Database.js => WebCore/inspector/InspectorFrontendHost.h
	C	WebCore/inspector/front-end/TestController.js => WebCore/inspector/InspectorFrontendHost.idl
	C	WebCore/inspector/front-end/InspectorControllerStub.js => WebCore/inspector/front-end/InspectorBackendStub.js
	C	WebCore/inspector/front-end/TestController.js => WebCore/inspector/front-end/InspectorFrontendHostStub.js
	D	WebCore/bindings/js/JSInspectorBackendCustom.cpp
	D	WebCore/bindings/v8/custom/V8InspectorBackendCustom.cpp
	M	WebCore/ChangeLog
	M	WebCore/DerivedSources.make
	M	WebCore/GNUmakefile.am
	M	WebCore/WebCore.gypi
	M	WebCore/WebCore.pro
	M	WebCore/WebCore.vcproj/WebCore.vcproj
	M	WebCore/WebCore.xcodeproj/project.pbxproj
	M	WebCore/WebCoreSources.bkl
	M	WebCore/bindings/js/JSBindingsAllInOne.cpp
	A	WebCore/bindings/js/JSInjectedScriptHostCustom.cpp
	A	WebCore/bindings/js/JSInspectorFrontendHostCustom.cpp
	M	WebCore/bindings/js/ScriptObject.cpp
	M	WebCore/bindings/js/ScriptObject.h
	M	WebCore/bindings/v8/DOMObjectsInclude.h
	M	WebCore/bindings/v8/DerivedSourcesAllInOne.cpp
	M	WebCore/bindings/v8/ScriptObject.cpp
	M	WebCore/bindings/v8/ScriptObject.h
	M	WebCore/bindings/v8/V8Index.cpp
	M	WebCore/bindings/v8/V8Index.h
	M	WebCore/bindings/v8/custom/V8CustomBinding.h
	A	WebCore/bindings/v8/custom/V8InjectedScriptHostCustom.cpp
	A	WebCore/bindings/v8/custom/V8InspectorFrontendHostCustom.cpp
	A	WebCore/inspector/InjectedScriptHost.cpp
	A	WebCore/inspector/InjectedScriptHost.h
	M	WebCore/inspector/InspectorBackend.cpp
	M	WebCore/inspector/InspectorBackend.h
	M	WebCore/inspector/InspectorBackend.idl
	M	WebCore/inspector/InspectorController.cpp
	M	WebCore/inspector/InspectorController.h
	A	WebCore/inspector/InspectorFrontendHost.cpp
	M	WebCore/inspector/front-end/Breakpoint.js
	M	WebCore/inspector/front-end/BreakpointsSidebarPane.js
	M	WebCore/inspector/front-end/ConsoleView.js
	M	WebCore/inspector/front-end/CookieItemsView.js
	M	WebCore/inspector/front-end/DOMAgent.js
	M	WebCore/inspector/front-end/DOMStorage.js
	M	WebCore/inspector/front-end/Database.js
	M	WebCore/inspector/front-end/ElementsPanel.js
	M	WebCore/inspector/front-end/ElementsTreeOutline.js
	M	WebCore/inspector/front-end/EventListenersSidebarPane.js
	M	WebCore/inspector/front-end/InjectedScript.js
	M	WebCore/inspector/front-end/InjectedScriptAccess.js
	M	WebCore/inspector/front-end/InspectorControllerStub.js
	M	WebCore/inspector/front-end/ProfileView.js
	M	WebCore/inspector/front-end/ProfilesPanel.js
	M	WebCore/inspector/front-end/ResourcesPanel.js
	M	WebCore/inspector/front-end/ScriptView.js
	M	WebCore/inspector/front-end/ScriptsPanel.js
	M	WebCore/inspector/front-end/SourceView.js
	M	WebCore/inspector/front-end/StylesSidebarPane.js
	M	WebCore/inspector/front-end/TestController.js
	M	WebCore/inspector/front-end/TimelinePanel.js
	M	WebCore/inspector/front-end/WatchExpressionsSidebarPane.js
	M	WebCore/inspector/front-end/WebKit.qrc
	M	WebCore/inspector/front-end/inspector.html
	M	WebCore/inspector/front-end/inspector.js
Committed r51528