Remove the compile time flag for DATALIST Element.

Remove the compile time flag for DATALIST Element.

This is part of work for cleaning up #ifdefs from Blink source to reduce complexity.
    DATALIST Element is always Enabled on the chromium side.
    A separate patch ( https://codereview.chromium.org/14861002) will disable them on Android platform.

BUG=

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=150903

[tkent, 2013-05-07 01:14:03]

https://codereview.chromium.org/14859003/diff/1/Source/core/css/html.css
File Source/core/css/html.css (left):

https://codereview.chromium.org/14859003/diff/1/Source/core/css/html.css#oldc...
Source/core/css/html.css:491: #if defined(ENABLE_DATALIST_ELEMENT) &&
ENABLE_DATALIST_ELEMENT
It makes <datalist> display:none even if the runtime flags is off, and feature
detection in web sites will fail.  This rule should be added in
RenderTheme::extraDefaultStyleSheet depending on the runtime flag.

[riju_, 2013-05-20 09:58:53]

On 2013/05/07 01:14:03, tkent wrote:
> https://codereview.chromium.org/14859003/diff/1/Source/core/css/html.css
> File Source/core/css/html.css (left):
> 
>
https://codereview.chromium.org/14859003/diff/1/Source/core/css/html.css#oldc...
> Source/core/css/html.css:491: #if defined(ENABLE_DATALIST_ELEMENT) &&
> ENABLE_DATALIST_ELEMENT
> It makes <datalist> display:none even if the runtime flags is off, and feature
> detection in web sites will fail.  This rule should be added in
> RenderTheme::extraDefaultStyleSheet depending on the runtime flag.

uploaded new patchset, is this the correct way to do it ?

[tkent, 2013-05-20 21:32:11]

We need the runtime flag check in RenderThemeChromiumCommon::supportsDataListUI
too.

https://codereview.chromium.org/14859003/diff/3001/Source/WebKit/chromium/src...
File Source/WebKit/chromium/src/WebInputElement.cpp (right):

https://codereview.chromium.org/14859003/diff/3001/Source/WebKit/chromium/src...
Source/WebKit/chromium/src/WebInputElement.cpp:193: HTMLDataListElement*
dataList =
static_cast<HTMLDataListElement*>(constUnwrap<HTMLInputElement>()->list());
This function should refer to the runtime flag.

https://codereview.chromium.org/14859003/diff/3001/Source/core/css/html.css
File Source/core/css/html.css (right):

https://codereview.chromium.org/14859003/diff/3001/Source/core/css/html.css#n...
Source/core/css/html.css:867: input[type="color"][list] {
These three blocks should be added dynamically by the runtime flag like
datalist{display:none;}.

https://codereview.chromium.org/14859003/diff/3001/Source/core/html/HTMLDataL...
File Source/core/html/HTMLDataListElement.idl (left):

https://codereview.chromium.org/14859003/diff/3001/Source/core/html/HTMLDataL...
Source/core/html/HTMLDataListElement.idl:32: Conditional=DATALIST_ELEMENT,
This should be replaced with EnabledAtRuntime=DatalistElement

https://codereview.chromium.org/14859003/diff/3001/Source/core/html/HTMLInput...
File Source/core/html/HTMLInputElement.cpp (right):

https://codereview.chromium.org/14859003/diff/3001/Source/core/html/HTMLInput...
Source/core/html/HTMLInputElement.cpp:697: }
should be "} else if (..."

https://codereview.chromium.org/14859003/diff/3001/Source/core/html/HTMLInput...
File Source/core/html/HTMLInputElement.idl (right):

https://codereview.chromium.org/14859003/diff/3001/Source/core/html/HTMLInput...
Source/core/html/HTMLInputElement.idl:41: readonly attribute HTMLElement list;
This should have [EnabledAtRuntime=DatalistElement]

https://codereview.chromium.org/14859003/diff/3001/Source/core/html/HTMLOptio...
File Source/core/html/HTMLOptionElement.cpp (right):

https://codereview.chromium.org/14859003/diff/3001/Source/core/html/HTMLOptio...
Source/core/html/HTMLOptionElement.cpp:200: } else
should be "} else if (..."

