Bug 37125

Summary: Remove obsolete MOBILE flag
Product: WebKit Reporter: Laszlo Gombos <laszlo.gombos>
Component: PlatformAssignee: 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 Flags
1st try, remove the define
none
2nd try
none
3rd try - remove the MOBILE flag none

Laszlo Gombos
Reported 2010-04-05 16:14:30 PDT
Currently the MOBILE flag is unconditionally disabled in WebCore/config.h.
Attachments
1st try, remove the define (881 bytes, patch)
2010-04-05 16:30 PDT, Laszlo Gombos
no flags
2nd try (1.93 KB, patch)
2010-04-05 23:03 PDT, Laszlo Gombos
no flags
3rd try - remove the MOBILE flag (2.48 KB, patch)
2010-04-06 21:13 PDT, Laszlo Gombos
no flags
Laszlo Gombos
Comment 1 2010-04-05 16:30: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.
Eric Seidel (no email)
Comment 2 2010-04-05 16:56:12 PDT
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.
Eric Seidel (no email)
Comment 3 2010-04-05 16:56:31 PDT
This bug is not security sensitive, removing flag.
Laszlo Gombos
Comment 4 2010-04-05 17:54:06 PDT
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.
George Staikos
Comment 5 2010-04-05 18:12:21 PDT
This flag is really old - I think it predates everything except S60 browser in terms of mobile webkit. We don't use it.
Eric Seidel (no email)
Comment 6 2010-04-05 19:44:06 PDT
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.
Laszlo Gombos
Comment 7 2010-04-05 23:03:00 PDT
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.
David Kilzer (:ddkilzer)
Comment 8 2010-04-06 08:45:10 PDT
(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.
David Kilzer (:ddkilzer)
Comment 9 2010-04-06 08:47:42 PDT
(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. :)
Ben Murdoch
Comment 10 2010-04-06 10:08:50 PDT
Android does not use this flag.
Eric Seidel (no email)
Comment 11 2010-04-06 16:56:03 PDT
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.
Laszlo Gombos
Comment 12 2010-04-06 21:13:33 PDT
Created attachment 52701 [details] 3rd try - remove the MOBILE flag Thanks for all the feedback.
WebKit Commit Bot
Comment 13 2010-04-06 21:58:36 PDT
Comment on attachment 52701 [details] 3rd try - remove the MOBILE flag Clearing flags on attachment: 52701 Committed r57193: <http://trac.webkit.org/changeset/57193>
WebKit Commit Bot
Comment 14 2010-04-06 21:58:43 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.