[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