Bug 103844 - [Qt] Use a separate QWEBKITWIDGETS_EXPORT macro for WebKitWidgets
Summary: [Qt] Use a separate QWEBKITWIDGETS_EXPORT macro for WebKitWidgets
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Pierre Rossi
URL:
Keywords:
Depends on:
Blocks: 103859
  Show dependency tree / graph
 
Reported: 2012-12-02 15:40 PST by Pierre Rossi
Modified: 2012-12-03 06:53 PST (History)
3 users (show)

See Also:

Attachments
Patch (5.26 KB, patch)
2012-12-02 16:22 PST, Pierre Rossi 		no flags Details | Formatted Diff | Diff
Patch (6.48 KB, patch)
2012-12-03 06:12 PST, Pierre Rossi 		hausmann: review+
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Pierre Rossi 2012-12-02 15:40:11 PST 
[Qt] Use a separate QWEBKITWIDGETS_EXPORT macro for WebKitWidgets
Comment 1 Pierre Rossi 2012-12-02 16:22:16 PST 
Created attachment 177161 [details]
Patch
Comment 2 Thiago Macieira 2012-12-02 23:27:53 PST 
Looks good to me.

Won't this break Qt 4 builds? Also, please note that InitWebKitQt.h will need some more work (see the bug I filed)
Comment 3 Andras Becsi 2012-12-03 03:28:49 PST 
(In reply to comment #2)
> Looks good to me.
> 
> Won't this break Qt 4 builds? Also, please note that InitWebKitQt.h will need some more work (see the bug I filed)

We do not support Qt4 on trunk any more, so this should not be a problem, and as far as I can tell InitWebKitQt.h is only included in widget specific code (qtwebkitwidgets library), so it shouldn't be an issue either.
Comment 4 Pierre Rossi 2012-12-03 06:12:04 PST 
Created attachment 177241 [details]
Patch
Comment 5 Simon Hausmann 2012-12-03 06:45:16 PST 
Comment on attachment 177241 [details]
Patch

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

Excellent solution. Fully agreed that this is much cleaner. Hadn't realized that qt_module.prf sets the QT_BUILDING_*_LIB macros automatically. Lovely.

Carefuly when landing wrt MSVC

> Source/WebKit/qt/WidgetSupport/InitWebKitQt.cpp:33
> -Q_DECL_EXPORT void initializeWebKitWidgets()
> +void initializeWebKitWidgets()

I think that this is likely to cause compilation errors on Windows. I've seen MSVC at least complain that the declaration had to match the definition. I suggest to land carefully in trunk first to see of the MSVC bot complains or not.
Comment 6 Pierre Rossi 2012-12-03 06:50:50 PST 
(In reply to comment #5)
> (From update of attachment 177241 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=177241&action=review
> 
> Excellent solution. Fully agreed that this is much cleaner. Hadn't realized that qt_module.prf sets the QT_BUILDING_*_LIB macros automatically. Lovely.
> 
> Carefuly when landing wrt MSVC
> 
> > Source/WebKit/qt/WidgetSupport/InitWebKitQt.cpp:33
> > -Q_DECL_EXPORT void initializeWebKitWidgets()
> > +void initializeWebKitWidgets()
> 
> I think that this is likely to cause compilation errors on Windows. I've seen MSVC at least complain that the declaration had to match the definition. I suggest to land carefully in trunk first to see of the MSVC bot complains or not.

Alright, I'll put it back for landing then, just to be on the safe side.
Comment 7 Pierre Rossi 2012-12-03 06:53:09 PST 
Committed r136394: <http://trac.webkit.org/changeset/136394>