Bug 51313

Summary: [Qt] Baseline qt_minimal configuration
Product: WebKit Reporter: Laszlo Gombos <laszlo.gombos>
Component: PlatformAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, commit-queue, ossy, smagnuso
Priority: P2 Keywords: Qt, QtTriaged
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Attachments:
Description Flags
proposed patch
none
same as previous, but replace += to *= in WebKit.pri
ossy: review+, ossy: commit-queue-
Patch with proposed minor changes for cq none

Description Laszlo Gombos 2010-12-19 17:15:56 PST
I'd like to propose to introduce a single-switch build configuration to turn on all the _supported_ QT_NO_FEATURE flags for QtWebKit. This would be similar to the existing "minimal" WebKit configuration (could be called qt_minimal). I would also propose to turn this flag on on the minimal QtWebKit buildbot. Justifications:

1./ It seems that there is interest in the community to make these configurations work (e.g. r69110, r72394, bug 38324, etc.) 
2./ It seems we break these build configurations all the time as no bot is testing them (e.g. bug 51221)
3./ We do not have (yet) have buildbots for some of the embedded ports (e.g. Symbian) and as a gap filler it would be beneficial to use the bot building on Linux in the same configuration as Qt is configured in those embedded systems (e.g. by setting Qt_NO_PRINTER, etc.) This would prevent issues like this one - bug 51204.

Patch will follow.
Comment 1 Laszlo Gombos 2010-12-19 17:49:49 PST
Created attachment 76965 [details]
proposed patch

I left the following configurations out from the baseline as these configurations did not resulted in a functional build (QtTestBrowser usually crashed at startup). I will file a separate bugs/patchess (e.g 51221) to try to fix these (or perhaps remove them and declare them unsupported).

 - QT_NO_ACTION, 
 - QT_NO_CONTEXTMENU
 - QT_NO_DATESTRING" (autotest framework seems to have a dependency on this)
 - QT_NO_DRAGANDDROP
 - QT_NO_LIBRARY
 - QT_NO_PROPERTIES
 - QT_NO_SETTINGS
 - QT_NO_WHEELEVENT
Comment 2 Laszlo Gombos 2010-12-19 18:17:21 PST
Created attachment 76967 [details]
same as previous, but replace += to *= in WebKit.pri
Comment 3 Benjamin Poulain 2010-12-19 18:23:17 PST
I really like the idea of having that tested on a bot. It would make it easier not to break those configurations by accident.

Is QtTestBrowser usable in this qt_minimal configuration?

Maybe you could also create an official qconfig with which it is possible to compile Qt with WebKit? This minimal Qt would be used for testing by us and could be used by users asking for minimal QtWebKit footprint.
Comment 4 Laszlo Gombos 2010-12-19 19:36:42 PST
(In reply to comment #3)
> Is QtTestBrowser usable in this qt_minimal configuration?

Yes.

> Maybe you could also create an official qconfig with which it is possible to compile Qt with WebKit? This minimal Qt would be used for testing by us and could be used by users asking for minimal QtWebKit footprint.

Let's establish that configuration in WebKit and than perhaps we can push it "down" to Qt.
Comment 5 Csaba Osztrogonác 2010-12-22 10:31:39 PST
Comment on attachment 76967 [details]
same as previous, but replace += to *= in WebKit.pri

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

Great patch, thanks for it. ;) Please fix typos and QT_NO_BEARERMANAGEMENT before landing.
(I tested the patch, QtTestBrowser works fine.)

> ChangeLog:9
> +        * WebKit.pri: List the supported QT_NO_FUTURE flags
> +        under qt_minial configuration.

s/QT_NO_FUTURE/QT_NO_FEATURE
s/minial/minimal

> Tools/QtTestBrowser/launcherwindow.h:79
> +class QPropertyAnimation;
> +

Why do we need it?

> WebKit.pri:118
> +    DEFINES *= QT_NO_BEARERMANAGEMENT

To use QT_NO_BEARERMANAGEMENT we need one more little patch. Including of 
moc_NetworkStateNotifierPrivate.cpp should be inside the ENABLE(BEARER) guard:

diff --git a/WebCore/platform/network/qt/NetworkStateNotifierQt.cpp b/WebCore/platform/network/qt/NetworkStateNotifierQt.cpp
index 959e74a..f3e7023 100644
--- a/WebCore/platform/network/qt/NetworkStateNotifierQt.cpp
+++ b/WebCore/platform/network/qt/NetworkStateNotifierQt.cpp
@@ -91,6 +91,6 @@ void NetworkStateNotifier::setNetworkAccessAllowed(bool isAllowed)

 } // namespace WebCore

-#endif
-
 #include "moc_NetworkStateNotifierPrivate.cpp"
+
+#endif
Comment 6 Csaba Osztrogonác 2010-12-22 10:40:55 PST
I'm going to build a qt_minimal with same QT_NO_FEATURES options, and make 
the QtWebKit minimal buildbot use this minimal Qt and build QtWebKit with
additional CONFIG+=qt_minimal flag. It will guarantee that folks won't break
this supported qt_minimal build by accident. 

Additionally we are planning to start running QtWebKit unittests (run-qtwebkit-tests) on the minimal bot too.
Comment 7 Csaba Osztrogonác 2010-12-22 10:48:35 PST
(In reply to comment #1)
>  - QT_NO_DATESTRING" (autotest framework seems to have a dependency on this)
It seems it is a bug in Qt 4.7.1, because toString methods should be
guarded by #ifndef QT_NO_DATESTRING in qtest.h. I'll file a bug in Qt 
bug tracker about it.
Comment 8 Laszlo Gombos 2011-01-10 17:36:34 PST
Created attachment 78477 [details]
Patch with proposed minor changes for cq
Comment 9 WebKit Commit Bot 2011-01-10 18:51:20 PST
Comment on attachment 78477 [details]
Patch with proposed minor changes for cq

Clearing flags on attachment: 78477

Committed r75467: <http://trac.webkit.org/changeset/75467>
Comment 10 WebKit Commit Bot 2011-01-10 18:51:25 PST
All reviewed patches have been landed.  Closing bug.