Bug 35340 – [Qt] Enable alternate HTML/JavaScript front-ends for Web Inspector

• Component: WebKit Qt
• Status: CLOSED FIXED

[Jamey Hicks]

This patch enables the use of alternate UIs for Web Inspector by loading different HTML/JavaScript when QWebInspector is instantiated and shown.

We're planning to use this to allow remote use of Web Inspector via HTTP and to use Eclipse or Aptana to debug and profile WebKit.

An earlier patch was submitted but that approach was given review- and the approach used in this patch (i.e., using alternate HTML/JavaScript) was recommended.

[Jamey Hicks]

Created attachment 49385 [details]
Adds QWebInspector::setFrontendURL() and QWebInspector::setDefaultFrontendUrl()

[Jamey Hicks]

Created attachment 49395 [details]
Adds QWebInspector::setFrontendURL() and QWebInspector::setDefaultFrontendUrl()

Revised ChangeLog

[WebKit Review Bot]

Attachment 49395 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
Ignoring "WebKit/qt/Api/qwebinspector.cpp": this file is exempt from the style guide.
Ignoring "WebKit/qt/Api/qwebinspector.h": this file is exempt from the style guide.
WebKit/qt/ChangeLog:5:  Line contains tab character.  [whitespace/tab] [5]
WebKit/qt/ChangeLog:6:  Line contains tab character.  [whitespace/tab] [5]
WebKit/qt/ChangeLog:8:  Line contains tab character.  [whitespace/tab] [5]
WebKit/qt/ChangeLog:9:  Line contains tab character.  [whitespace/tab] [5]
WebKit/qt/ChangeLog:11:  Line contains tab character.  [whitespace/tab] [5]
WebKit/qt/ChangeLog:12:  Line contains tab character.  [whitespace/tab] [5]
WebKit/qt/ChangeLog:13:  Line contains tab character.  [whitespace/tab] [5]
WebKit/qt/ChangeLog:14:  Line contains tab character.  [whitespace/tab] [5]
WebKit/qt/ChangeLog:15:  Line contains tab character.  [whitespace/tab] [5]
Ignoring "WebKit/qt/Api/qwebpage_p.h": this file is exempt from the style guide.
Ignoring "WebKit/qt/Api/qwebpage.h": this file is exempt from the style guide.
Ignoring "WebKit/qt/Api/qwebpage.cpp": this file is exempt from the style guide.
Ignoring "WebKit/qt/Api/qwebinspector_p.h": this file is exempt from the style guide.
Total errors found: 9 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Jamey Hicks]

Created attachment 49418 [details]
revised patch to fix style problems in WebKit/qt/ChangeLog

[Laszlo Gombos]

Comment on attachment 49418 [details]
revised patch to fix style problems in WebKit/qt/ChangeLog

Approach looks good to me, this is a great addition to QtWebKit.

ChangeLog does not seems to follow the template. The "Reviewed by NOBODY (OOPS!)." part is missing. Also I have added [Qt] to the title of the bug, so if you could regenerate the ChnageLog that would be good.

Based on IRC discussion with Kenneth, Jamey and myself we concluded the the API for setting/getting the Inspector URL should be:

void QWebSettings::setInspectorUrl (const QUrl &inspector);
QUrl QWebSettings::inspectorUrl() const;

Please document these new APIs well.

This means that changes in InspectorClientQt.h would not be necessary but your style fixes are very welcome.

r- to use the new proposed API; general approach is good.

[Jamey Hicks]

Created attachment 49528 [details]
revised to use QWebSettings::setInspectorUrl() and QWebSettings::inspectorUrl() instead of a change to the QWebInspector API.

Revised to use QWebSettings::setInspectorUrl() and QWebSettings::inspectorUrl() instead of a change to the QWebInspector API.

Also added -inspector-url argument to QtLauncher so this can be tested.

[Laszlo Gombos]

Comment on attachment 49528 [details]
revised to use QWebSettings::setInspectorUrl() and QWebSettings::inspectorUrl() instead of a change to the QWebInspector API.

LGTM, Thanks Jamey.

[WebKit Commit Bot]

Comment on attachment 49528 [details]
revised to use QWebSettings::setInspectorUrl() and QWebSettings::inspectorUrl() instead of a change to the QWebInspector API.

Clearing flags on attachment: 49528

Committed r55273: <http://trac.webkit.org/changeset/55273>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Jocelyn Turcotte]

Is there any plan to make the server side of the remote inspector available in WebKit trunk somehow?

It would be nice to have a complete solution available to users without having to rely on external components possibly not compatible with the version of WebKit they use.

For example a separate executable compiled with the rest or eventually some kind of QRemoteWebInspector widget class that takes in some network parameters and start the HTTP server.

[Jamey Hicks]

I agree that it would be helpful to have the remote inspector available in WebKit trunk.

There are two pieces of this that are platform specific: The first is the piece I just patched for Qt -- the platform specific code that launches a particular html/javascript page for the inspector frontend. 

