Bug 35036 - Introduce InspectorFrontendClient that would provide InspectorFrontend with an interface to the embedder
Summary: Introduce InspectorFrontendClient that would provide InspectorFrontend with a...
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: Yury Semikhatsky
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-02-17 06:52 PST by Yury Semikhatsky
Modified: 2010-03-16 04:00 PDT (History)
5 users (show)

See Also:


Attachments
patch for Mac platform (79.45 KB, patch)
2010-03-01 08:24 PST, Yury Semikhatsky
no flags Details | Formatted Diff | Diff
patch (157.47 KB, patch)
2010-03-10 12:38 PST, Yury Semikhatsky
no flags Details | Formatted Diff | Diff
patch (157.47 KB, patch)
2010-03-11 05:58 PST, Yury Semikhatsky
no flags Details | Formatted Diff | Diff
patch (159.19 KB, patch)
2010-03-11 06:20 PST, Yury Semikhatsky
no flags Details | Formatted Diff | Diff
patch (159.19 KB, patch)
2010-03-11 06:27 PST, Yury Semikhatsky
no flags Details | Formatted Diff | Diff
patch (159.81 KB, patch)
2010-03-11 07:23 PST, Yury Semikhatsky
no flags Details | Formatted Diff | Diff
patch (165.25 KB, patch)
2010-03-12 06:52 PST, Yury Semikhatsky
no flags Details | Formatted Diff | Diff
patch (169.53 KB, patch)
2010-03-16 02:42 PDT, Yury Semikhatsky
no flags Details | Formatted Diff | Diff
patch (163.60 KB, patch)
2010-03-16 02:47 PDT, Yury Semikhatsky
pfeldman: review+
Details | Formatted Diff | Diff
patch that I'm going to land (167.38 KB, patch)
2010-03-16 03:38 PDT, Yury Semikhatsky
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yury Semikhatsky 2010-02-17 06:52:23 PST
Currently there is a bunch of methods in InspectorController that are called from InspctorFrontend to modify its window(such as moveWindowBy, showWindow, attachWindow etc.). Most of them are delegated to the InspectorClient. This design assumes that the frontend lives in the same process as the inspectorcontroller of the inspected page. To support out-of-process frontend we need to break this dependency.

Currently InspectorFrontendHost depends on InspectorClient, InspectorController and InspectorFrontend. The proposal is to introduce InspectorFrontendClient which would provide the frontend with an interface to the underlying platform. Some of its functionality is currently a part of InspectorClient and should be removed from there.

InspectorController-->o--IncpetctorClient
  |
  +-->o--InspectorFrontend-->o--(inspector UI(*.js))-->o--InspectorFrontendHost-->o---InspectorFrontendClient
                        |
  InspectorBackend--o<--+

All the messages from InspectorController to the frontend should go through InspectorFrontend which may be either inprocess implementation or remote frontend proxy. Frontend host should communicate with InspectorFrontendClient only and eventually all the communication with the InspectorBackend should be serialized and go through the client.
Comment 1 Pavel Feldman 2010-02-17 07:03:59 PST
Design image from one of the previous commits:
https://bugs.webkit.org/attachment.cgi?id=43869
Comment 2 Yury Semikhatsky 2010-03-01 08:24:00 PST
Created attachment 49726 [details]
patch for Mac platform

Please take a look at the patch. It works only on Mac but it should reflect general approach. I'm working on support for other platforms.
Comment 3 Pavel Feldman 2010-03-01 11:56:16 PST
Comment on attachment 49726 [details]
patch for Mac platform

Putting r? for xenon to get this wip patch in his review queue.
Comment 4 WebKit Review Bot 2010-03-01 11:57:46 PST
Attachment 49726 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebCore/inspector/InspectorFrontendClientLocal.cpp:39:  Alphabetical sorting problem.  [build/include_order] [4]
WebCore/inspector/InspectorFrontendClientLocal.cpp:50:  Alphabetical sorting problem.  [build/include_order] [4]
WebCore/inspector/ConsoleMessage.h:41:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 3 in 25 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 WebKit Review Bot 2010-03-01 12:08:02 PST
Attachment 49726 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/320196
Comment 6 Yury Semikhatsky 2010-03-10 12:38:36 PST
Created attachment 50424 [details]
patch
Comment 7 Pavel Feldman 2010-03-10 23:52:17 PST
r=me.

Could you try doing rebase? (bots could not apply the patch)
Comment 8 Yury Semikhatsky 2010-03-11 05:58:37 PST
Created attachment 50492 [details]
patch

Rebased previous patch.
Comment 9 WebKit Review Bot 2010-03-11 06:03:25 PST
Attachment 50492 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebCore/inspector/InspectorFrontendClientLocal.cpp:39:  Alphabetical sorting problem.  [build/include_order] [4]
WebCore/inspector/InspectorFrontendClientLocal.cpp:50:  Alphabetical sorting problem.  [build/include_order] [4]
WARNING: File exempt from style guide. Skipping: "WebKit/qt/Api/qwebinspector.cpp"
WARNING: File exempt from style guide. Skipping: "WebKit/qt/Api/qwebpage.h"
WARNING: File exempt from style guide. Skipping: "WebKit/qt/Api/qwebinspector.h"
WebCore/inspector/ConsoleMessage.h:41:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 3 in 59 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 10 Yury Semikhatsky 2010-03-11 06:20:16 PST
Created attachment 50493 [details]
patch

