127420 2014-01-22 02:52:36 -0800 [EFL][WK2] defaultTextEncodingName should be stored as a member variable of EwkSettings 2014-01-22 19:05:18 -0800 1 1 1 Unclassified WebKit WebKit EFL 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 jinwoo7.song jinwoo7.song bunhere cdumez commit-queue gyuyoung.kim gyuyoung.kim lucas.de.marchi ryuan.choi oldest_to_newest 970639 0 jinwoo7.song 2014-01-22 02:52:36 -0800 As the defaultTextEncodingName is returned as a local WKEinaSharedString variable, the stringshared data is de-refed by eina_stringshare_del(). To maintain the reference counter, the defaultTextEncodingName should be stored as a member variable of EwkSettings class. 970646 1 221849 jinwoo7.song 2014-01-22 03:01:32 -0800 Created attachment 221849 Patch 970651 2 ryuan.choi 2014-01-22 03:20:37 -0800 Looks fine to me. 970653 3 221849 ryuan.choi 2014-01-22 03:26:33 -0800 Comment on attachment 221849 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=221849&action=review > Source/WebKit2/UIProcess/API/efl/ewk_settings.cpp:353 > +void EwkSettings::setDefaultTextEncodingName(const char* name) Nit, we normally added methods above the ewk APIs. 970659 4 221849 gyuyoung.kim 2014-01-22 03:47:30 -0800 Comment on attachment 221849 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=221849&action=review > Source/WebKit2/UIProcess/API/efl/ewk_settings.cpp:188 > + settings->setDefaultTextEncodingName(encoding); IIRC, we have tried to use C-API in ewk directly. For example, ewk_view.cpp, ewk_context_menu.cpp and so on. Can't we adjust it to ewk_settings as well ? 970956 5 221849 jinwoo7.song 2014-01-22 17:49:26 -0800 Comment on attachment 221849 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=221849&action=review >> Source/WebKit2/UIProcess/API/efl/ewk_settings.cpp:188 >> + settings->setDefaultTextEncodingName(encoding); > > IIRC, we have tried to use C-API in ewk directly. For example, ewk_view.cpp, ewk_context_menu.cpp and so on. Can't we adjust it to ewk_settings as well ? There is a previous bug, https://bugs.webkit.org/show_bug.cgi?id=107820. I maybe follow up the bug after this one. How do you think about this? 970964 6 gyuyoung.kim 2014-01-22 18:07:56 -0800 (In reply to comment #5) > (From update of attachment 221849 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=221849&action=review > > >> Source/WebKit2/UIProcess/API/efl/ewk_settings.cpp:188 > >> + settings->setDefaultTextEncodingName(encoding); > > > > IIRC, we have tried to use C-API in ewk directly. For example, ewk_view.cpp, ewk_context_menu.cpp and so on. Can't we adjust it to ewk_settings as well ? > > There is a previous bug, https://bugs.webkit.org/show_bug.cgi?id=107820. I maybe follow up the bug after this one. > How do you think about this? I see. Let's update the bug after landing this. 970988 7 221849 commit-queue 2014-01-22 19:05:13 -0800 Comment on attachment 221849 Patch Clearing flags on attachment: 221849 Committed r162584: <http://trac.webkit.org/changeset/162584> 970989 8 commit-queue 2014-01-22 19:05:18 -0800 All reviewed patches have been landed. Closing bug. 221849 2014-01-22 03:01:32 -0800 2014-01-22 19:05:12 -0800 Patch bug-127420-20140122200127.patch text/plain 4629 jinwoo7.song U3VidmVyc2lvbiBSZXZpc2lvbjogMTYyNTExCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0Mi9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViS2l0Mi9DaGFuZ2VMb2cKaW5kZXggZmJkYjE4ZWU2ZmI1ZGFk NDRiZDg2ODA5NzE0YjU5MTBjYjU2ZDRkNC4uODk1YjJmZGJlZDM2NjRlMmZhYTY1ZWE4MjQxMWVj ZmUzZGQ4OWMzOSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJLaXQyL0NoYW5nZUxvZwpAQCAtMSw1ICsxLDI2IEBACiAyMDE0LTAxLTIyICBKaW53 b28gU29uZyAgPGppbndvbzcuc29uZ0BzYW1zdW5nLmNvbT4KIAorICAgICAgICBbRUZMXVtXSzJd IGRlZmF1bHRUZXh0RW5jb2RpbmdOYW1lIHNob3VsZCBiZSBzdG9yZWQgYXMgYSBtZW1iZXIgdmFy aWFibGUgb2YgRXdrU2V0dGluZ3MKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hv d19idWcuY2dpP2lkPTEyNzQyMAorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEp LgorCisgICAgICAgIEFzIHRoZSBkZWZhdWx0VGV4dEVuY29kaW5nTmFtZSBpcyByZXR1cm5lZCBh cyBhIGxvY2FsIFdLRWluYVNoYXJlZFN0cmluZyB2YXJpYWJsZSwgCisgICAgICAgIHRoZSBzdHJp bmdzaGFyZWQgZGF0YSBpcyBkZS1yZWZlZCBieSBlaW5hX3N0cmluZ3NoYXJlX2RlbCgpLgorICAg ICAgICBUbyBtYWludGFpbiB0aGUgcmVmZXJlbmNlIGNvdW50ZXIsIHRoZSBkZWZhdWx0VGV4dEVu Y29kaW5nTmFtZSBzaG91bGQgYmUgc3RvcmVkIAorICAgICAgICBhcyBhIG1lbWJlciB2YXJpYWJs ZSBvZiBFd2tTZXR0aW5ncyBjbGFzcy4KKworICAgICAgICAqIFVJUHJvY2Vzcy9BUEkvZWZsL2V3 a19zZXR0aW5ncy5jcHA6CisgICAgICAgIChld2tfc2V0dGluZ3NfZW5jb2RpbmdfZGV0ZWN0b3Jf ZW5hYmxlZF9nZXQpOiBSZW9yZGVyaW5nLgorICAgICAgICAoZXdrX3NldHRpbmdzX2RlZmF1bHRf dGV4dF9lbmNvZGluZ19uYW1lX3NldCk6CisgICAgICAgIChld2tfc2V0dGluZ3NfZGVmYXVsdF90 ZXh0X2VuY29kaW5nX25hbWVfZ2V0KToKKyAgICAgICAgKEV3a1NldHRpbmdzOjpzZXREZWZhdWx0 VGV4dEVuY29kaW5nTmFtZSk6CisgICAgICAgIChFd2tTZXR0aW5nczo6ZGVmYXVsdFRleHRFbmNv ZGluZ05hbWUpOgorICAgICAgICAqIFVJUHJvY2Vzcy9BUEkvZWZsL2V3a19zZXR0aW5nc19wcml2 YXRlLmg6CisgICAgICAgIChFd2tTZXR0aW5nczo6RXdrU2V0dGluZ3MpOiBBZGQgbV9kZWZhdWx0 VGV4dEVuY29kaW5nTmFtZSBhbmQgc2V0dGVyL2dldHRlciBtZXRob2RzLgorCisyMDE0LTAxLTIy ICBKaW53b28gU29uZyAgPGppbndvbzcuc29uZ0BzYW1zdW5nLmNvbT4KKwogICAgICAgICBbRUZM XVtXSzJdIFVzZSBzdGQ6OmZ1bmN0aW9uIGluIHRoZSByZXN0IG9mIEVGTCdzIFdvcmtRdWV1ZSBp bXBsZW1lbnRhdGlvbgogICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5j Z2k/aWQ9MTI3Mzk3CiAKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9BUEkv ZWZsL2V3a19zZXR0aW5ncy5jcHAgYi9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvQVBJL2VmbC9l d2tfc2V0dGluZ3MuY3BwCmluZGV4IGNhODk5ZmI0YzdlYzlkMDA2OWM4YzA1MDYxZjg2OWYzMjI1 MzQ2NTcuLjRmZTNhN2JmZjRkMTU4MzI2NGQyNzVlYTdjZTFmZTgyMWJkYTliYjkgMTAwNjQ0Ci0t LSBhL1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9BUEkvZWZsL2V3a19zZXR0aW5ncy5jcHAKKysr IGIvU291cmNlL1dlYktpdDIvVUlQcm9jZXNzL0FQSS9lZmwvZXdrX3NldHRpbmdzLmNwcApAQCAt MTc0LDI5ICsxNzQsMjcgQEAgRWluYV9Cb29sIGV3a19zZXR0aW5nc19lbmNvZGluZ19kZXRlY3Rv cl9lbmFibGVkX3NldChFd2tfU2V0dGluZ3MqIHNldHRpbmdzLCBFaW4KICAgICByZXR1cm4gdHJ1 ZTsKIH0KIAotY29uc3QgY2hhciogZXdrX3NldHRpbmdzX2RlZmF1bHRfdGV4dF9lbmNvZGluZ19u YW1lX2dldChjb25zdCBFd2tfU2V0dGluZ3MqIHNldHRpbmdzKQorRWluYV9Cb29sIGV3a19zZXR0 aW5nc19lbmNvZGluZ19kZXRlY3Rvcl9lbmFibGVkX2dldChjb25zdCBFd2tfU2V0dGluZ3MqIHNl dHRpbmdzKQogewotICAgIEVJTkFfU0FGRVRZX09OX05VTExfUkVUVVJOX1ZBTChzZXR0aW5ncywg bnVsbHB0cik7Ci0KLSAgICBXS0VpbmFTaGFyZWRTdHJpbmcgbmFtZSA9IHNldHRpbmdzLT5wcmVm ZXJlbmNlcygpLT5kZWZhdWx0VGV4dEVuY29kaW5nTmFtZSgpLnV0ZjgoKS5kYXRhKCk7CisgICAg RUlOQV9TQUZFVFlfT05fTlVMTF9SRVRVUk5fVkFMKHNldHRpbmdzLCBmYWxzZSk7CiAKLSAgICBy ZXR1cm4gbmFtZTsKKyAgICByZXR1cm4gc2V0dGluZ3MtPnByZWZlcmVuY2VzKCktPnVzZXNFbmNv ZGluZ0RldGVjdG9yKCk7CiB9CiAKIEVpbmFfQm9vbCBld2tfc2V0dGluZ3NfZGVmYXVsdF90ZXh0 X2VuY29kaW5nX25hbWVfc2V0KEV3a19TZXR0aW5ncyogc2V0dGluZ3MsIGNvbnN0IGNoYXIqIGVu Y29kaW5nKQogewogICAgIEVJTkFfU0FGRVRZX09OX05VTExfUkVUVVJOX1ZBTChzZXR0aW5ncywg ZmFsc2UpOwogCi0gICAgc2V0dGluZ3MtPnByZWZlcmVuY2VzKCktPnNldERlZmF1bHRUZXh0RW5j b2RpbmdOYW1lKFN0cmluZzo6ZnJvbVVURjgoZW5jb2RpbmcpKTsKKyAgICBzZXR0aW5ncy0+c2V0 RGVmYXVsdFRleHRFbmNvZGluZ05hbWUoZW5jb2RpbmcpOwogCiAgICAgcmV0dXJuIHRydWU7CiB9 CiAKLUVpbmFfQm9vbCBld2tfc2V0dGluZ3NfZW5jb2RpbmdfZGV0ZWN0b3JfZW5hYmxlZF9nZXQo Y29uc3QgRXdrX1NldHRpbmdzKiBzZXR0aW5ncykKK2NvbnN0IGNoYXIqIGV3a19zZXR0aW5nc19k ZWZhdWx0X3RleHRfZW5jb2RpbmdfbmFtZV9nZXQoY29uc3QgRXdrX1NldHRpbmdzKiBzZXR0aW5n cykKIHsKLSAgICBFSU5BX1NBRkVUWV9PTl9OVUxMX1JFVFVSTl9WQUwoc2V0dGluZ3MsIGZhbHNl KTsKKyAgICBFSU5BX1NBRkVUWV9PTl9OVUxMX1JFVFVSTl9WQUwoc2V0dGluZ3MsIG51bGxwdHIp OwogCi0gICAgcmV0dXJuIHNldHRpbmdzLT5wcmVmZXJlbmNlcygpLT51c2VzRW5jb2RpbmdEZXRl Y3RvcigpOworICAgIHJldHVybiBzZXR0aW5ncy0+ZGVmYXVsdFRleHRFbmNvZGluZ05hbWUoKTsK IH0KIAogRWluYV9Cb29sIGV3a19zZXR0aW5nc19wcmVmZXJyZWRfbWluaW11bV9jb250ZW50c193 aWR0aF9zZXQoRXdrX1NldHRpbmdzICpzZXR0aW5ncywgdW5zaWduZWQgd2lkdGgpCkBAIC0zNTEs MyArMzQ5LDE3IEBAIEVpbmFfQm9vbCBld2tfc2V0dGluZ3Nfc3BhdGlhbF9uYXZpZ2F0aW9uX2Vu YWJsZWRfZ2V0KGNvbnN0IEV3a19TZXR0aW5ncyogc2V0dGluCiAKICAgICByZXR1cm4gc2V0dGlu Z3MtPnByZWZlcmVuY2VzKCktPnNwYXRpYWxOYXZpZ2F0aW9uRW5hYmxlZCgpOwogfQorCit2b2lk IEV3a1NldHRpbmdzOjpzZXREZWZhdWx0VGV4dEVuY29kaW5nTmFtZShjb25zdCBjaGFyKiBuYW1l KQoreworICAgIGlmIChtX2RlZmF1bHRUZXh0RW5jb2RpbmdOYW1lID09IG5hbWUpCisgICAgICAg IHJldHVybjsKKworICAgIHByZWZlcmVuY2VzKCktPnNldERlZmF1bHRUZXh0RW5jb2RpbmdOYW1l KFN0cmluZzo6ZnJvbVVURjgobmFtZSkpOworICAgIG1fZGVmYXVsdFRleHRFbmNvZGluZ05hbWUg PSBuYW1lOworfQorCitjb25zdCBjaGFyKiBFd2tTZXR0aW5nczo6ZGVmYXVsdFRleHRFbmNvZGlu Z05hbWUoKSBjb25zdAoreworICAgIHJldHVybiBtX2RlZmF1bHRUZXh0RW5jb2RpbmdOYW1lOwor fQpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYktpdDIvVUlQcm9jZXNzL0FQSS9lZmwvZXdrX3NldHRp bmdzX3ByaXZhdGUuaCBiL1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9BUEkvZWZsL2V3a19zZXR0 aW5nc19wcml2YXRlLmgKaW5kZXggYWE0ZWY0YmU0MmNhNzQ1MzZiODkyODg4NTMyZjJkOWY2NGFl MzMwNS4uYWUxMmJhOTY3MDRlYmQ4YTc0YWFkOTgxM2RlYzdmMWRlYTkxMmQwNyAxMDA2NDQKLS0t IGEvU291cmNlL1dlYktpdDIvVUlQcm9jZXNzL0FQSS9lZmwvZXdrX3NldHRpbmdzX3ByaXZhdGUu aAorKysgYi9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvQVBJL2VmbC9ld2tfc2V0dGluZ3NfcHJp dmF0ZS5oCkBAIC00MSwxMyArNDEsMTggQEAgcHVibGljOgogICAgICAgICA6IG1fdmlldyh2aWV3 SW1wbCkKICAgICB7CiAgICAgICAgIEFTU0VSVChtX3ZpZXcpOworICAgICAgICBtX2RlZmF1bHRU ZXh0RW5jb2RpbmdOYW1lID0gcHJlZmVyZW5jZXMoKS0+ZGVmYXVsdFRleHRFbmNvZGluZ05hbWUo KS51dGY4KCkuZGF0YSgpOwogICAgIH0KIAogICAgIGNvbnN0IFdlYktpdDo6V2ViUHJlZmVyZW5j ZXMqIHByZWZlcmVuY2VzKCkgY29uc3Q7CiAgICAgV2ViS2l0OjpXZWJQcmVmZXJlbmNlcyogcHJl ZmVyZW5jZXMoKTsKIAorICAgIGNvbnN0IGNoYXIqIGRlZmF1bHRUZXh0RW5jb2RpbmdOYW1lKCkg Y29uc3Q7CisgICAgdm9pZCBzZXREZWZhdWx0VGV4dEVuY29kaW5nTmFtZShjb25zdCBjaGFyKik7 CisKIHByaXZhdGU6CiAgICAgRXdrVmlldyogbV92aWV3OworICAgIFdLRWluYVNoYXJlZFN0cmlu ZyBtX2RlZmF1bHRUZXh0RW5jb2RpbmdOYW1lOwogfTsKIAogI2VuZGlmIC8vIGV3a19zZXR0aW5n c19wcml2YXRlX2gK