Bug 73094 – InspectorServer: Enable and connect the WebInspectorServer with WebKit2 pages.

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

[Jocelyn Turcotte]

Created attachment 116542 [details]
Original patch

Tracks the "New Part 5" patch of bug #51364.

[Jocelyn Turcotte]

Created attachment 116543 [details]
Patch

This patch depends on another patch exposing the DeveloperExtras preference that I'll upload once this looks good.

[Jocelyn Turcotte]

Sam, Timothy, could you have a look at WebInspectorProxy, WebPageProxy and WebInspector to tell me if the approach looks reasonable?

[Jocelyn Turcotte]

Created attachment 117925 [details]
Patch

Changes from last patch:
- The actual server implementation has been moved to bug #73855 and this patch now only contains the bindings.

[Joseph Pecoraro]

Comment on attachment 117925 [details]
Patch

This patch looks like it adds InspectorServer to the build and includes "WebInspectorServer.h" but I don't see those files anywhere. Are they in another patch or am I just missing them? The path looks okay from my perspective.

[Jocelyn Turcotte]

(In reply to comment #4)
> (From update of attachment 117925 [details])
> This patch looks like it adds InspectorServer to the build and includes "WebInspectorServer.h" but I don't see those files anywhere. Are they in another patch or am I just missing them? The path looks okay from my perspective.

The files are added in bug #73855 but adding the include path to other builds shouldn't be necessary if I guard the include with ENABLE(INSPECTOR_SERVER).
I'll upload an update.

[Jocelyn Turcotte]

Created attachment 118425 [details]
Patch

Guard the include instead of adding InspectorServer to platforms without ENABLE(INSPECTOR_SERVER)

[Yael]

Comment on attachment 118425 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=118425&action=review

> Source/WebKit2/UIProcess/WebPageProxy.cpp:1500
> +#endif

You shouldn't register the page unconditionally each time that preferences change. You will end up with the same page being registered multiple times.

[Yael]

Sorry, the context of my comment got messed up. This is about 
>@@ -1489,6 +1494,11 @@ void WebPageProxy::preferencesDidChange()
>     if (!isValid())
>         return;
> 
>+#if ENABLE(INSPECTOR_SERVER)
>+    if (m_pageGroup->preferences()->developerExtrasEnabled())
>+        inspector()->enableRemoteInspection();
>+#endif

[Vsevolod Vlasov]

Comment on attachment 118425 [details]
Patch

r- per comment #7

[Jocelyn Turcotte]

Created attachment 134629 [details]
Patch

- Rebased
- Fixed the registration problem

[Early Warning System Bot]

Comment on attachment 134629 [details]
Patch

Attachment 134629 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/12202425

[Kenneth Rohde Christiansen]

Comment on attachment 134629 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=134629&action=review

> Source/WebKit2/ChangeLog:9
> +        Pages are registered and unregistered as they are created
> +        and destroyed if they have developer extras enabled. The server is

So they are destroyed if developer extras is enabled? :) I think this could be written to avoid confusion.

> Source/WebKit2/UIProcess/qt/QtWebContext.cpp:65
> +            LOG_ERROR("Non numeric port for the inspector server \"%s\". Examples of valid input: \"12345\" or \"192.168.2.14:12345\" (with the address of one of this host's interface).", qPrintable(portStr));

LOG_ERROR here

> Source/WebKit2/UIProcess/qt/QtWebContext.cpp:72
> +            qWarning("Inspector server started successfully. Try pointing a WebKit browser to %s", qPrintable(inspectorServerUrl));

Then qWarning here...

> Source/WebKit2/UIProcess/qt/QtWebContext.cpp:78
> +static void initWebKit()

Could we find a better name?

[Jocelyn Turcotte]

Created attachment 135100 [details]
Patch

- Addressed noted issues.
- Changed the Nokia copyrights to 2012 for new files.

[Early Warning System Bot]

Comment on attachment 135100 [details]
Patch

Attachment 135100 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/12311617

[Simon Hausmann]

Comment on attachment 135100 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=135100&action=review

> Source/WebKit2/UIProcess/qt/QtWebContext.cpp:50
> +    QString inspectorEnv = QString::fromLatin1(qgetenv("QTWEBKIT_INSPECTOR_SERVER"));

I suggest to use fromUtf8.

[Jocelyn Turcotte]

Committed r113028: <http://trac.webkit.org/changeset/113028>