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+

Iain Campbell
Reported 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)).
Attachments
Patch (1008 bytes, patch)
2010-04-26 08:05 PDT, Simon Hausmann 		kenneth: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Iain Campbell
Comment 1 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 {
Simon Hausmann
Comment 2 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? :)
Janne Koskinen
Comment 3 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.
Simon Hausmann
Comment 4 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?
Simon Hausmann
Comment 5 2010-04-26 08:05:00 PDT
Created attachment 54301 [details] Patch
Simon Hausmann
Comment 6 2010-04-26 08:12:11 PDT
Committed r58257: <http://trac.webkit.org/changeset/58257>
Simon Hausmann
Comment 7 2010-04-26 08:12:39 PDT
Cherry-picked into qtwebkit-4.6 with commit 4fb414b38f7c7c8439ce6a4323f1acb057a3ff20
Laszlo Gombos
Comment 8 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);
Simon Hausmann
Comment 9 2010-04-26 08:25:39 PDT
Revision r58257 cherry-picked into qtwebkit-2.0 with commit b8a9d4b4dd69f952857333bcd5596989008560f3
Note You need to log in before you can comment on or make changes to this bug.