[Merge] lp:~abreu-alexandre/webbrowser-app/host-support-devtools into lp:webbrowser-app
Olivier Tilloy
olivier.tilloy at canonical.com
Mon Oct 6 14:04:16 UTC 2014
Looks good, but I have one comment (see inline) to improve code readability.
Diff comments:
> === modified file 'src/Ubuntu/Web/UbuntuWebContext.qml'
> --- src/Ubuntu/Web/UbuntuWebContext.qml 2014-08-11 15:13:50 +0000
> +++ src/Ubuntu/Web/UbuntuWebContext.qml 2014-10-06 13:57:05 +0000
> @@ -82,4 +82,5 @@
>
> devtoolsEnabled: webviewDevtoolsDebugPort !== -1
> devtoolsPort: webviewDevtoolsDebugPort
> + devtoolsIp: webviewDevtoolsDebugHost
> }
>
> === modified file 'src/Ubuntu/Web/plugin.cpp'
> --- src/Ubuntu/Web/plugin.cpp 2014-09-26 13:30:46 +0000
> +++ src/Ubuntu/Web/plugin.cpp 2014-10-06 13:57:05 +0000
> @@ -83,6 +83,18 @@
> return port > 0 ? port : DEVTOOLS_INVALID_PORT;
> }
>
> +
> +static QString getDevtoolsHost()
> +{
> + QString host;
> + const char* DEVTOOLS_HOST_ENV_VAR = "UBUNTU_WEBVIEW_DEVTOOLS_HOST";
> +
> + if (qEnvironmentVariableIsSet(DEVTOOLS_HOST_ENV_VAR)) {
> + host = qgetenv(DEVTOOLS_HOST_ENV_VAR);
> + }
> + return host;
> +}
> +
> void UbuntuBrowserPlugin::initializeEngine(QQmlEngine* engine, const char* uri)
> {
> Q_UNUSED(uri);
> @@ -104,6 +116,7 @@
> context->setContextProperty("webviewDevtoolsDebugPort", getDevtoolsPort());
>
> engine->addImageProvider("favicon", new FaviconImageProvider());
> + context->setContextProperty("webviewDevtoolsDebugHost", getDevtoolsHost());
This line is not at the right place (although it won’t affect the functionality, it does affect code readability).
Can you please move it three lines up, to be next to the setting of the "webviewDevtoolsDebugPort" property?
> }
>
> void UbuntuBrowserPlugin::registerTypes(const char* uri)
>
> === modified file 'src/app/browserapplication.cpp'
> --- src/app/browserapplication.cpp 2014-09-15 17:08:41 +0000
> +++ src/app/browserapplication.cpp 2014-10-06 13:57:05 +0000
> @@ -77,6 +77,18 @@
> return port;
> }
>
> +QString BrowserApplication::inspectorHost() const
> +{
> + QString host;
> + Q_FOREACH(QHostAddress address, QNetworkInterface::allAddresses()) {
> + if (!address.isLoopback() && (address.protocol() == QAbstractSocket::IPv4Protocol)) {
> + host = address.toString();
> + break;
> + }
> + }
> + return host;
> +}
> +
> QString BrowserApplication::appId() const
> {
> Q_FOREACH(const QString& argument, m_arguments) {
> @@ -123,8 +135,10 @@
> #endif
>
> QString devtoolsPort = inspectorPort();
> + QString devtoolsHost = inspectorHost();
> bool inspectorEnabled = !devtoolsPort.isEmpty();
> if (inspectorEnabled) {
> + qputenv("UBUNTU_WEBVIEW_DEVTOOLS_HOST", devtoolsHost.toUtf8());
> qputenv("UBUNTU_WEBVIEW_DEVTOOLS_PORT", devtoolsPort.toUtf8());
> }
>
>
> === modified file 'src/app/browserapplication.h'
> --- src/app/browserapplication.h 2014-07-03 19:49:54 +0000
> +++ src/app/browserapplication.h 2014-10-06 13:57:05 +0000
> @@ -59,6 +59,7 @@
> private:
> QString appId() const;
> QString inspectorPort() const;
> + QString inspectorHost() const;
>
> WebBrowserWindow *m_webbrowserWindowProxy;
> };
>
--
https://code.launchpad.net/~abreu-alexandre/webbrowser-app/host-support-devtools/+merge/236965
Your team Ubuntu Phablet Team is subscribed to branch lp:webbrowser-app.
More information about the Ubuntu-reviews
mailing list