Summary: | Remove obsolete MOBILE flag | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Laszlo Gombos <laszlo.gombos> | ||||||||
Component: | Platform | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | aroben, benm, commit-queue, dacarson, dbates, ddkilzer, eric, koivisto, ostap73, staikos, steveblock | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | PC | ||||||||||
OS: | All | ||||||||||
Attachments: |
|
Description
Laszlo Gombos
2010-04-05 16:14:30 PDT
Created attachment 52587 [details]
1st try, remove the define
Alternatively we can put the define around guards like other feature flags, like this:
#ifndef MOBILE
#define MOBILE 0
#endif
I'm also happy to change the name of the define if there is a better proposal. I think a global define like MOBILE does not quite make sense.
I suspect this was added for iPhone: http://trac.webkit.org/changeset/23281 That or windows mobile. ;p It probably isn't needed anymore, but only someone on team iPhone (like ddkilzer) would know. This bug is not security sensitive, removing flag. The MOBILE flag only used in WebCore/html/HTMLTokenizer.cpp at the moment and sets up some sensible defaults for CPU-constrained environments. Those defaults that are controlled by this flag can be changed runtime, so I have no strong preference to keep the build flag around. Hope others can chip in as well. This flag is really old - I think it predates everything except S60 browser in terms of mobile webkit. We don't use it. If we don't here back from ddkilzer and co. then I suggest changing this to #ifndef MOBILE and then I would feel comfortable r+ing it. Created attachment 52608 [details]
2nd try
Change the patch based on Eric's feedback to keep a default value for the MOBILE flag in config.h but only set it if it was not defined before.
Also took the liberty of renaming the flag to ENABLE_MOBILE_TOKENIZER.
(In reply to comment #6) > If we don't here back from ddkilzer and co. then I suggest changing this to > #ifndef MOBILE and then I would feel comfortable r+ing it. This macro was added for Android in r16821. http://trac.webkit.org/changeset/16821 You'll have to ask them if they still use it. (In reply to comment #8) > (In reply to comment #6) > > If we don't here back from ddkilzer and co. then I suggest changing this to > > #ifndef MOBILE and then I would feel comfortable r+ing it. > > This macro was added for Android in r16821. > > http://trac.webkit.org/changeset/16821 > > You'll have to ask them if they still use it. Or maybe it was Nokia. In either case, it was not added for iPhone and we don't use it. :) Android does not use this flag. S60 never built out of trunk, right? And Nokia is now Qt. So it seems we should just rip out this flag and all the associated code. Created attachment 52701 [details]
3rd try - remove the MOBILE flag
Thanks for all the feedback.
Comment on attachment 52701 [details] 3rd try - remove the MOBILE flag Clearing flags on attachment: 52701 Committed r57193: <http://trac.webkit.org/changeset/57193> All reviewed patches have been landed. Closing bug. |