45157 2010-09-02 18:47:56 -0700 [Qt] V8 port: Fix "WTF::String::utf8" returns incomplete type WTF::CString 2010-09-11 13:12:14 -0700 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) All All RESOLVED FIXED Qt, QtTriaged P3 Normal --- 45136 1 vladbph webkit-qt-unassigned abarth eric hausmann kling koshuin laszlo.gombos webkit.review.bot oldest_to_newest 273725 0 vladbph 2010-09-02 18:47:56 -0700 Eliminate ambiguous inclusion of animation.h and CString.h 273733 1 66453 vladbph 2010-09-02 18:53:39 -0700 Created attachment 66453 Eliminate ambiguous inclusion of animation.h and CString.h for Symbian 273886 2 66453 kling 2010-09-03 05:02:51 -0700 Comment on attachment 66453 Eliminate ambiguous inclusion of animation.h and CString.h for Symbian > +#if PLATFORM(QT) > +#include <platform/animation/Animation.h> > +#else > #include "Animation.h" > +#endif Does this really need to be Qt-specific? > +#include <wtf/text/CString.h> Why is this necessary? CString.h is always referenced by wtf/text/CString.h AFAIK. 273907 3 laszlo.gombos 2010-09-03 06:38:18 -0700 I agree with Andreas comments. Also I think these changes are not just for V8 but perhaps generally needed to make reliable builds on Symbian with JSC. 273977 4 66453 darin 2010-09-03 09:38:14 -0700 Comment on attachment 66453 Eliminate ambiguous inclusion of animation.h and CString.h for Symbian > +#if PLATFORM(QT) > +#include <platform/animation/Animation.h> > +#else > #include "Animation.h" > +#endif This is a bad direction -- it does not seem like a good long-term solution to have platform-specific different ways of including header files. Can we instead fix the paths passed in to the compiler so that WebCore compiles will find WebCore headers first, before system headers? > +#include <wtf/text/CString.h> We don't want every single file that includes PlatformString to also include CString. Why is this a good idea? 274045 5 vladbph 2010-09-03 11:13:01 -0700 Animation.h and cstring.h files are present in Symbian system include folders as well. At the time of the port there was conflict between system includes and webkit. It is fixed now by include order... (In reply to comment #2) > (From update of attachment 66453 [details]) > > +#if PLATFORM(QT) > > +#include <platform/animation/Animation.h> > > +#else > > #include "Animation.h" > > +#endif > This one prevents v8 binding compilation errors on symbian. "WebCore\bindings\v8\V8Proxy.cpp", line 196: Error: #409: function "WTF::String::utf8" returns incomplete type "WTF::CString" targetDocument->url().string().utf8().data() ^ Otherwise I need to include #include <wtf/text/CString.h> in many v8 binding files > Does this really need to be Qt-specific? > > > +#include <wtf/text/CString.h> > > Why is this necessary? CString.h is always referenced by wtf/text/CString.h AFAIK. 274071 6 vladbph 2010-09-03 11:40:38 -0700 Tried #include <wtf/text/CString.h> in V8Proxy.h. Bindings compile fine, but generated files not :( 274323 7 vladbph 2010-09-03 18:51:26 -0700 (In reply to comment #6) RVCT complains about incomplete type: FAILED compile for armv5_urel: generated\InspectorBackendDispatcher.cpp mmp: WebCore_0x200267C2.mmp "\webkit\WebCore\generated\InspectorBackendDispatcher.cpp", line 3688: Error: #409: function "WTF::String::utf8" returns incomplete type WTF::CString" reportProtocolError(callId, String::format("Protocol Error: Invalid command was received. '%s' wasn't found.", command.utf8().data())); ^ X:\webkit.trunk1\WebCore\generated\InspectorBackendDispatcher.cpp: 0 warnings, 1 error > Tried #include <wtf/text/CString.h> in V8Proxy.h. Bindings compile fine, but generated files not :( 274326 8 66579 vladbph 2010-09-03 19:16:42 -0700 Created attachment 66579 added CString.h to QT section of WTFString.h I think I found compromise - added CString.h to QT section of WTFString.h 274463 9 kling 2010-09-05 17:55:23 -0700 (In reply to comment #6) > Tried #include <wtf/text/CString.h> in V8Proxy.h. Bindings compile fine, but generated files not :( If this is indeed the problem, it would be better to modify the code generator script(s) to include the necessary header instead of messing with WTFString.h 274499 10 66579 abarth 2010-09-05 23:24:09 -0700 Comment on attachment 66579 added CString.h to QT section of WTFString.h I agree with kling. This doesn't seem like the right place to include the header. If it were the right place, we should do so unconditionally anyway. 277754 11 67311 kling 2010-09-11 11:23:10 -0700 Created attachment 67311 Proposed patch 277755 12 67311 hausmann 2010-09-11 11:24:21 -0700 Comment on attachment 67311 Proposed patch r=me Explicit inclusion is better than implicit :) 277757 13 67311 kling 2010-09-11 11:28:58 -0700 Comment on attachment 67311 Proposed patch Clearing flags on attachment: 67311 Committed r67308: <http://trac.webkit.org/changeset/67308> 277758 14 kling 2010-09-11 11:29:08 -0700 All reviewed patches have been landed. Closing bug. 277773 15 webkit.review.bot 2010-09-11 13:12:14 -0700 http://trac.webkit.org/changeset/67308 might have broken GTK Linux 32-bit Debug 66453 2010-09-02 18:53:39 -0700 2010-09-03 19:17:19 -0700 Eliminate ambiguous inclusion of animation.h and CString.h for Symbian v8_platform.patch text/plain 1632 vladbph SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nDQo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09DQotLS0gV2ViQ29yZS9DaGFuZ2VM b2cJKHJldmlzaW9uIDY2NjkyKQ0KKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkp DQpAQCAtMSwzICsxLDE1IEBADQorMjAxMC0wOS0wMiAgVmxhZCBCdXJsaWsgIDx2b2xvZGltaXIu YnVybGlrQG5va2lhLmNvbT4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4K KworICAgICAgICBbUXRdIFY4IHBvcnQgZm9yIFFUIHBsYXRmb3JtOiBFbGltaW5hdGUgYW1iaWd1 b3VzIGluY2x1c2lvbiBvZiBhbmltYXRpb24uaCBhbmQgQ1N0cmluZy5oCisgICAgICAgIGh0dHBz Oi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD00NTE1NworCisgICAgICAgIEVsaW1p bmF0ZSBhbWJpZ3VvdXMgaW5jbHVzaW9uIG9mIGFuaW1hdGlvbi5oIGFuZCBDU3RyaW5nLmgKKwor ICAgICAgICAqIHBsYXRmb3JtL2dyYXBoaWNzL0dyYXBoaWNzTGF5ZXIuaDoKKyAgICAgICAgKiBw bGF0Zm9ybS90ZXh0L1BsYXRmb3JtU3RyaW5nLmg6CisKIDIwMTAtMDktMDIgIEFkYW0gTGFuZ2xl eSAgPGFnbEBjaHJvbWl1bS5vcmc+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgQWRhbSBCYXJ0aC4K SW5kZXg6IFdlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvR3JhcGhpY3NMYXllci5oDQo9PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09DQotLS0gV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9HcmFwaGljc0xheWVyLmgJKHJldmlz aW9uIDY2NjkyKQ0KKysrIFdlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvR3JhcGhpY3NMYXllci5o CSh3b3JraW5nIGNvcHkpDQpAQCAtMjgsNyArMjgsMTEgQEANCiAKICNpZiBVU0UoQUNDRUxFUkFU RURfQ09NUE9TSVRJTkcpCiAKKyNpZiBQTEFURk9STShRVCkKKyNpbmNsdWRlIDxwbGF0Zm9ybS9h bmltYXRpb24vQW5pbWF0aW9uLmg+CisjZWxzZQogI2luY2x1ZGUgIkFuaW1hdGlvbi5oIgorI2Vu ZGlmCiAjaW5jbHVkZSAiQ29sb3IuaCIKICNpbmNsdWRlICJGbG9hdFBvaW50LmgiCiAjaW5jbHVk ZSAiRmxvYXRQb2ludDNELmgiCkluZGV4OiBXZWJDb3JlL3BsYXRmb3JtL3RleHQvUGxhdGZvcm1T dHJpbmcuaA0KPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PQ0KLS0tIFdlYkNvcmUvcGxhdGZvcm0vdGV4dC9QbGF0Zm9ybVN0 cmluZy5oCShyZXZpc2lvbiA2NjY5MikNCisrKyBXZWJDb3JlL3BsYXRmb3JtL3RleHQvUGxhdGZv cm1TdHJpbmcuaAkod29ya2luZyBjb3B5KQ0KQEAgLTI0LDcgKzI0LDcgQEANCiAKIC8vIFRoaXMg ZmlsZSB3b3VsZCBiZSBjYWxsZWQgU3RyaW5nLmgsIGJ1dCB0aGF0IGNvbmZsaWN0cyB3aXRoIDxz dHJpbmcuaD4KIC8vIG9uIHN5c3RlbXMgd2l0aG91dCBjYXNlLXNlbnNpdGl2ZSBmaWxlIHN5c3Rl bXMuCi0KKyNpbmNsdWRlIDx3dGYvdGV4dC9DU3RyaW5nLmg+CiAjaW5jbHVkZSA8d3RmL3RleHQv V1RGU3RyaW5nLmg+CiAKIG5hbWVzcGFjZSBXZWJDb3JlIHsK 66579 2010-09-03 19:16:42 -0700 2010-09-11 11:23:10 -0700 added CString.h to QT section of WTFString.h v8_cstring.patch text/plain 1005 vladbph SW5kZXg6IEphdmFTY3JpcHRDb3JlL0NoYW5nZUxvZw0KPT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQ0KLS0tIEphdmFTY3Jp cHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gNjY3MzUpDQorKysgSmF2YVNjcmlwdENvcmUvQ2hh bmdlTG9nCSh3b3JraW5nIGNvcHkpDQpAQCAtMSwzICsxLDEyIEBADQorMjAxMC0wOS0wMyAgVmxh ZCBCdXJsaWsgIDx2b2xvZGltaXIuYnVybGlrQG5va2lhLmNvbT4KKworICAgICAgICBSZXZpZXdl ZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBbUXRdW1N5bWJpYW5dIFY4IHBvcnQgZm9y IFFUIHBsYXRmb3JtOiBGaXggIldURjo6U3RyaW5nOjp1dGY4IiByZXR1cm5zIGluY29tcGxldGUg dHlwZSBXVEY6OkNTdHJpbmcKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19i dWcuY2dpP2lkPTQ1MTU3CisKKyAgICAgICAgKiB3dGYvdGV4dC9XVEZTdHJpbmcuaDpBZGQgQ1N0 cmluZy5oCisKIDIwMTAtMDktMDMgIENzYWJhIE9zenRyb2dvbsOhYyAgPG9zc3lAd2Via2l0Lm9y Zz4KIAogICAgICAgICBSZXZpZXdlZCBieSBTaW1vbiBIYXVzbWFubi4KSW5kZXg6IEphdmFTY3Jp cHRDb3JlL3d0Zi90ZXh0L1dURlN0cmluZy5oDQo9PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09DQotLS0gSmF2YVNjcmlwdENv cmUvd3RmL3RleHQvV1RGU3RyaW5nLmgJKHJldmlzaW9uIDY2NzM1KQ0KKysrIEphdmFTY3JpcHRD b3JlL3d0Zi90ZXh0L1dURlN0cmluZy5oCSh3b3JraW5nIGNvcHkpDQpAQCAtNDAsNyArNDAsMTAg QEANCiBjbGFzcyBRU3RyaW5nOwogUVRfRU5EX05BTUVTUEFDRQogI2luY2x1ZGUgPFFEYXRhU3Ry ZWFtPgorI2lmIFVTRShWOCkKKyNpbmNsdWRlICJDU3RyaW5nLmgiCiAjZW5kaWYKKyNlbmRpZgog CiAjaWYgUExBVEZPUk0oV1gpCiBjbGFzcyB3eFN0cmluZzsK 67311 2010-09-11 11:23:10 -0700 2010-09-11 11:28:57 -0700 Proposed patch bug-45157.diff text/plain 2436 kling SW5kZXg6IFdlYkNvcmUvcGx1Z2lucy9QbHVnaW5EYXRhYmFzZS5jcHAKPT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0g V2ViQ29yZS9wbHVnaW5zL1BsdWdpbkRhdGFiYXNlLmNwcAkocmV2aXNpb24gNjczMDQpCisrKyBX ZWJDb3JlL3BsdWdpbnMvUGx1Z2luRGF0YWJhc2UuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC0zNCw2 ICszNCw3IEBACiAjaW5jbHVkZSAiRmlsZVN5c3RlbS5oIgogI2VuZGlmCiAjaW5jbHVkZSA8c3Rk bGliLmg+CisjaW5jbHVkZSA8d3RmL3RleHQvQ1N0cmluZy5oPgogCiBuYW1lc3BhY2UgV2ViQ29y ZSB7CiAKSW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hh bmdlTG9nCShyZXZpc2lvbiA2NzMwNikKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNv cHkpCkBAIC0xLDMgKzEsMTggQEAKKzIwMTAtMDktMTEgIEFuZHJlYXMgS2xpbmcgIDxhbmRyZWFz LmtsaW5nQG5va2lhLmNvbT4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4K KworICAgICAgICBbUXRdIFY4IHBvcnQ6IEZpeCAiV1RGOjpTdHJpbmc6OnV0ZjgiIHJldHVybnMg aW5jb21wbGV0ZSB0eXBlIFdURjo6Q1N0cmluZworICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0 Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NDUxNTcKKworICAgICAgICBJbmNsdWRlIENTdHJpbmcuaCB3 aGVyZSBuZWNlc3NhcnkuIENocm9taXVtIGJ1aWxkcyB3b3VsZCBnZXQgaXQgZnJvbQorICAgICAg ICBLVVJMR29vZ2xlUHJpdmF0ZS5oIHNvIHRoaXMgaXMgb25seSBhbiBpc3N1ZSBmb3IgUXQgKyBW OC4KKworICAgICAgICAqIGJpbmRpbmdzL3Y4L1Y4UHJveHkuY3BwOgorICAgICAgICAqIGluc3Bl Y3Rvci9JbnNwZWN0b3JEZWJ1Z2dlckFnZW50LmNwcDoKKyAgICAgICAgKiBpbnNwZWN0b3IvSW5z cGVjdG9yUHJvZmlsZXJBZ2VudC5jcHA6CisgICAgICAgICogcGx1Z2lucy9QbHVnaW5EYXRhYmFz ZS5jcHA6CisKIDIwMTAtMDktMDMgIFZsYWQgQnVybGlrICA8dm9sb2RpbWlyLmJ1cmxpa0Bub2tp YS5jb20+LCBTaW1vbiBIYXVzbWFubiAgPHNpbW9uLmhhdXNtYW5uQG5va2lhLmNvbT4KIAogICAg ICAgICBSZXZpZXdlZCBieSBBbmRyZWFzIEtsaW5nLgpJbmRleDogV2ViQ29yZS9iaW5kaW5ncy92 OC9WOFByb3h5LmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBXZWJDb3JlL2JpbmRpbmdzL3Y4L1Y4UHJveHku Y3BwCShyZXZpc2lvbiA2NzMwNCkKKysrIFdlYkNvcmUvYmluZGluZ3MvdjgvVjhQcm94eS5jcHAJ KHdvcmtpbmcgY29weSkKQEAgLTc3LDYgKzc3LDcgQEAKICNpbmNsdWRlIDx3dGYvU3RkTGliRXh0 cmFzLmg+CiAjaW5jbHVkZSA8d3RmL1N0cmluZ0V4dHJhcy5oPgogI2luY2x1ZGUgPHd0Zi9VbnVz ZWRQYXJhbS5oPgorI2luY2x1ZGUgPHd0Zi90ZXh0L0NTdHJpbmcuaD4KIAogbmFtZXNwYWNlIFdl YkNvcmUgewogCkluZGV4OiBXZWJDb3JlL2luc3BlY3Rvci9JbnNwZWN0b3JQcm9maWxlckFnZW50 LmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09Ci0tLSBXZWJDb3JlL2luc3BlY3Rvci9JbnNwZWN0b3JQcm9maWxlckFn ZW50LmNwcAkocmV2aXNpb24gNjczMDQpCisrKyBXZWJDb3JlL2luc3BlY3Rvci9JbnNwZWN0b3JQ cm9maWxlckFnZW50LmNwcAkod29ya2luZyBjb3B5KQpAQCAtNDMsNiArNDMsNyBAQAogI2luY2x1 ZGUgIlNjcmlwdFByb2ZpbGUuaCIKICNpbmNsdWRlICJTY3JpcHRQcm9maWxlci5oIgogI2luY2x1 ZGUgPHd0Zi9Pd25QdHIuaD4KKyNpbmNsdWRlIDx3dGYvdGV4dC9DU3RyaW5nLmg+CiAKICNpZiBV U0UoSlNDKQogI2luY2x1ZGUgIkpTRE9NV2luZG93LmgiCkluZGV4OiBXZWJDb3JlL2luc3BlY3Rv ci9JbnNwZWN0b3JEZWJ1Z2dlckFnZW50LmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBXZWJDb3JlL2luc3Bl Y3Rvci9JbnNwZWN0b3JEZWJ1Z2dlckFnZW50LmNwcAkocmV2aXNpb24gNjczMDQpCisrKyBXZWJD b3JlL2luc3BlY3Rvci9JbnNwZWN0b3JEZWJ1Z2dlckFnZW50LmNwcAkod29ya2luZyBjb3B5KQpA QCAtMzgsNiArMzgsNyBAQAogI2luY2x1ZGUgIlBsYXRmb3JtU3RyaW5nLmgiCiAjaW5jbHVkZSAi U2NyaXB0RGVidWdTZXJ2ZXIuaCIKICNpbmNsdWRlIDx3dGYvTUQ1Lmg+CisjaW5jbHVkZSA8d3Rm L3RleHQvQ1N0cmluZy5oPgogCiBuYW1lc3BhY2UgV2ViQ29yZSB7CiAK