105844 2012-12-28 19:20:52 -0800 [EFL][WK2] MiniBrowser could not be launched on specific machine 2012-12-30 17:48:12 -0800 1 1 1 Unclassified WebKit WebKit EFL 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 kangil.han ryuan.choi cdumez gyuyoung.kim lucas.de.marchi rakuco ryuan.choi tmpsantos webkit.review.bot oldest_to_newest 798025 0 kangil.han 2012-12-28 19:20:52 -0800 Launching MibiBrowser doesn't work on my machine because executable path of WebProcess is NULL in child process. 798030 1 180910 ryuan.choi 2012-12-28 20:26:14 -0800 Created attachment 180910 suggestion 798033 2 kangil.han 2012-12-28 20:55:31 -0800 FOA, MiniBrowser works with your suggestion, thanks. :) Would you please mention in ChangeLog why we should use CString instead of const char* in detail? Please name an attachment as a patch as well. :) 798046 3 180910 gyuyoung.kim 2012-12-28 21:20:02 -0800 Comment on attachment 180910 suggestion As Kangil said, it would be good if you say why we need to use CString instead of const char*. However, I think we have to use CString inside WebKit. 798052 4 180910 webkit.review.bot 2012-12-28 21:31:51 -0800 Comment on attachment 180910 suggestion Clearing flags on attachment: 180910 Committed r138557: <http://trac.webkit.org/changeset/138557> 798053 5 webkit.review.bot 2012-12-28 21:31:55 -0800 All reviewed patches have been landed. Closing bug. 798213 6 180910 tmpsantos 2012-12-30 04:46:26 -0800 Comment on attachment 180910 suggestion View in context: https://bugs.webkit.org/attachment.cgi?id=180910&action=review > Source/WebKit2/UIProcess/Launcher/efl/ProcessLauncherEfl.cpp:48 > - executablePath = executablePathOfWebProcess().utf8().data(); > + executablePath = executablePathOfWebProcess().utf8(); Now I see the issue here. A better fix would be make executablePathOfWebProcess() return a const reference instead of a copy, since it is getting static data. 798214 7 180910 cdumez 2012-12-30 04:49:30 -0800 Comment on attachment 180910 suggestion View in context: https://bugs.webkit.org/attachment.cgi?id=180910&action=review >> Source/WebKit2/UIProcess/Launcher/efl/ProcessLauncherEfl.cpp:48 >> + executablePath = executablePathOfWebProcess().utf8(); > > Now I see the issue here. A better fix would be make executablePathOfWebProcess() return a const reference instead of a copy, since it is getting static data. Thiago, the issue is that String::utf8() returns a new temporary CString object and CString::data() returns a pointer to the internal CString data representation. Since the CString is temporary, the pointer returned by data() becomes invalid on next line. 798219 8 180910 tmpsantos 2012-12-30 05:15:46 -0800 Comment on attachment 180910 suggestion View in context: https://bugs.webkit.org/attachment.cgi?id=180910&action=review >>> Source/WebKit2/UIProcess/Launcher/efl/ProcessLauncherEfl.cpp:48 >>> + executablePath = executablePathOfWebProcess().utf8(); >> >> Now I see the issue here. A better fix would be make executablePathOfWebProcess() return a const reference instead of a copy, since it is getting static data. > > Thiago, the issue is that String::utf8() returns a new temporary CString object and CString::data() returns a pointer to the internal CString data representation. Since the CString is temporary, the pointer returned by data() becomes invalid on next line. Now I see, this patch is fine. Sorry for introducing this bug and thanks guys for fixing. 798280 9 kangil.han 2012-12-30 17:48:12 -0800 (In reply to comment #7) > Thiago, the issue is that String::utf8() returns a new temporary CString object and CString::data() returns a pointer to the internal CString data representation. Since the CString is temporary, the pointer returned by data() becomes invalid on next line. Yes, right! thanks for explanation Chris! :) (In reply to comment #8) > Now I see, this patch is fine. Sorry for introducing this bug and thanks guys for fixing. No problem! :) 180910 2012-12-28 20:26:14 -0800 2012-12-30 05:15:46 -0800 suggestion bug-105844-20121229132324.patch text/plain 2636 ryuan.choi U3VidmVyc2lvbiBSZXZpc2lvbjogMTM4NTU1CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0Mi9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViS2l0Mi9DaGFuZ2VMb2cKaW5kZXggMDJjNjc4MGNmNWFjMDFh ZGZhZDZiZGMwN2EyMDk4Mzg2ZDk3MjMxYy4uMDcxMTAyZjJkM2U3ODY0NDg1YjE3MGQyZjdlZDcy NTAwMjVjZGI4NyAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJLaXQyL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE0IEBACisyMDEyLTEyLTI4ICBSeXVh biBDaG9pICA8cnl1YW4uY2hvaUBzYW1zdW5nLmNvbT4KKworICAgICAgICBbRUZMXVtXSzJdIE1p bmlCcm93c2VyIGNvdWxkIG5vdCBiZSBsYXVuY2hlZCBvbiBzcGVjaWZpYyBtYWNoaW5lCisgICAg ICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xMDU4NDQKKworICAg ICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIFVJUHJvY2Vzcy9M YXVuY2hlci9lZmwvUHJvY2Vzc0xhdW5jaGVyRWZsLmNwcDoKKyAgICAgICAgKFdlYktpdDo6UHJv Y2Vzc0xhdW5jaGVyOjpsYXVuY2hQcm9jZXNzKToKKyAgICAgICAgTW9kaWZpZWQgdG8gYXZvaWQg YmFkIHVzZSBvZiB0ZW1wb3Jhcnkgb2JqZWN0LgorCiAyMDEyLTEyLTI4ICBKb29uZSBIdXIgIDxq b29uZS5odXJAaW50ZWwuY29tPgogCiAgICAgICAgIFtFRkxdW1dLMl0gSW1wbGVtZW50IGV3a19j b250ZXh0X3Jlc291cmNlX2NhY2hlX2NsZWFyCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0Mi9V SVByb2Nlc3MvTGF1bmNoZXIvZWZsL1Byb2Nlc3NMYXVuY2hlckVmbC5jcHAgYi9Tb3VyY2UvV2Vi S2l0Mi9VSVByb2Nlc3MvTGF1bmNoZXIvZWZsL1Byb2Nlc3NMYXVuY2hlckVmbC5jcHAKaW5kZXgg MjkyMGYwYmQ2NjE3NWM3Yzc3MjU0NjI2OWI2M2I0ZWYzYzIxYTg1My4uMmIzMDg5NmI1ZDY0NGIy NTEzMjE2YWQyZTVkZjc4YjZmYjA5NzMwYyAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdDIvVUlQ cm9jZXNzL0xhdW5jaGVyL2VmbC9Qcm9jZXNzTGF1bmNoZXJFZmwuY3BwCisrKyBiL1NvdXJjZS9X ZWJLaXQyL1VJUHJvY2Vzcy9MYXVuY2hlci9lZmwvUHJvY2Vzc0xhdW5jaGVyRWZsLmNwcApAQCAt NDIsMTQgKzQyLDE0IEBAIHZvaWQgUHJvY2Vzc0xhdW5jaGVyOjpsYXVuY2hQcm9jZXNzKCkKICAg ICAgICAgcmV0dXJuOwogICAgIH0KIAotICAgIGNvbnN0IGNoYXIqIGV4ZWN1dGFibGVQYXRoID0g MDsKKyAgICBDU3RyaW5nIGV4ZWN1dGFibGVQYXRoOwogICAgIHN3aXRjaCAobV9sYXVuY2hPcHRp b25zLnByb2Nlc3NUeXBlKSB7CiAgICAgY2FzZSBXZWJQcm9jZXNzOgotICAgICAgICBleGVjdXRh YmxlUGF0aCA9IGV4ZWN1dGFibGVQYXRoT2ZXZWJQcm9jZXNzKCkudXRmOCgpLmRhdGEoKTsKKyAg ICAgICAgZXhlY3V0YWJsZVBhdGggPSBleGVjdXRhYmxlUGF0aE9mV2ViUHJvY2VzcygpLnV0Zjgo KTsKICAgICAgICAgYnJlYWs7CiAjaWYgRU5BQkxFKFBMVUdJTl9QUk9DRVNTKQogICAgIGNhc2Ug UGx1Z2luUHJvY2VzczoKLSAgICAgICAgZXhlY3V0YWJsZVBhdGggPSBleGVjdXRhYmxlUGF0aE9m UGx1Z2luUHJvY2VzcygpLnV0ZjgoKS5kYXRhKCk7CisgICAgICAgIGV4ZWN1dGFibGVQYXRoID0g ZXhlY3V0YWJsZVBhdGhPZlBsdWdpblByb2Nlc3MoKS51dGY4KCk7CiAgICAgICAgIGJyZWFrOwog I2VuZGlmCiAgICAgZGVmYXVsdDoKQEAgLTY0LDcgKzY0LDcgQEAgdm9pZCBQcm9jZXNzTGF1bmNo ZXI6OmxhdW5jaFByb2Nlc3MoKQogICAgIGNvbnN0IGNoYXIqIHByZWZpeGVkRXhlY3V0YWJsZVBh dGggPSAwOwogICAgIFN0cmluZyBwcmVmaXhlZEV4ZWN1dGFibGVQYXRoU3RyOwogICAgIGlmICgh bV9sYXVuY2hPcHRpb25zLnByb2Nlc3NDbWRQcmVmaXguaXNFbXB0eSgpKSB7Ci0gICAgICAgIHBy ZWZpeGVkRXhlY3V0YWJsZVBhdGhTdHIgPSBtX2xhdW5jaE9wdGlvbnMucHJvY2Vzc0NtZFByZWZp eCArICcgJyArIGV4ZWN1dGFibGVQYXRoICsgJyAnICsgc29ja2V0OworICAgICAgICBwcmVmaXhl ZEV4ZWN1dGFibGVQYXRoU3RyID0gbV9sYXVuY2hPcHRpb25zLnByb2Nlc3NDbWRQcmVmaXggKyAn ICcgKyBleGVjdXRhYmxlUGF0aC5kYXRhKCkgKyAnICcgKyBzb2NrZXQ7CiAgICAgICAgIHByZWZp eGVkRXhlY3V0YWJsZVBhdGggPSBwcmVmaXhlZEV4ZWN1dGFibGVQYXRoU3RyLnV0ZjgoKS5kYXRh KCk7CiAgICAgfQogI2VuZGlmCkBAIC04Nyw3ICs4Nyw3IEBAIHZvaWQgUHJvY2Vzc0xhdW5jaGVy OjpsYXVuY2hQcm9jZXNzKCkKICAgICAgICAgICAgICAgICBleGl0KEVYSVRfU1VDQ0VTUyk7CiAg ICAgICAgIH0KICNlbmRpZgotICAgICAgICBleGVjbChleGVjdXRhYmxlUGF0aCwgZXhlY3V0YWJs ZVBhdGgsIHNvY2tldCwgc3RhdGljX2Nhc3Q8Y2hhcio+KDApKTsKKyAgICAgICAgZXhlY2woZXhl Y3V0YWJsZVBhdGguZGF0YSgpLCBleGVjdXRhYmxlUGF0aC5kYXRhKCksIHNvY2tldCwgc3RhdGlj X2Nhc3Q8Y2hhcio+KDApKTsKICAgICB9IGVsc2UgaWYgKHBpZCA+IDApIHsgLy8gcGFyZW50IHBy b2Nlc3M7CiAgICAgICAgIGNsb3NlKHNvY2tldHNbMF0pOwogICAgICAgICBtX3Byb2Nlc3NJZGVu dGlmaWVyID0gcGlkOwo=