https://codereview.chromium.org/14859003/diff/3001/Source/core/html/HTMLOptio...
Source/core/html/HTMLOptionElement.cpp:278: else
should be "else if (..."

https://codereview.chromium.org/14859003/diff/3001/Source/core/html/RangeInpu...
File Source/core/html/RangeInputType.cpp (right):

https://codereview.chromium.org/14859003/diff/3001/Source/core/html/RangeInpu...
Source/core/html/RangeInputType.cpp:58: #include
"core/html/HTMLDataListElement.h"
Please merge these #include to the above block.

https://codereview.chromium.org/14859003/diff/3001/Source/core/page/RuntimeEn...
File Source/core/page/RuntimeEnabledFeatures.in (right):

https://codereview.chromium.org/14859003/diff/3001/Source/core/page/RuntimeEn...
Source/core/page/RuntimeEnabledFeatures.in:24: Datalist status=stable
Please name this DatalistElement.

https://codereview.chromium.org/14859003/diff/3001/Source/core/rendering/Rend...
File Source/core/rendering/RenderThemeChromiumDefault.cpp (right):

https://codereview.chromium.org/14859003/diff/3001/Source/core/rendering/Rend...
Source/core/rendering/RenderThemeChromiumDefault.cpp:107: static String
datalistCss = (RuntimeEnabledFeatures::datalistEnabled()) ?
RenderThemeChromiumDefault is used only in Chromium Linux.
We should have this code in RenderTheme.cpp, and existing
extraDefaultStyleSheet() overrides should be like the following:

String RenderThemeChromiumXXX::extraDefaultStyleSheet()
{
    return RenderTheme::extraDefaultStyleSheet() + ....

[tkent, 2013-05-20 21:33:35]

On 2013/05/20 21:32:11, tkent wrote:
https://codereview.chromium.org/14859003/diff/3001/Source/core/page/RuntimeEn...
> Source/core/page/RuntimeEnabledFeatures.in:24: Datalist status=stable
> Please name this DatalistElement.

should be Data*L*istElement for consistency.

[riju_, 2013-05-21 17:45:21]

incorporated tkent's comments

[tkent, 2013-05-21 21:05:44]

https://codereview.chromium.org/14859003/diff/21001/Source/core/html/ColorInp...
File Source/core/html/ColorInputType.cpp (right):

https://codereview.chromium.org/14859003/diff/21001/Source/core/html/ColorInp...
Source/core/html/ColorInputType.cpp:222: return
element()->fastHasAttribute(listAttr);
Needs runtime flag check.

https://codereview.chromium.org/14859003/diff/21001/Source/core/html/HTMLData...
File Source/core/html/HTMLDataListElement.idl (right):

https://codereview.chromium.org/14859003/diff/21001/Source/core/html/HTMLData...
Source/core/html/HTMLDataListElement.idl:32: EnabledAtRuntime=DataListElement
nit: use four space indentation

https://codereview.chromium.org/14859003/diff/21001/Source/core/rendering/Ren...
File Source/core/rendering/RenderTheme.cpp (right):

https://codereview.chromium.org/14859003/diff/21001/Source/core/rendering/Ren...
Source/core/rendering/RenderTheme.cpp:55: #include
"core/dom/shadow/ElementShadow.h"
Merge these #includes to the above block.

https://codereview.chromium.org/14859003/diff/21001/Source/core/rendering/Ren...
Source/core/rendering/RenderTheme.cpp:459: static String datalistElementCss =
(RuntimeEnabledFeatures::dataListElementEnabled) ? "datalist {display: none ;}"
: "";
Acronyms should be upper-case.  So the variable name should be
dataListElementCSS.
Also, please use ASCIILiteral notation.

https://codereview.chromium.org/14859003/diff/21001/Source/core/rendering/Ren...
Source/core/rendering/RenderTheme.cpp:461: #if defined(ENABLE_INPUT_TYPE_COLOR)
&& ENABLE_INPUT_TYPE_COLOR
#if ENABLE(INPUT_TYPE_COLOR)

https://codereview.chromium.org/14859003/diff/21001/Source/core/rendering/Ren...
Source/core/rendering/RenderTheme.cpp:462: StringBuilder inputTypeCss;
Css -> CSS

https://codereview.chromium.org/14859003/diff/21001/Source/core/rendering/Ren...
Source/core/rendering/RenderTheme.cpp:467: return datalistElementCss +
inputTypeCss.toString();
Because you already have a StringBuilder, use it.
inputTypeCSS.append(dataListElementCSS);
return inputTypeCSS.toString();

https://codereview.chromium.org/14859003/diff/21001/Source/core/rendering/Ren...
Source/core/rendering/RenderTheme.cpp:468: #endif //
defined(ENABLE_INPUT_TYPE_COLOR) && ENABLE_INPUT_TYPE_COLOR
This is a small #if block. I don't think we need a comment.

https://codereview.chromium.org/14859003/diff/21001/Source/core/rendering/Ren...
File Source/core/rendering/RenderThemeChromiumDefault.cpp (right):

https://codereview.chromium.org/14859003/diff/21001/Source/core/rendering/Ren...
Source/core/rendering/RenderThemeChromiumDefault.cpp:29: #include
"RuntimeEnabledFeatures.h"
You don't need to include this.

[riju_, 2013-05-22 00:30:34]

patch

[tkent, 2013-05-22 00:40:37]

https://codereview.chromium.org/14859003/diff/45001/Source/core/html/HTMLInpu...
File Source/core/html/HTMLInputElement.cpp (right):

https://codereview.chromium.org/14859003/diff/45001/Source/core/html/HTMLInpu...
Source/core/html/HTMLInputElement.cpp:693: }
should be "} else if (..."

