Bug 92339 – [Forms] Introduce runtime feature flags for input type datetime, datetimelocal, month, time, week

• Status: RESOLVED
• Resolution: FIXED
• Component: Forms

Using runtime feature flags brings us following benefit:
 1. Detecting compilation failure on each local build on Chromium developer
 2. Detecting behavior change via layout tests

Created attachment 154585 [details]
Patch 1

[WebKit Review Bot]

Please wait for approval from [e-mail], [e-mail], [e-mail], [e-mail] or [e-mail] before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.

[Adam Barth]

Comment on attachment 154585 [details]
Patch 1

WebKit API change LGTM.  Please be sure to disable these features appropriately on chromium-android.

[Adam Barth]

Comment on attachment 154585 [details]
Patch 1

View in context: https://bugs.webkit.org/attachment.cgi?id=154585&action=review

> Source/WebCore/bindings/generic/RuntimeEnabledFeatures.cpp:202
> +#if OS(ANDROID)

Is this the best way to do this?  I would have expected the embedder to do this work.

(In reply to comment #4)
> (From update of attachment 154585 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=154585&action=review
> 
> > Source/WebCore/bindings/generic/RuntimeEnabledFeatures.cpp:202
> > +#if OS(ANDROID)
> 
> Is this the best way to do this?  I would have expected the embedder to do this work.

No, this isn't beautiful. We should not have this kind of #if in RuntimeEnabledFeatures.cpp

I'll do following three steps to make RuntimeEnabledFeatures.cpp clean:

1. RuntimEnabledFeatures.cpp: Change #if OS(ANDROID) to #if PLATFORM(CHROMIUM) && !OS(ANDROID) to set false, otherwise set true -- keep current behavior
2. content/renderrer/render_thread_impl.cc: RenderThreadImpl::EnsureWebKitInitialized() of Chromium side, I'll add enableInputTypeDateTime(false), ... for !OS(ANDROID)
3. Then, remove #if PLATFORM(CHROMIUM) && !OS(ANDROID) from RuntimeEnabledFeatures.cpp

[Kent Tamura]

Comment on attachment 154585 [details]
Patch 1

Please touch InputType.h to rebuild *InputType.cpp.  Without it, Chromium-win bots will fail to build.

Created attachment 154592 [details]
Patch 2

Created attachment 154597 [details]
Patch 3

[Adam Barth]

> I'll do following three steps to make RuntimeEnabledFeatures.cpp clean:

That sounds great.  Thanks@

Comment on attachment 154597 [details]
Patch 3

Could you review this patch?
Thanks in advance.

[Kent Tamura]

Comment on attachment 154597 [details]
Patch 3

View in context: https://bugs.webkit.org/attachment.cgi?id=154597&action=review

The code looks ok.

> Source/WebCore/ChangeLog:3
> +        [Forms] Introduce runtime feature flags for input type datetime, datetimelocal, month, time, week for Chromium port

Flags will be introduced to WebCore.  "for Chromium port" is false.

Created attachment 154804 [details]
Patch 4

Comment on attachment 154804 [details]
Patch 4

Clearing flags on attachment: 154804

Committed r123826: <http://trac.webkit.org/changeset/123826>

All reviewed patches have been landed.  Closing bug.