108651 2013-02-01 09:30:20 -0800 [WK2][Qt] Replace WebPagePageGroup usage for user scripts with WKPageGroupRef 2013-02-04 04:23:38 -0800 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 108471 1 hausmann hausmann abecsi cmarcelo menard webkit.review.bot oldest_to_newest 822867 0 hausmann 2013-02-01 09:30:20 -0800 [WK2][Qt] Replace WebPagePageGroup usage for user scripts with WKPageGroupRef 822871 1 186062 hausmann 2013-02-01 09:31:13 -0800 Created attachment 186062 Patch 822874 2 186062 kenneth 2013-02-01 09:33:18 -0800 Comment on attachment 186062 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=186062&action=review > Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp:740 > +static WKStringRef readUserScript(const QUrl& url) shouldn't you add Create/Copy or so in the method name to make it obvious that you need to adopt the result > Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp:779 > + WKRetainPtr<WKStringRef> contents = adoptWK(readUserScript(url)); > + if (WKStringIsEmpty(contents.get())) don't you need to check whehter contents is null or not? 823150 3 186062 hausmann 2013-02-01 12:48:05 -0800 Comment on attachment 186062 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=186062&action=review >> Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp:740 >> +static WKStringRef readUserScript(const QUrl& url) > > shouldn't you add Create/Copy or so in the method name to make it obvious that you need to adopt the result Not sure how important that is here. createUserScriptString? >> Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp:779 >> + if (WKStringIsEmpty(contents.get())) > > don't you need to check whehter contents is null or not? You're right, at least WKStringIsEmpty seems to crash with a null pointer, so I'm going to write it as if (!contents || WKStringIsEmpty(contents.get())) 823251 4 186062 sam 2013-02-01 14:10:44 -0800 Comment on attachment 186062 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=186062&action=review Other than the existing comments, r=me. > Source/WebKit2/ChangeLog:3 > + [WK2][Qt] Replace WebPagePageGroup usage for user scripts with WKPageGroupRef I think you meant WebPageGroup. 824238 5 hausmann 2013-02-04 04:23:38 -0800 Committed r141756: <http://trac.webkit.org/changeset/141756> 186062 2013-02-01 09:31:13 -0800 2013-02-01 14:10:42 -0800 Patch bug-108651-20130201182756.patch text/plain 5699 hausmann U3VidmVyc2lvbiBSZXZpc2lvbjogMTQxNTk4CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0Mi9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViS2l0Mi9DaGFuZ2VMb2cKaW5kZXggMDIwZjE1MzkyZWU4Yzcw YmY3MDRmYTMyZWRkYTE2NWQwYWUxYzUwOS4uMjZkMzcyMGM4MTdhYmJiYjlmZWYxNGVjMWVlZmQ4 MzgyMGQwYTE4MiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJLaXQyL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE5IEBACisyMDEzLTAyLTAxICBTaW1v biBIYXVzbWFubiAgPHNpbW9uLmhhdXNtYW5uQGRpZ2lhLmNvbT4KKworICAgICAgICBbV0syXVtR dF0gUmVwbGFjZSBXZWJQYWdlUGFnZUdyb3VwIHVzYWdlIGZvciB1c2VyIHNjcmlwdHMgd2l0aCBX S1BhZ2VHcm91cFJlZgorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5j Z2k/aWQ9MTA4NjUxCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAg ICAgICAgSXQncyBzdHJhaWdodC1mb3J3YXJkIHBvcnQgdG93YXJkcyB0aGUgQyBBUEkuCisKKyAg ICAgICAgKiBVSVByb2Nlc3MvQVBJL3F0L3FxdWlja3dlYnZpZXcuY3BwOgorICAgICAgICAoUVF1 aWNrV2ViVmlld1ByaXZhdGU6OmluaXRpYWxpemUpOgorICAgICAgICAocmVhZFVzZXJTY3JpcHQp OgorICAgICAgICAoUVF1aWNrV2ViVmlld1ByaXZhdGU6OnVwZGF0ZVVzZXJTY3JpcHRzKToKKyAg ICAgICAgKiBVSVByb2Nlc3MvQVBJL3F0L3FxdWlja3dlYnZpZXdfcF9wLmg6CisgICAgICAgIChR UXVpY2tXZWJWaWV3UHJpdmF0ZSk6CisKIDIwMTMtMDItMDEgIFNoZXJpZmYgQm90ICA8d2Via2l0 LnJldmlldy5ib3RAZ21haWwuY29tPgogCiAgICAgICAgIFVucmV2aWV3ZWQsIHJvbGxpbmcgb3V0 IHIxNDEzMTkuCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvQVBJL3F0L3Fx dWlja3dlYnZpZXcuY3BwIGIvU291cmNlL1dlYktpdDIvVUlQcm9jZXNzL0FQSS9xdC9xcXVpY2t3 ZWJ2aWV3LmNwcAppbmRleCA2ZjUzMmMyNTgyYTQxNDM5MWNhM2Y0Nzg4ODkwODE0YjgxZTY0NWU1 Li5kMWZlZWNjYjJmZjE4OGI3YjM5NjdmMWI1Y2U3MTdlZjA2YWVkYzhhIDEwMDY0NAotLS0gYS9T b3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvQVBJL3F0L3FxdWlja3dlYnZpZXcuY3BwCisrKyBiL1Nv dXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9BUEkvcXQvcXF1aWNrd2Vidmlldy5jcHAKQEAgLTY1LDcg KzY1LDkgQEAKICNpbmNsdWRlIDxRdFFtbC9RSlNWYWx1ZT4KICNpbmNsdWRlIDxRdFF1aWNrL1FR dWlja1ZpZXc+CiAjaW5jbHVkZSA8V0tPcGVuUGFuZWxSZXN1bHRMaXN0ZW5lci5oPgorI2luY2x1 ZGUgPFdLUGFnZUdyb3VwLmg+CiAjaW5jbHVkZSA8V0tTZXJpYWxpemVkU2NyaXB0VmFsdWUuaD4K KyNpbmNsdWRlIDxXS1N0cmluZy5oPgogI2luY2x1ZGUgPFdlYkNvcmUvSW50UG9pbnQuaD4KICNp bmNsdWRlIDxXZWJDb3JlL0ludFJlY3QuaD4KICNpbmNsdWRlIDxsaW1pdHM+CkBAIC0yOTUsMTQg KzI5NywxMiBAQCBRUXVpY2tXZWJWaWV3UHJpdmF0ZTo6flFRdWlja1dlYlZpZXdQcml2YXRlKCkK IC8vIE5vdGU6IHdlIGRlbGF5IHRoaXMgaW5pdGlhbGl6YXRpb24gdG8gbWFrZSBzdXJlIHRoYXQg UVF1aWNrV2ViVmlldyBoYXMgaXRzIGQtcHRyIGluLXBsYWNlLgogdm9pZCBRUXVpY2tXZWJWaWV3 UHJpdmF0ZTo6aW5pdGlhbGl6ZShXS0NvbnRleHRSZWYgY29udGV4dFJlZiwgV0tQYWdlR3JvdXBS ZWYgcGFnZUdyb3VwUmVmKQogewotICAgIFJlZlB0cjxXZWJQYWdlR3JvdXA+IHBhZ2VHcm91cDsK LSAgICBpZiAocGFnZUdyb3VwUmVmKQotICAgICAgICBwYWdlR3JvdXAgPSB0b0ltcGwocGFnZUdy b3VwUmVmKTsKLSAgICBlbHNlCi0gICAgICAgIHBhZ2VHcm91cCA9IFdlYlBhZ2VHcm91cDo6Y3Jl YXRlKCk7CisgICAgcGFnZUdyb3VwID0gcGFnZUdyb3VwUmVmOworICAgIGlmICghcGFnZUdyb3Vw KQorICAgICAgICBwYWdlR3JvdXAgPSBhZG9wdFdLKFdLUGFnZUdyb3VwQ3JlYXRlV2l0aElkZW50 aWZpZXIoMCkpOwogCiAgICAgY29udGV4dCA9IGNvbnRleHRSZWYgPyBRdFdlYkNvbnRleHQ6OmNy ZWF0ZSh0b0ltcGwoY29udGV4dFJlZikpIDogUXRXZWJDb250ZXh0OjpkZWZhdWx0Q29udGV4dCgp OwotICAgIHdlYlBhZ2VQcm94eSA9IGNvbnRleHQtPmNyZWF0ZVdlYlBhZ2UoJnBhZ2VDbGllbnQs IHBhZ2VHcm91cC5nZXQoKSk7CisgICAgd2ViUGFnZVByb3h5ID0gY29udGV4dC0+Y3JlYXRlV2Vi UGFnZSgmcGFnZUNsaWVudCwgdG9JbXBsKHBhZ2VHcm91cC5nZXQoKSkpOwogICAgIHdlYlBhZ2VQ cm94eS0+c2V0VXNlRml4ZWRMYXlvdXQoc19mbGlja2FibGVWaWV3cG9ydEVuYWJsZWQpOwogI2lm IEVOQUJMRShGVUxMU0NSRUVOX0FQSSkKICAgICB3ZWJQYWdlUHJveHktPmZ1bGxTY3JlZW5NYW5h Z2VyKCktPnNldFdlYlZpZXcocV9wdHIpOwpAQCAtNzM3LDcgKzczNyw3IEBAIHZvaWQgUVF1aWNr V2ViVmlld1ByaXZhdGU6OnNldE5hdmlnYXRvclF0T2JqZWN0RW5hYmxlZChib29sIGVuYWJsZWQp CiAgICAgd2ViUGFnZVByb3h5LT5wb3N0TWVzc2FnZVRvSW5qZWN0ZWRCdW5kbGUobWVzc2FnZU5h bWUsIHdlYkVuYWJsZWQuZ2V0KCkpOwogfQogCi1zdGF0aWMgUVN0cmluZyByZWFkVXNlclNjcmlw dChjb25zdCBRVXJsJiB1cmwpCitzdGF0aWMgV0tTdHJpbmdSZWYgcmVhZFVzZXJTY3JpcHQoY29u c3QgUVVybCYgdXJsKQogewogICAgIFFTdHJpbmcgcGF0aDsKICAgICBpZiAodXJsLmlzTG9jYWxG aWxlKCkpCkBAIC03NDYsMjggKzc0NiwyNyBAQCBzdGF0aWMgUVN0cmluZyByZWFkVXNlclNjcmlw dChjb25zdCBRVXJsJiB1cmwpCiAgICAgICAgIHBhdGggPSBRU3RyaW5nTGl0ZXJhbCgiOiIpICsg dXJsLnBhdGgoKTsKICAgICBlbHNlIHsKICAgICAgICAgcVdhcm5pbmcoIlFRdWlja1dlYlZpZXc6 IENvdWxkbid0IG9wZW4gJyVzJyBhcyB1c2VyIHNjcmlwdCBiZWNhdXNlIG9ubHkgZmlsZTovLy8g YW5kIHFyYzovLy8gVVJMcyBhcmUgc3VwcG9ydGVkLiIsIHFQcmludGFibGUodXJsLnRvU3RyaW5n KCkpKTsKLSAgICAgICAgcmV0dXJuIFFTdHJpbmcoKTsKKyAgICAgICAgcmV0dXJuIDA7CiAgICAg fQogCiAgICAgUUZpbGUgZmlsZShwYXRoKTsKICAgICBpZiAoIWZpbGUub3BlbihRSU9EZXZpY2U6 OlJlYWRPbmx5IHwgUUlPRGV2aWNlOjpUZXh0KSkgewogICAgICAgICBxV2FybmluZygiUVF1aWNr V2ViVmlldzogQ291bGRuJ3Qgb3BlbiAnJXMnIGFzIHVzZXIgc2NyaXB0IGR1ZSB0byBlcnJvciAn JXMnLiIsIHFQcmludGFibGUodXJsLnRvU3RyaW5nKCkpLCBxUHJpbnRhYmxlKGZpbGUuZXJyb3JT dHJpbmcoKSkpOwotICAgICAgICByZXR1cm4gUVN0cmluZygpOworICAgICAgICByZXR1cm4gMDsK ICAgICB9CiAKLSAgICBRU3RyaW5nIGNvbnRlbnRzID0gUVN0cmluZzo6ZnJvbVV0ZjgoZmlsZS5y ZWFkQWxsKCkpOworICAgIFFCeXRlQXJyYXkgY29udGVudHMgPSBmaWxlLnJlYWRBbGwoKTsKICAg ICBpZiAoY29udGVudHMuaXNFbXB0eSgpKQogICAgICAgICBxV2FybmluZygiUVF1aWNrV2ViVmll dzogSWdub3JpbmcgJyVzJyBhcyB1c2VyIHNjcmlwdCBiZWNhdXNlIGZpbGUgaXMgZW1wdHkuIiwg cVByaW50YWJsZSh1cmwudG9TdHJpbmcoKSkpOwogCi0gICAgcmV0dXJuIGNvbnRlbnRzOworICAg IHJldHVybiBXS1N0cmluZ0NyZWF0ZVdpdGhVVEY4Q1N0cmluZyhjb250ZW50cy5jb25zdERhdGEo KSk7CiB9CiAKIHZvaWQgUVF1aWNrV2ViVmlld1ByaXZhdGU6OnVwZGF0ZVVzZXJTY3JpcHRzKCkK IHsKICAgICAvLyBUaGlzIGZlYXR1cmUgd29ya3MgcGVyLVdlYlZpZXcgYmVjYXVzZSB3ZSBrZWVw IGFuIHVuaXF1ZSBwYWdlIGdyb3VwIGZvcgogICAgIC8vIGVhY2ggUGFnZS9XZWJWaWV3IHBhaXIg d2UgY3JlYXRlLgotICAgIFdlYlBhZ2VHcm91cCogcGFnZUdyb3VwID0gd2ViUGFnZVByb3h5LT5w YWdlR3JvdXAoKTsKLSAgICBwYWdlR3JvdXAtPnJlbW92ZUFsbFVzZXJTY3JpcHRzKCk7CisgICAg V0tQYWdlR3JvdXBSZW1vdmVBbGxVc2VyU2NyaXB0cyhwYWdlR3JvdXAuZ2V0KCkpOwogCiAgICAg Zm9yICh1bnNpZ25lZCBpID0gMDsgaSA8IHVzZXJTY3JpcHRzLnNpemUoKTsgKytpKSB7CiAgICAg ICAgIGNvbnN0IFFVcmwmIHVybCA9IHVzZXJTY3JpcHRzLmF0KGkpOwpAQCAtNzc2LDEwICs3NzUs MTAgQEAgdm9pZCBRUXVpY2tXZWJWaWV3UHJpdmF0ZTo6dXBkYXRlVXNlclNjcmlwdHMoKQogICAg ICAgICAgICAgY29udGludWU7CiAgICAgICAgIH0KIAotICAgICAgICBRU3RyaW5nIGNvbnRlbnRz ID0gcmVhZFVzZXJTY3JpcHQodXJsKTsKLSAgICAgICAgaWYgKGNvbnRlbnRzLmlzRW1wdHkoKSkK KyAgICAgICAgV0tSZXRhaW5QdHI8V0tTdHJpbmdSZWY+IGNvbnRlbnRzID0gYWRvcHRXSyhyZWFk VXNlclNjcmlwdCh1cmwpKTsKKyAgICAgICAgaWYgKFdLU3RyaW5nSXNFbXB0eShjb250ZW50cy5n ZXQoKSkpCiAgICAgICAgICAgICBjb250aW51ZTsKLSAgICAgICAgcGFnZUdyb3VwLT5hZGRVc2Vy U2NyaXB0KFN0cmluZyhjb250ZW50cyksIGVtcHR5U3RyaW5nKCksIDAsIDAsIEluamVjdEluVG9w RnJhbWVPbmx5LCBJbmplY3RBdERvY3VtZW50RW5kKTsKKyAgICAgICAgV0tQYWdlR3JvdXBBZGRV c2VyU2NyaXB0KHBhZ2VHcm91cC5nZXQoKSwgY29udGVudHMuZ2V0KCksIC8qYmFzZVVSTCovIDAs IC8qd2hpdGVsaXN0ZWRVUkxQYXR0ZXJucyovIDAsIC8qYmxhY2tsaXN0ZWRVUkxQYXR0ZXJucyov IDAsIGtXS0luamVjdEluVG9wRnJhbWVPbmx5LCBrV0tJbmplY3RBdERvY3VtZW50RW5kKTsKICAg ICB9CiB9CiAKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9BUEkvcXQvcXF1 aWNrd2Vidmlld19wX3AuaCBiL1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9BUEkvcXQvcXF1aWNr d2Vidmlld19wX3AuaAppbmRleCBhMGNhNjlhZWZjMWEyZmRmMTI1YzBlN2I0NzQxMDQyNjkyY2Jh Y2ZiLi44OGIyOGM0NzNkM2YxMjU2YWZmYzk1N2VlZDBmOGNhNjYyMDNmMmNkIDEwMDY0NAotLS0g YS9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvQVBJL3F0L3FxdWlja3dlYnZpZXdfcF9wLmgKKysr IGIvU291cmNlL1dlYktpdDIvVUlQcm9jZXNzL0FQSS9xdC9xcXVpY2t3ZWJ2aWV3X3BfcC5oCkBA IC0zMyw2ICszMyw3IEBACiAjaW5jbHVkZSA8UXRDb3JlL1FPYmplY3Q+CiAjaW5jbHVkZSA8UXRD b3JlL1FTY29wZWRQb2ludGVyPgogI2luY2x1ZGUgPFdlYkNvcmUvVmlld3BvcnRBcmd1bWVudHMu aD4KKyNpbmNsdWRlIDxXZWJLaXQyL1dLUmV0YWluUHRyLmg+CiAjaW5jbHVkZSA8d3RmL093blB0 ci5oPgogI2luY2x1ZGUgPHd0Zi9SZWZQdHIuaD4KIApAQCAtMTYxLDYgKzE2Miw3IEBAIHByb3Rl Y3RlZDoKICAgICBRUXVpY2tXZWJWaWV3UHJpdmF0ZShRUXVpY2tXZWJWaWV3KiB2aWV3cG9ydCk7 CiAgICAgUmVmUHRyPFdlYktpdDo6UXRXZWJDb250ZXh0PiBjb250ZXh0OwogICAgIFJlZlB0cjxX ZWJLaXQ6OldlYlBhZ2VQcm94eT4gd2ViUGFnZVByb3h5OworICAgIFdLUmV0YWluUHRyPFdLUGFn ZUdyb3VwUmVmPiBwYWdlR3JvdXA7CiAKICAgICBXZWJLaXQ6OlF0UGFnZUNsaWVudCBwYWdlQ2xp ZW50OwogICAgIFdlYktpdDo6RGVmYXVsdFVuZG9Db250cm9sbGVyIHVuZG9Db250cm9sbGVyOwo=