[webkit-reviews] review denied: [Bug 39357] [QT] QT_BEARER flag is not enabled on S60 properly : [Attachment 56488] proposed solution

[bugzilla-daemon at webkit.org]

Laszlo Gombos <laszlo.1.gombos at nokia.com> has denied maheshK
<mahesh.kulkarni at nokia.com>'s request for review:
Bug 39357: [QT] QT_BEARER flag is not enabled on S60 properly
https://bugs.webkit.org/show_bug.cgi?id=39357

Attachment 56488: proposed solution
https://bugs.webkit.org/attachment.cgi?id=56488&action=review

------- Additional Comments from Laszlo Gombos <laszlo.1.gombos at nokia.com>
>  !CONFIG(QTDIR_build):!contains(DEFINES, ENABLE_QT_BEARER=.) {
> -    symbian: {
> -	   exists($${EPOCROOT}epoc32/release/winscw/udeb/QtBearer.lib)| \
> -	   exists($${EPOCROOT}epoc32/release/armv5/lib/QtBearer.lib) {
> -	       DEFINES += ENABLE_QT_BEARER=1
> -	   }

I find this existing build logic useful to continue building WebKit trunk for
Symbian devices with QtNetwork 4.6 and a separate QtBearer module. We can maybe
add an extra guard around this logic to state explicitly that this only make
sense if Qt version is less than 4.7, but I prefer not to remove this code.

> +    exists($$[QMAKE_MKSPECS]/features/mobility.prf) {
> +	  DEFINES += ENABLE_QT_BEARER=1
>      }

This might make sense for other QtMobility modules (like Geolocation), but it
does not make sense to me for BearerMgmt as BearerMgmt is being rolled into
QtNetwork and it will not be a separate QtMobility module going forward.

We should check for Qt version to see if QtNetwork module supports BearerMgmt,
but that is already part of the build system.

# Bearer management is part of Qt 4.7
!lessThan(QT_MINOR_VERSION, 7):!contains(DEFINES, ENABLE_QT_BEARER=.):DEFINES
+= ENABLE_QT_BEARER=1

r- for taking this feature discovery approach for BearerMgmt; this will make
more sense to mo for other QtMobility modules (e.g. Geolocation).