Fixed style errors.
Comment 11 WebKit Review Bot 2010-03-11 06:24:48 PST
Attachment 50493 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WARNING: File exempt from style guide. Skipping: "WebKit/qt/Api/qwebinspector.cpp"
WARNING: File exempt from style guide. Skipping: "WebKit/qt/Api/qwebpage.h"
WARNING: File exempt from style guide. Skipping: "WebKit/qt/Api/qwebinspector.h"
WebCore/inspector/ConsoleMessage.h:55:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 1 in 59 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 12 Yury Semikhatsky 2010-03-11 06:27:30 PST
Created attachment 50494 [details]
patch

One more style fix.
Comment 13 Yury Semikhatsky 2010-03-11 07:23:32 PST
Created attachment 50498 [details]
patch

Added new files to WebCore.vcproj
Comment 14 Yury Semikhatsky 2010-03-12 06:52:41 PST
Created attachment 50594 [details]
patch

Fixed layout issue when inspector opens attached.
Comment 15 Yury Semikhatsky 2010-03-16 02:42:36 PDT
Created attachment 50777 [details]
patch

Inroduced InspectorController::disconnectFrontend method and inlined couple of others.
Comment 16 Yury Semikhatsky 2010-03-16 02:47:43 PDT
Created attachment 50778 [details]
patch

WebCore/ChangeLog merge conflict resolved.
Comment 17 Yury Semikhatsky 2010-03-16 03:38:44 PDT
Created attachment 50779 [details]
patch that I'm going to land

Added support for Efl platform.
Comment 18 Yury Semikhatsky 2010-03-16 04:00:49 PDT
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	C	WebCore/inspector/InspectorFrontendHost.idl => WebCore/inspector/InspectorFrontendClient.h
	C	WebCore/inspector/front-end/InspectorFrontendHostStub.js => WebCore/inspector/InspectorFrontendClientLocal.h
	M	WebCore/ChangeLog
	M	WebCore/GNUmakefile.am
	M	WebCore/WebCore.Inspector.exp
	M	WebCore/WebCore.base.exp
	M	WebCore/WebCore.gypi
	M	WebCore/WebCore.order
	M	WebCore/WebCore.pro
	M	WebCore/WebCore.vcproj/WebCore.vcproj
	M	WebCore/WebCore.xcodeproj/project.pbxproj
	M	WebCore/inspector/ConsoleMessage.cpp
	M	WebCore/inspector/ConsoleMessage.h
	M	WebCore/inspector/InspectorClient.h
	M	WebCore/inspector/InspectorController.cpp
	M	WebCore/inspector/InspectorController.h
	M	WebCore/inspector/InspectorFrontend.cpp
	M	WebCore/inspector/InspectorFrontend.h
	A	WebCore/inspector/InspectorFrontendClientLocal.cpp
	M	WebCore/inspector/InspectorFrontendHost.cpp
	M	WebCore/inspector/InspectorFrontendHost.h
	M	WebCore/inspector/InspectorFrontendHost.idl
	M	WebCore/inspector/InspectorResource.cpp
	M	WebCore/inspector/InspectorResource.h
	M	WebCore/inspector/front-end/InspectorFrontendHostStub.js
	M	WebCore/inspector/front-end/inspector.js
	M	WebCore/loader/EmptyClients.h
	M	WebCore/loader/FrameLoader.cpp
	M	WebCore/page/Page.cpp
	M	WebCore/page/Page.h
	M	WebCore/platform/ContextMenu.cpp
	M	WebKit/ChangeLog
	M	WebKit/chromium/ChangeLog
	M	WebKit/chromium/src/InspectorClientImpl.cpp
	M	WebKit/chromium/src/InspectorClientImpl.h
	M	WebKit/chromium/src/WebDevToolsAgentImpl.cpp
	M	WebKit/chromium/src/WebDevToolsAgentImpl.h
	M	WebKit/chromium/src/WebDevToolsFrontendImpl.cpp
	M	WebKit/chromium/src/WebDevToolsFrontendImpl.h
	M	WebKit/chromium/src/js/InjectDispatch.js
	M	WebKit/efl/WebCoreSupport/InspectorClientEfl.cpp
	M	WebKit/efl/WebCoreSupport/InspectorClientEfl.h
	M	WebKit/gtk/ChangeLog
	M	WebKit/gtk/WebCoreSupport/InspectorClientGtk.cpp
	M	WebKit/gtk/WebCoreSupport/InspectorClientGtk.h
	M	WebKit/haiku/ChangeLog
	M	WebKit/haiku/WebCoreSupport/InspectorClientHaiku.cpp
	M	WebKit/haiku/WebCoreSupport/InspectorClientHaiku.h
	M	WebKit/mac/ChangeLog
	M	WebKit/mac/WebCoreSupport/WebInspectorClient.h
	M	WebKit/mac/WebCoreSupport/WebInspectorClient.mm
	M	WebKit/mac/WebInspector/WebInspector.mm
	M	WebKit/qt/Api/qwebinspector.cpp
	M	WebKit/qt/Api/qwebinspector.h
	M	WebKit/qt/Api/qwebpage.h
	M	WebKit/qt/ChangeLog
	M	WebKit/qt/WebCoreSupport/InspectorClientQt.cpp
	M	WebKit/qt/WebCoreSupport/InspectorClientQt.h
	M	WebKit/win/ChangeLog
	M	WebKit/win/WebCoreSupport/WebInspectorClient.cpp
	M	WebKit/win/WebCoreSupport/WebInspectorClient.h
	M	WebKit/win/WebInspector.cpp
	M	WebKit/wx/ChangeLog
	M	WebKit/wx/WebKitSupport/InspectorClientWx.cpp
	M	WebKit/wx/WebKitSupport/InspectorClientWx.h
Committed r56051