Someone would need to do this for other platforms.

The second is the server part. Using WebSockets to connect the UI and the inspector might remove the need for an HTTP server in WebKit, which would reduce the platform dependences.

In the short term, I'll be working on the HTTP server in Qt/WebKit and I do plan on making that public.

[Simon Hausmann]

I don't think this is the right API and I would like to roll out this patch.

Properties in QWebSettings should be useful for anyone using QWebPage, QWebView and QGraphicsWebView, QtWebKit in general.

This property is highly specific to the web inspector and therefore belongs into QWebInspector I think. The documentation should make it clear what it is useful for.

Jamey, can you explain exactly what you're doing and when you set this property? What features do you need of the inspector itself when you run it on the desktop?

[Laszlo Gombos]

Simon, the patch before (https://bug-35340-attachments.webkit.org/attachment.cgi?id=49418) use to have the interface on QWebInspector. Are you proposing a similar interface ?

This API tells Webkit the URL for the front-end resource files.

I would suggest to discuss (and fix if needed) the API without rolling the patch out.

[Jamey Hicks]

Simon,

We are enabling remote usage of all the inspector functionality. I'm working on two such remote cases:
1) Using Web Inspector UI remoted over HTTP to enable on-device debug and profiling.
2) Implementing ChromeDevTools protocol for Eclipse or Aptana debug and profiling on-device

The previous version of the patch extended the QWebInspector API but a discussion with laszlo and kenne on #qtwebkit led to this QWebSettings API. I have no particular preference between the two.

Jamey

[Jocelyn Turcotte]

(In reply to comment #14)
Just to dump how I see both alternatives:
- The API in QWebSettings is nice since it make it simple to apply it on all the pages. It also don't bind to the QWebInspector class which is currently only a widget class. Though its name suggests it could do more.
- The API in QWebInspector is nice since it don't clutter the normal web page usage API.

Both of them goes in the opposite was that we see usual remote debugging where the debuggee would open the server and the client would attach to the debuggee. If we want to change the way the implementation is done in the future this might cause us headaches.

[Simon Hausmann]

(In reply to comment #14)
> Simon,
> 
> We are enabling remote usage of all the inspector functionality. I'm working on
> two such remote cases:
> 1) Using Web Inspector UI remoted over HTTP to enable on-device debug and
> profiling.
> 2) Implementing ChromeDevTools protocol for Eclipse or Aptana debug and
> profiling on-device
> 
> The previous version of the patch extended the QWebInspector API but a
> discussion with laszlo and kenne on #qtwebkit led to this QWebSettings API. I
> have no particular preference between the two.

Ah, so when you bring up the inspector with a remote URL, do you actually use any functionaliy of the inspector backend?

Do I understand correctly that the _use case_ behind this is that you have WebKit running on a mobile platform, with the embedded web server enabled. Then on your _desktop_ machine you take a QWebInspector, set the remote inspector url to the address of the mobile device and fire up the inspector?

In that case, how much of QWebInspector is actually used? Or could you also just point a QWebView to the remote url and it would work just as well?

Please elaborate and explain :), then it's easier for us to understand the API.

[Jamey Hicks]

Created attachment 50213 [details]
diagram of remote Web Inspector use case

In this use case, the inspected webkit (on the left) exports a JSON over HTTP interface to the full inspector backend functionality, enabling the Web Inspector UI to be run on a different machine.

In order for the javascript to accept a connection, either it will have to use an intermediary, so that both inspected and inspector javascript use HTTP client sockets, or one of the browsers will need to use an extension enabling HTTP server sockets.

[Jamey Hicks]

The frontend URL being specified in this patch is actually a local URL. In the use cases we are most interested in, the URL exposes an interface to the inspector backend so that the backend may be used remotely.

[Simon Hausmann]

I see, thanks Jamey for the explanation!

I understand that this feature is still in development, so for the moment you don't really need a finalized public API, right?

What about the option of building WebKit without the embedded .qrc?

Then you could simply load your own resource with with the different inspector html/js/css files at run-time through QResource::registerResource and qrc:/webkit/inspector as mapping root?


Perhaps we should always build WebKit with the inspector frontend separated and just install the bundle together with WebKit.

[Jamey Hicks]

