Bug 92339

Summary: [Forms] Introduce runtime feature flags for input type datetime, datetimelocal, month, time, week
Product: WebKit Reporter: yosin
Component: FormsAssignee: yosin
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, dglazkov, fishd, jamesr, mifenton, tkent, tkent+wkapi, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 88970    
Attachments:
Description Flags
Patch 1
none
Patch 2
none
Patch 3
none
Patch 4 none

yosin
Reported 2012-07-25 23:31:41 PDT
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
Attachments
Patch 1 (22.21 KB, patch)
2012-07-26 02:15 PDT, yosin 		no flags
DetailsFormatted DiffDiff
Patch 2 (22.49 KB, patch)
2012-07-26 02:58 PDT, yosin 		no flags
DetailsFormatted DiffDiff
Patch 3 (23.30 KB, patch)
2012-07-26 03:08 PDT, yosin 		no flags
DetailsFormatted DiffDiff
Patch 4 (23.21 KB, patch)
2012-07-26 18:25 PDT, yosin 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
yosin
Comment 1 2012-07-26 02:15:36 PDT
Created attachment 154585 [details] Patch 1
WebKit Review Bot
Comment 2 2012-07-26 02:17:37 PDT
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.
Adam Barth
Comment 3 2012-07-26 02:21:10 PDT
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 4 2012-07-26 02:21:59 PDT
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.
yosin
Comment 5 2012-07-26 02:51:59 PDT
(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 6 2012-07-26 02:54:44 PDT
Comment on attachment 154585 [details] Patch 1 Please touch InputType.h to rebuild *InputType.cpp. Without it, Chromium-win bots will fail to build.
yosin
Comment 7 2012-07-26 02:58:27 PDT
Created attachment 154592 [details] Patch 2
yosin
Comment 8 2012-07-26 03:08:49 PDT
Created attachment 154597 [details] Patch 3
Adam Barth
Comment 9 2012-07-26 08:35:34 PDT
> I'll do following three steps to make RuntimeEnabledFeatures.cpp clean: That sounds great. Thanks@
yosin
Comment 10 2012-07-26 18:09:07 PDT
Comment on attachment 154597 [details] Patch 3 Could you review this patch? Thanks in advance.
Kent Tamura
Comment 11 2012-07-26 18:19:08 PDT
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.
yosin
Comment 12 2012-07-26 18:25:19 PDT
Created attachment 154804 [details] Patch 4
yosin
Comment 13 2012-07-26 18:27:29 PDT
Comment on attachment 154804 [details] Patch 4 Clearing flags on attachment: 154804 Committed r123826: <http://trac.webkit.org/changeset/123826>
yosin
Comment 14 2012-07-26 18:27:38 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.