https://codereview.chromium.org/14859003/diff/45001/Source/core/rendering/Ren...
File Source/core/rendering/RenderTheme.cpp (right):

https://codereview.chromium.org/14859003/diff/45001/Source/core/rendering/Ren...
Source/core/rendering/RenderTheme.cpp:461: StringBuilder inputTypeCSS;
This code block should be enabled only if the runtime flag is on.

So, the beginning of this function should be:
  if (!RuntimeEnabledFeature::dataListElementEnabled())
      return String();

[tkent, 2013-05-22 01:09:08]

https://codereview.chromium.org/14859003/diff/53001/Source/core/rendering/Ren...
File Source/core/rendering/RenderTheme.cpp (right):

https://codereview.chromium.org/14859003/diff/53001/Source/core/rendering/Ren...
Source/core/rendering/RenderTheme.cpp:460: static String dataListElementCSS =
(RuntimeEnabledFeatures::dataListElementEnabled()) ? ASCIILiteral("datalist
{display: none ;}") : ASCIILiteral("");
You don't need to check the runtime flag again.

[tkent, 2013-05-22 01:46:17]

lgtm with a minor comment

https://codereview.chromium.org/14859003/diff/57001/Source/core/rendering/Ren...
File Source/core/rendering/RenderTheme.cpp (right):

https://codereview.chromium.org/14859003/diff/57001/Source/core/rendering/Ren...
Source/core/rendering/RenderTheme.cpp:460: static String dataListElementCSS =
ASCIILiteral("datalist {display: none ;}");
Please use DEFINE_STATIC_LOCAL.

DEFINE_STATIC_LOCAL(String dataListElementCSS, (ASCIILiteral("...")));

[commit-bot: I haz the power, 2013-05-22 14:18:58]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rijubrata.bhaumik@intel.com/14859003/6...

[commit-bot: I haz the power, 2013-05-22 14:29:20]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rijubrata.bhaumik@intel.com/14859003/6...

[commit-bot: I haz the power, 2013-05-22 15:12:43]

Change committed as 150903