Bug 26015

Summary: [Qt] Single-threaded QtWebKit configuration
Product: WebKit Reporter: Laszlo Gombos <laszlo.gombos>
Component: WebKit QtAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: ariya.hidayat
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Attachments:
Description Flags
Proposed patch.
none
Updated the patch to use ENABLE_SINGLE_THREADED as the guard.
ariya.hidayat: review+
Fix compilation warnings in ThreadingNone.cpp
zecke: review+
Use UNUSED_PARAM macro for unused arguments as Ariya suggested. none

Laszlo Gombos
Reported 2009-05-25 19:02:30 PDT
The following patch introduces a new configuration option (ENABLE_NO_THREADING) for the Qt port to build single-threaded WebKit. The proposed change impacts only the build system. The single-threaded option builds on top of existing WebKit configuration options. The motivation of this change is to maintain a single-threaded QtWebKit option for embedded systems where creating threads dynamically is not desired.
Attachments
Proposed patch. (4.10 KB, patch)
2009-05-25 19:46 PDT, Laszlo Gombos
no flags
Updated the patch to use ENABLE_SINGLE_THREADED as the guard. (4.20 KB, patch)
2009-06-01 11:59 PDT, Laszlo Gombos
ariya.hidayat: review+
Fix compilation warnings in ThreadingNone.cpp (1.28 KB, patch)
2009-06-05 20:23 PDT, Laszlo Gombos
zecke: review+
Use UNUSED_PARAM macro for unused arguments as Ariya suggested. (1.54 KB, patch)
2009-06-08 14:15 PDT, Laszlo Gombos
no flags
Laszlo Gombos
Comment 1 2009-05-25 19:46:38 PDT
Created attachment 30662 [details] Proposed patch.
Mark Rowe (bdash)
Comment 2 2009-05-26 12:57:51 PDT
ENABLE_NO_THREADING is an awkward name. Why not DISABLE_THREADING, or set ENABLE_THREADING=0?
Laszlo Gombos
Comment 3 2009-05-26 17:29:59 PDT
(In reply to comment #2) > ENABLE_NO_THREADING is an awkward name. Why not DISABLE_THREADING, or set > ENABLE_THREADING=0? The guidelines I set to myself - stay consistent with the ENABLE_XXX naming schema - the build should not change if the newly introduced flag is not set to minimize the impact of this less frequently used configuration. How about ENABLE_SINGLE_THREADED ?
Laszlo Gombos
Comment 4 2009-06-01 11:59:17 PDT
Created attachment 30838 [details] Updated the patch to use ENABLE_SINGLE_THREADED as the guard.
Ariya Hidayat
Comment 5 2009-06-04 04:04:31 PDT
Landed in 44411
Laszlo Gombos
Comment 6 2009-06-05 20:23:32 PDT
Created attachment 31026 [details] Fix compilation warnings in ThreadingNone.cpp It seems to me that changes to ThreadingNone.cpp has not been committed as part of 44411; created a new patch just for those changes.
Laszlo Gombos
Comment 7 2009-06-05 20:26:00 PDT
Reopen as part of https://bugs.webkit.org/attachment.cgi?id=30838 has not been committed as part of 44411.
Ariya Hidayat
Comment 8 2009-06-08 02:41:12 PDT
I prefer that we use UNUSED_PARAM macro. The argument names are always useful to help understanding the function.
Laszlo Gombos
Comment 9 2009-06-08 14:15:11 PDT
Created attachment 31062 [details] Use UNUSED_PARAM macro for unused arguments as Ariya suggested.
Holger Freyther
Comment 10 2009-06-09 07:51:51 PDT
(In reply to comment #8) > I prefer that we use UNUSED_PARAM macro. The argument names are always useful > to help understanding the function. Hehe, it is a matter of taste. In general we omit parameter names when it is obvious. I would argue that for a *None file the parameter names are not relevant at all and should be omitted. Something funny I stumbled across is in in UnusedParam.h: /* don't use this for C++, it should only be used in plain C files or ObjC methods, where leaving off the parameter name is not allowed. */ #define UNUSED_PARAM(x) (void)x we obviously don't stick to that rule...
Laszlo Gombos
Comment 11 2009-06-09 10:41:08 PDT
I share Zecke's opinion for *None files so I canceled the review for https://bugs.webkit.org/attachment.cgi?id=31062.
Ariya Hidayat
Comment 12 2009-06-11 10:35:26 PDT
Closing it, since the issue is settled
Laszlo Gombos
Comment 13 2009-06-11 10:41:54 PDT
(In reply to comment #12) > Closing it, since the issue is settled Ariya, I think we should still commit https://bugs.webkit.org/attachment.cgi?id=31026 before closing this bug. Change the status back to REOPEN.
Ariya Hidayat
Comment 14 2009-06-11 10:58:27 PDT
Oops, sorry. I didn't read it carefully. Landed in r44607 http://trac.webkit.org/changeset/44607
Note You need to log in before you can comment on or make changes to this bug.