(In reply to comment #19)
> I see, thanks Jamey for the explanation!
> 
> I understand that this feature is still in development, so for the moment you
> don't really need a finalized public API, right?

We need a finalized public API this month to get it into the next software release for phones. The development of HTML and javascript for remote access to the backend can proceed in parallel with that.

> What about the option of building WebKit without the embedded .qrc?
> 
> Then you could simply load your own resource with with the different inspector
> html/js/css files at run-time through QResource::registerResource and
> qrc:/webkit/inspector as mapping root?

That would work. It makes it difficult to specify per page inspector URLs but then again that's not a requirement for our use cases.

[Laszlo Gombos]

> I understand that this feature is still in development, so for the moment you
> don't really need a finalized public API, right?
> 
> What about the option of building WebKit without the embedded .qrc?


At least in my mind that was very much one of the use-cases as the resources are not enabled by default on Symbian (and maybe they should not be enabled for maemo either).

I did try to use this API to see if it enables this use-case and heppy to report that it did work.

[Laszlo Gombos]

Simon, do you have a better proposal for the API, or should we just close this bug so that it is ready for the release ?

[Diego Gonzalez]

(In reply to comment #22)
> Simon, do you have a better proposal for the API, or should we just close this
> bug so that it is ready for the release ?

It seems this bug could be closed? Isn't it?

[Simon Hausmann]

How about using a dynamic property on QWebInspector instead of this public API?

AFAICS the purpose of this API is to develop the remote inspector feature. It is not meant to be useful for developers using QtWebKit for application development.

[Jamey Hicks]

(In reply to comment #24)
> How about using a dynamic property on QWebInspector instead of this public API?
> 

A dynamic property would also work. As an internal property, should the name of the property begin with _q_? In this case _q_inspectorFrontendUrl ?

As we are debugging the remote debugger, a second useful property would be to create the inspector page without showing it, which could be controlled by property _q_createInspectorImmediately or something like that.

[Simon Hausmann]

(In reply to comment #25)
> (In reply to comment #24)
> > How about using a dynamic property on QWebInspector instead of this public API?
> > 
> 
> A dynamic property would also work. As an internal property, should the name of
> the property begin with _q_? In this case _q_inspectorFrontendUrl ?

Yeah, that's okay.

Just remember to never use these properties in application code outside the WebKit source tree. So the use in DRT or QtLauncher is fine. Using it in other application is bad and will cause regressions.

> As we are debugging the remote debugger, a second useful property would be to
> create the inspector page without showing it, which could be controlled by
> property _q_createInspectorImmediately or something like that.

Hm, when should it be created instead? The first time show() is called?

[Jocelyn Turcotte]

Any update on this?

[Laszlo Gombos]

(In reply to comment #26)
> (In reply to comment #25)
> > (In reply to comment #24)
> > > How about using a dynamic property on QWebInspector instead of this public API?
> > > 
> > 
> > A dynamic property would also work. As an internal property, should the name of
> > the property begin with _q_? In this case _q_inspectorFrontendUrl ?
> 
> Yeah, that's okay.
> 
> Just remember to never use these properties in application code outside the
> WebKit source tree. So the use in DRT or QtLauncher is fine. Using it in other
> application is bad and will cause regressions.

I think the use-case requires using this API outside of WebKit. I'd like to see this functionality API publicly exposed as a supported API.

[Simon Hausmann]

(In reply to comment #28)
> (In reply to comment #26)
> > (In reply to comment #25)
> > > (In reply to comment #24)
> > > > How about using a dynamic property on QWebInspector instead of this public API?
> > > > 
> > > 
> > > A dynamic property would also work. As an internal property, should the name of
> > > the property begin with _q_? In this case _q_inspectorFrontendUrl ?
> > 
> > Yeah, that's okay.
> > 
> > Just remember to never use these properties in application code outside the
> > WebKit source tree. So the use in DRT or QtLauncher is fine. Using it in other
> > application is bad and will cause regressions.
> 
> I think the use-case requires using this API outside of WebKit. I'd like to see
> this functionality API publicly exposed as a supported API.

I think it makes sense to expose this as a supported API once the feature - remote debugging with the web inspector - is complete, so that it becomes an actually useful feature for third-party developers.

It is my understanding that at this point this is merely a tool during the development.

[Simon Hausmann]

Created attachment 55124 [details]
Patch to replace public API with private dynamic property

[WebKit Commit Bot]

Comment on attachment 55124 [details]
Patch to replace public API with private dynamic property

Clearing flags on attachment: 55124

Committed r58876: <http://trac.webkit.org/changeset/58876>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Simon Hausmann]

Re-opening

[Kenneth Rohde Christiansen]

(In reply to comment #33)
> Re-opening

Why was this reopened?

[Simon Hausmann]

Landed http://trac.webkit.org/changeset/59231 to clarify that the dynamic property is there on purpose.

[Simon Hausmann]

<cherry-pick-for-backport: r59231>

[Simon Hausmann]

Revision r55273 cherry-picked into qtwebkit-2.0 with commit d63440e7437006c1105bc17f769a07812512fa57
Revision r58876 cherry-picked into qtwebkit-2.0 with commit 015d13573930a82c684dec0f30a1c92bbb5534e2
Revision r59231 cherry-picked into qtwebkit-2.0 with commit dc1fbb8ce49e6066c6136e5dfcf66fd35fb77af8

[Eugene Ostroukhov]

> We are enabling remote usage of all the inspector functionality. I'm working on two such remote cases:
> 1) Using Web Inspector UI remoted over HTTP to enable on-device debug and profiling.
> 2) Implementing ChromeDevTools protocol for Eclipse or Aptana debug and profiling on-device

Will these parts be available as a part of Qt or they will be parts of proprietary code?