Bug 37727

Summary: [Qt] Webkit fixes for RVCT4
Product: WebKit Reporter: Iain Campbell <iain.campbell>
Component: PlatformAssignee: QtWebKit Unassigned <webkit-qt-unassigned>
Status: CLOSED FIXED    
Severity: Normal CC: hausmann, koshuin, laszlo.gombos
Priority: P2 Keywords: Qt
Version: 528+ (Nightly build)   
Hardware: S60 Hardware   
OS: Other   
Bug Depends on:    
Bug Blocks: 35784    
Attachments:
Description Flags
Patch kenneth: review+

Description Iain Campbell 2010-04-16 13:48:15 PDT
RVCT 4 is far more strict with regards to symbol visiblity that RVCT2.2, and will hide symbols unless all references have default visibility in the object files. This patch corrects the symbol visibility which was set incorrectly for DLL-based platforms like Symbian (those that use __declspec(dllimport) and (dllexport)).
Comment 1 Iain Campbell 2010-04-16 13:50:43 PDT
diff --git a/src/3rdparty/webkit/WebCore/page/qt/EventHandlerQt.cpp b/src/3rdparty/webkit/WebCore/page/qt/EventHandlerQt.cpp
index d7982fa..2fcce5b 100644
--- a/src/3rdparty/webkit/WebCore/page/qt/EventHandlerQt.cpp
+++ b/src/3rdparty/webkit/WebCore/page/qt/EventHandlerQt.cpp
@@ -51,7 +51,7 @@
 #include "NotImplemented.h"

 QT_BEGIN_NAMESPACE
-extern Q_GUI_EXPORT bool qt_tab_all_widgets; // from qapplication.cpp
+Q_DECL_IMPORT extern bool qt_tab_all_widgets; // from qapplication.cpp
 QT_END_NAMESPACE

 namespace WebCore {
Comment 2 Simon Hausmann 2010-04-16 14:31:45 PDT
(In reply to comment #1)
> diff --git a/src/3rdparty/webkit/WebCore/page/qt/EventHandlerQt.cpp
> b/src/3rdparty/webkit/WebCore/page/qt/EventHandlerQt.cpp
> index d7982fa..2fcce5b 100644
> --- a/src/3rdparty/webkit/WebCore/page/qt/EventHandlerQt.cpp
> +++ b/src/3rdparty/webkit/WebCore/page/qt/EventHandlerQt.cpp
> @@ -51,7 +51,7 @@
>  #include "NotImplemented.h"
> 
>  QT_BEGIN_NAMESPACE
> -extern Q_GUI_EXPORT bool qt_tab_all_widgets; // from qapplication.cpp
> +Q_DECL_IMPORT extern bool qt_tab_all_widgets; // from qapplication.cpp

So this patch does two things:

    1) It removes the extern keyword

and

    2) It replaces Q_GUI_EXPORT with Q_DECL_IMPORT

AFAIK Q_GUI_EXPORT should be defined to Q_DECL_IMPORT in qglobal.h _unless_ we're building QtGui itself, which is not the case here.

Can you elaborate how your fix works? :)
Comment 3 Janne Koskinen 2010-04-20 00:50:30 PDT
(In reply to comment #2)
>     1) It removes the extern keyword
>     2) It replaces Q_GUI_EXPORT with Q_DECL_IMPORT

It doesn't remove the extern keyword, but swaps declspec and extern.
You are right that there is no need to replace Q_GUI_EXPORT with Q_DECL_IMPORT i.e.
Q_GUI_EXPORT extern bool qt_tab_all_widgets; // from qapplication.cpp
would be equally good fix.
Comment 4 Simon Hausmann 2010-04-20 07:14:23 PDT
(In reply to comment #3)
> (In reply to comment #2)
> >     1) It removes the extern keyword
> >     2) It replaces Q_GUI_EXPORT with Q_DECL_IMPORT
> 
> It doesn't remove the extern keyword, but swaps declspec and extern.
> You are right that there is no need to replace Q_GUI_EXPORT with Q_DECL_IMPORT
> i.e.
> Q_GUI_EXPORT extern bool qt_tab_all_widgets; // from qapplication.cpp
> would be equally good fix.

Ooops, I see, so the fix is to swap the two?
Comment 5 Simon Hausmann 2010-04-26 08:05:00 PDT
Created attachment 54301 [details]
Patch
Comment 6 Simon Hausmann 2010-04-26 08:12:11 PDT
Committed r58257: <http://trac.webkit.org/changeset/58257>
Comment 7 Simon Hausmann 2010-04-26 08:12:39 PDT
Cherry-picked into qtwebkit-4.6 with commit 4fb414b38f7c7c8439ce6a4323f1acb057a3ff20
Comment 8 Laszlo Gombos 2010-04-26 08:13:49 PDT
Two more instances of this - maybe we want to change those as well:

WebKit/qt/Api/qwebframe.cpp:extern Q_GUI_EXPORT int qt_defaultDpi();
WebCore/plugins/mac/PluginViewMac.cpp:extern Q_GUI_EXPORT OSWindowRef qt_mac_window_for(const QWidget* w);
Comment 9 Simon Hausmann 2010-04-26 08:25:39 PDT
Revision r58257 cherry-picked into qtwebkit-2.0 with commit b8a9d4b4dd69f952857333bcd5596989008560f3