96254 2012-09-10 03:06:29 -0700 [WTR] Generated source files should include config.h 2012-09-10 14:43:23 -0700 1 1 1 Unclassified WebKit WebKit2 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 96273 1 ryuan.choi ryuan.choi benjamin gyuyoung.kim webkit.review.bot oldest_to_newest 715882 0 ryuan.choi 2012-09-10 03:06:29 -0700 As http://www.webkit.org/coding/coding-style.html mentioned, all implementations should include config.h. 716024 1 163115 ryuan.choi 2012-09-10 06:38:50 -0700 Created attachment 163115 Patch 716048 2 163119 ryuan.choi 2012-09-10 07:07:11 -0700 Created attachment 163119 Patch 716392 3 benjamin 2012-09-10 12:59:44 -0700 Is this solving a real problem? You are just gonna slow down compile time for following the coding style _guidelines_. 716475 4 ryuan.choi 2012-09-10 13:53:16 -0700 (In reply to comment #3) > Is this solving a real problem? You are just gonna slow down compile time for following the coding style _guidelines_. Yes. In fact, a real problem is that WTR/Efl can not distinguish compile time options such as ENABLE(TOUCH_EVENTS) because - Derived sources does not include config.h WebKit/Efl does not use pre compiled header, so we should include config.h to determine options - CMake build scripts does not provide FEATURE DEFINES to generator. (Bug 96273 for this) After solving above issues, I want to implement missing functionality of TOUCH EVENTS in EventSenderProxyEfl.cpp Thank you. 716494 5 163119 benjamin 2012-09-10 14:03:18 -0700 Comment on attachment 163119 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=163119&action=review > In fact, a real problem is that WTR/Efl can not distinguish compile time options such as ENABLE(TOUCH_EVENTS) because > - Derived sources does not include config.h > WebKit/Efl does not use pre compiled header, > so we should include config.h to determine options > > - CMake build scripts does not provide FEATURE DEFINES to generator. (Bug 96273 for this) That's a good justification. > Tools/WebKitTestRunner/InjectedBundle/Bindings/CodeGeneratorTestRunner.pm:332 > unshift(@contents, map { "#include \"$_\"\n" } sort keys(%contentsIncludes)); > + unshift(@contents, "#include \"config.h\"\n"); > unshift(@contents, @contentsPrefix); Is this really respecting the order of #includes regarding config.h? 716504 6 163119 ryuan.choi 2012-09-10 14:18:19 -0700 Comment on attachment 163119 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=163119&action=review >> Tools/WebKitTestRunner/InjectedBundle/Bindings/CodeGeneratorTestRunner.pm:332 >> unshift(@contents, @contentsPrefix); > > Is this really respecting the order of #includes regarding config.h? Yes, this obey the order of #include rule 1, `All implementation files must #include "config.h" first. Header files should never include "config.h"`. CodeGeneratorTestRunner.pm still not follow other rules (All other headers are sorted). 716512 7 163119 benjamin 2012-09-10 14:25:30 -0700 Comment on attachment 163119 Patch > Yes, this obey the order of #include rule 1, `All implementation files must #include "config.h" first. Header files should never include "config.h"`. Oh, right! This is unshift, not shift. LGTM. 716515 8 163119 ryuan.choi 2012-09-10 14:27:42 -0700 Comment on attachment 163119 Patch Thank you. 716529 9 163119 webkit.review.bot 2012-09-10 14:43:19 -0700 Comment on attachment 163119 Patch Clearing flags on attachment: 163119 Committed r128108: <http://trac.webkit.org/changeset/128108> 716530 10 webkit.review.bot 2012-09-10 14:43:23 -0700 All reviewed patches have been landed. Closing bug. 163115 2012-09-10 06:38:50 -0700 2012-09-10 07:07:03 -0700 Patch bug-96254-20120910223826.patch text/plain 1589 ryuan.choi U3VidmVyc2lvbiBSZXZpc2lvbjogMTI4MDM3CmRpZmYgLS1naXQgYS9Ub29scy9DaGFuZ2VMb2cg Yi9Ub29scy9DaGFuZ2VMb2cKaW5kZXggZmE1ZDVhNWJkYWU3MDQwMDA2ZjdmOGVjNTk3MGQ1NDE5 OThiZDI3MC4uMjQ5Mjc4N2YxNjgxODgyNGJkNTg3YmFlY2M4M2JhNDA2MzA3Njc3ZSAxMDA2NDQK LS0tIGEvVG9vbHMvQ2hhbmdlTG9nCisrKyBiL1Rvb2xzL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE4 IEBACisyMDEyLTA5LTEwICBSeXVhbiBDaG9pICA8cnl1YW4uY2hvaUBzYW1zdW5nLmNvbT4KKwor ICAgICAgICBbV1RSXSBHZW5lcmF0ZWQgc291cmNlIGZpbGVzIHNob3VsZCBpbmNsdWRlIGNvbmZp Zy5oCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD05NjI1 NAorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIEN1cnJl bnQgY29kaW5nIHN0eWxlIGhhcyBlbmNvdXJnZWQgdG8gaW5jbHVkZSBjb25maWcuaCBhdCB0aGUg Zmlyc3QgcG9zaXRpb24sCisgICAgICAgIEJ1dCBkZXJpdmVkIHNvdXJjZXMsIGdlbmVyYXRlZCBi eSBDb2RlR2VuZXJhdG9yVGVzdFJ1bm5lci5wbSwgZG9uJ3QgZm9sbG93CisgICAgICAgIHRoaXMg cnVsZS4KKyAgICAgICAgVGhlcmVmb3JlLCB0aGlzIHBhdGNoIGFkZGVkIGl0LgorCisgICAgICAg ICogV2ViS2l0VGVzdFJ1bm5lci9JbmplY3RlZEJ1bmRsZS9CaW5kaW5ncy9Db2RlR2VuZXJhdG9y VGVzdFJ1bm5lci5wbToKKyAgICAgICAgKF9nZW5lcmF0ZUltcGxlbWVudGF0aW9uRmlsZSk6CisK IDIwMTItMDktMTAgIFNpbW9uIEhhdXNtYW5uICA8c2ltb24uaGF1c21hbm5Abm9raWEuY29tPgog CiAgICAgICAgIEZpeCBidWlsZCBvZiBXVFIgd2l0aCBRdCBvbiBXaW5kb3dzCmRpZmYgLS1naXQg YS9Ub29scy9XZWJLaXRUZXN0UnVubmVyL0luamVjdGVkQnVuZGxlL0JpbmRpbmdzL0NvZGVHZW5l cmF0b3JUZXN0UnVubmVyLnBtIGIvVG9vbHMvV2ViS2l0VGVzdFJ1bm5lci9JbmplY3RlZEJ1bmRs ZS9CaW5kaW5ncy9Db2RlR2VuZXJhdG9yVGVzdFJ1bm5lci5wbQppbmRleCAyZjVjNjZmMTA3MTM0 MDE2YjI1NjYxMjYwMWRhZDVkYjgxMjM5NTc1Li40MGQ5MmVhZDdhOTQ2M2M5NmM3NzUyZWNmZGFh NmU5NGExYjM1MjgzIDEwMDY0NAotLS0gYS9Ub29scy9XZWJLaXRUZXN0UnVubmVyL0luamVjdGVk QnVuZGxlL0JpbmRpbmdzL0NvZGVHZW5lcmF0b3JUZXN0UnVubmVyLnBtCisrKyBiL1Rvb2xzL1dl YktpdFRlc3RSdW5uZXIvSW5qZWN0ZWRCdW5kbGUvQmluZGluZ3MvQ29kZUdlbmVyYXRvclRlc3RS dW5uZXIucG0KQEAgLTMyOCw2ICszMjgsNyBAQCBFT0YKIEVPRgogCiAgICAgdW5zaGlmdChAY29u dGVudHMsIG1hcCB7ICIjaW5jbHVkZSBcIiRfXCJcbiIgfSBzb3J0IGtleXMoJWNvbnRlbnRzSW5j bHVkZXMpKTsKKyAgICB1bnNoaWZ0KEBjb250ZW50cywgIiNpbmNsdWRlIFwiY29uZmlnLmhcIlxu Iik7CiAgICAgdW5zaGlmdChAY29udGVudHMsIEBjb250ZW50c1ByZWZpeCk7CiAKICAgICByZXR1 cm4geyBuYW1lID0+ICRmaWxlbmFtZSwgY29udGVudHMgPT4gXEBjb250ZW50cyB9Owo= 163119 2012-09-10 07:07:11 -0700 2012-09-10 14:43:19 -0700 Patch bug-96254-20120910230646.patch text/plain 1612 ryuan.choi U3VidmVyc2lvbiBSZXZpc2lvbjogMTI4MDU1CmRpZmYgLS1naXQgYS9Ub29scy9DaGFuZ2VMb2cg Yi9Ub29scy9DaGFuZ2VMb2cKaW5kZXggNTdlM2M0YTAyODQ4YzY1YjVlOGZlYjUwMWYwZGU1NTlh ODA3MzhkZC4uZTBjNjk4NDg1NjRmZTI4NGExYTNlMzRhYWM2Y2NlZDdmNThkYWIxOSAxMDA2NDQK LS0tIGEvVG9vbHMvQ2hhbmdlTG9nCisrKyBiL1Rvb2xzL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE4 IEBACisyMDEyLTA5LTEwICBSeXVhbiBDaG9pICA8cnl1YW4uY2hvaUBzYW1zdW5nLmNvbT4KKwor ICAgICAgICBbV1RSXSBHZW5lcmF0ZWQgc291cmNlIGZpbGVzIHNob3VsZCBpbmNsdWRlIGNvbmZp Zy5oCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD05NjI1 NAorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIEN1cnJl bnQgY29kaW5nIHN0eWxlIGhhcyBlbmNvdXJhZ2VkIHRvIGluY2x1ZGUgY29uZmlnLmggYXQgdGhl IGZpcnN0IHBvc2l0aW9uLAorICAgICAgICBCdXQgZGVyaXZlZCBzb3VyY2VzLCBnZW5lcmF0ZWQg YnkgQ29kZUdlbmVyYXRvclRlc3RSdW5uZXIucG0sIGRvbid0IGZvbGxvdworICAgICAgICB0aGlz IHJ1bGUuCisgICAgICAgIFRoZXJlZm9yZSwgdGhpcyBwYXRjaCBhZGRlZCBpdC4KKworICAgICAg ICAqIFdlYktpdFRlc3RSdW5uZXIvSW5qZWN0ZWRCdW5kbGUvQmluZGluZ3MvQ29kZUdlbmVyYXRv clRlc3RSdW5uZXIucG06CisgICAgICAgIChfZ2VuZXJhdGVJbXBsZW1lbnRhdGlvbkZpbGUpOgor CiAyMDEyLTA5LTEwICBDaHJpc3RvcGhlIER1bWV6ICA8Y2hyaXN0b3BoZS5kdW1lekBpbnRlbC5j b20+CiAKICAgICAgICAgW0VGTF1bRFJUXSBFbmNvZGluZyBpc3N1ZSBpbiBwcmludGVkIGNvbnNv bGUgbWVzc2FnZXMKZGlmZiAtLWdpdCBhL1Rvb2xzL1dlYktpdFRlc3RSdW5uZXIvSW5qZWN0ZWRC dW5kbGUvQmluZGluZ3MvQ29kZUdlbmVyYXRvclRlc3RSdW5uZXIucG0gYi9Ub29scy9XZWJLaXRU ZXN0UnVubmVyL0luamVjdGVkQnVuZGxlL0JpbmRpbmdzL0NvZGVHZW5lcmF0b3JUZXN0UnVubmVy LnBtCmluZGV4IDJmNWM2NmYxMDcxMzQwMTZiMjU2NjEyNjAxZGFkNWRiODEyMzk1NzUuLjQwZDky ZWFkN2E5NDYzYzk2Yzc3NTJlY2ZkYWE2ZTk0YTFiMzUyODMgMTAwNjQ0Ci0tLSBhL1Rvb2xzL1dl YktpdFRlc3RSdW5uZXIvSW5qZWN0ZWRCdW5kbGUvQmluZGluZ3MvQ29kZUdlbmVyYXRvclRlc3RS dW5uZXIucG0KKysrIGIvVG9vbHMvV2ViS2l0VGVzdFJ1bm5lci9JbmplY3RlZEJ1bmRsZS9CaW5k aW5ncy9Db2RlR2VuZXJhdG9yVGVzdFJ1bm5lci5wbQpAQCAtMzI4LDYgKzMyOCw3IEBAIEVPRgog RU9GCiAKICAgICB1bnNoaWZ0KEBjb250ZW50cywgbWFwIHsgIiNpbmNsdWRlIFwiJF9cIlxuIiB9 IHNvcnQga2V5cyglY29udGVudHNJbmNsdWRlcykpOworICAgIHVuc2hpZnQoQGNvbnRlbnRzLCAi I2luY2x1ZGUgXCJjb25maWcuaFwiXG4iKTsKICAgICB1bnNoaWZ0KEBjb250ZW50cywgQGNvbnRl bnRzUHJlZml4KTsKIAogICAgIHJldHVybiB7IG5hbWUgPT4gJGZpbGVuYW1lLCBjb250ZW50cyA9 PiBcQGNvbnRlbnRzIH07Cg==