Bug 35340

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

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

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.

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

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.

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.

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

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>

All reviewed patches have been landed. Closing bug.

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.

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.

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?

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.

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

(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.

(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.

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.

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.

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.

(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.

> 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.

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 ?

(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?

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.

(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.

(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?

Any update on this?

(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.

(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.

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

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>

All reviewed patches have been landed. Closing bug.

Re-opening

(In reply to comment #33) > Re-opening Why was this reopened?

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

<cherry-pick-for-backport: r59231>

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

> 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?