Bug 37125 – Remove obsolete MOBILE flag

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Laszlo Gombos]

Currently the MOBILE flag is unconditionally disabled in WebCore/config.h.

[Laszlo Gombos]

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)]

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)]

This bug is not security sensitive, removing flag.

[Laszlo Gombos]

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]

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)]

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]

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)]

(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)]

(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]

Android does not use this flag.

[Eric Seidel (no email)]

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]

Created attachment 52701 [details]
3rd try - remove the MOBILE flag

Thanks for all the feedback.

[WebKit Commit Bot]

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]

All reviewed patches have been landed.  Closing bug.