41019 2010-06-22 16:02:27 -0700 Canvas: Remember verified clean origins for drawImage() 2010-06-25 12:15:25 -0700 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) All All RESOLVED FIXED Performance P2 Normal --- 1 kling webkit-unassigned benjamin commit-queue sam oldest_to_newest 241501 0 kling 2010-06-22 16:02:27 -0700 This canvas game: http://www.watersheep.org/~markh/html_canvas/game.html ..is doing a ton of drawImage() calls, and 25% of that time is spent checking that the image URL's don't taint the canvas. Simply caching the KURL's of verified origins helps performance a lot. 241503 1 59427 kling 2010-06-22 16:04:55 -0700 Created attachment 59427 Proposed patch 241567 2 59427 darin 2010-06-22 17:41:21 -0700 Comment on attachment 59427 Proposed patch Why is this optimization important? It seems dangerous to cache security decisions because the conditions the security decision was based on could change in theory, so we should only do it if we absolutely need to. How did you determine the optimization was needed? Did this come up in some real world benchmark? Is the common code path for checkOrigin the KURL one or the String one? I ask because we could easily store strings in the hash instead of URLs and then we could do the fast check before even turning a string into a URL. It's free to turn an URL back into a string with string(), and strings hash even better than URLs do, so I think you should really consider using strings instead of URLs as the hash keys. > + ListHashSet<KURL> m_cleanOrigins; You want HashSet, not ListHashSet. ListHashSet is a more expensive and complex variant of HashSet that guarantees you can get the elements of the set back in the order you added them. review- because this should be HashSet not ListHashSet. 241587 3 59456 kling 2010-06-22 18:35:12 -0700 Created attachment 59456 Proposed patch v2 The origin of the canvas's underlying document will not change, so caching verified URL's should be perfectly safe. The KURL path is the common one, but it indeed makes more sense to cache the Strings. I've updated the patch to use HashSet<String>, thanks for the feedback. 241696 4 sam 2010-06-22 23:55:57 -0700 (In reply to comment #3) > Created an attachment (id=59456) [details] > Proposed patch v2 > > The origin of the canvas's underlying document will not change, so caching verified URL's should be perfectly safe. Are you sure? What if document.domain is set in between calls to canvas methods. That could change the tainting the rules I believe. 241890 5 kling 2010-06-23 09:43:49 -0700 (In reply to comment #4) > Are you sure? What if document.domain is set in between calls to canvas methods. That could change the tainting the rules I believe. From SecurityOrigin::canRequest(KURL): // We call isSameSchemeHostPort here instead of canAccess because we want // to ignore document.domain effects. if (isSameSchemeHostPort(targetOrigin.get())) return true; This is on the path currently taken by canvas's origin check. 243121 6 59456 commit-queue 2010-06-25 12:15:18 -0700 Comment on attachment 59456 Proposed patch v2 Clearing flags on attachment: 59456 Committed r61877: <http://trac.webkit.org/changeset/61877> 243122 7 commit-queue 2010-06-25 12:15:25 -0700 All reviewed patches have been landed. Closing bug. 59427 2010-06-22 16:04:55 -0700 2010-06-22 18:35:12 -0700 Proposed patch bug-41019.diff text/plain 2320 kling ZGlmZiAtLWdpdCBhL1dlYkNvcmUvQ2hhbmdlTG9nIGIvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXgg ZjRjNDhkNi4uYzkxMjgzMyAxMDA2NDQKLS0tIGEvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvV2Vi Q29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxNyBAQAorMjAxMC0wNi0yMiAgQW5kcmVhcyBLbGlu ZyAgPGFuZHJlYXMua2xpbmdAbm9raWEuY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9E WSAoT09QUyEpLgorCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNn aT9pZD00MTAxOQorICAgICAgICBDYW52YXM6IFJlbWVtYmVyIHZlcmlmaWVkIGNsZWFuIG9yaWdp bnMgZm9yIGRyYXdJbWFnZSgpCisKKyAgICAgICAgTWFkZSBDYW52YXNSZW5kZXJpbmdDb250ZXh0 MkQgY2FjaGUgdGhlIEtVUkxzIG9mIGNsZWFuIG9yaWdpbnMKKyAgICAgICAgZm9yIGZhc3QgcmVw ZWF0ZWQgbG9va3VwLgorCisgICAgICAgICogaHRtbC9jYW52YXMvQ2FudmFzUmVuZGVyaW5nQ29u dGV4dDJELmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OkNhbnZhc1JlbmRlcmluZ0NvbnRleHQyRDo6 Y2hlY2tPcmlnaW4pOgorICAgICAgICAqIGh0bWwvY2FudmFzL0NhbnZhc1JlbmRlcmluZ0NvbnRl eHQyRC5oOgorCiAyMDEwLTA2LTIyICBDaHJpcyBNYXJyaW4gIDxjbWFycmluQGFwcGxlLmNvbT4K IAogICAgICAgICBSZXZpZXdlZCBieSBTaW1vbiBGcmFzZXIuCmRpZmYgLS1naXQgYS9XZWJDb3Jl L2h0bWwvY2FudmFzL0NhbnZhc1JlbmRlcmluZ0NvbnRleHQyRC5jcHAgYi9XZWJDb3JlL2h0bWwv Y2FudmFzL0NhbnZhc1JlbmRlcmluZ0NvbnRleHQyRC5jcHAKaW5kZXggNTIyZTUxZC4uNTE1MjFl OSAxMDA2NDQKLS0tIGEvV2ViQ29yZS9odG1sL2NhbnZhcy9DYW52YXNSZW5kZXJpbmdDb250ZXh0 MkQuY3BwCisrKyBiL1dlYkNvcmUvaHRtbC9jYW52YXMvQ2FudmFzUmVuZGVyaW5nQ29udGV4dDJE LmNwcApAQCAtOTU4LDggKzk1OCwxMyBAQCBzdGF0aWMgaW5saW5lIEZsb2F0UmVjdCBub3JtYWxp emVSZWN0KGNvbnN0IEZsb2F0UmVjdCYgcmVjdCkKIAogdm9pZCBDYW52YXNSZW5kZXJpbmdDb250 ZXh0MkQ6OmNoZWNrT3JpZ2luKGNvbnN0IEtVUkwmIHVybCkKIHsKKyAgICBpZiAobV9jbGVhbk9y aWdpbnMuY29udGFpbnModXJsKSkKKyAgICAgICAgcmV0dXJuOworCiAgICAgaWYgKGNhbnZhcygp LT5zZWN1cml0eU9yaWdpbigpLnRhaW50c0NhbnZhcyh1cmwpKQogICAgICAgICBjYW52YXMoKS0+ c2V0T3JpZ2luVGFpbnRlZCgpOworICAgIGVsc2UKKyAgICAgICAgbV9jbGVhbk9yaWdpbnMuYWRk KHVybCk7CiB9CiAKIHZvaWQgQ2FudmFzUmVuZGVyaW5nQ29udGV4dDJEOjpjaGVja09yaWdpbihj b25zdCBTdHJpbmcmIHVybCkKZGlmZiAtLWdpdCBhL1dlYkNvcmUvaHRtbC9jYW52YXMvQ2FudmFz UmVuZGVyaW5nQ29udGV4dDJELmggYi9XZWJDb3JlL2h0bWwvY2FudmFzL0NhbnZhc1JlbmRlcmlu Z0NvbnRleHQyRC5oCmluZGV4IGQ2YjBjOGEuLjVhMjE0YjggMTAwNjQ0Ci0tLSBhL1dlYkNvcmUv aHRtbC9jYW52YXMvQ2FudmFzUmVuZGVyaW5nQ29udGV4dDJELmgKKysrIGIvV2ViQ29yZS9odG1s L2NhbnZhcy9DYW52YXNSZW5kZXJpbmdDb250ZXh0MkQuaApAQCAtMzIsOCArMzIsMTAgQEAKICNp bmNsdWRlICJGbG9hdFNpemUuaCIKICNpbmNsdWRlICJGb250LmgiCiAjaW5jbHVkZSAiR3JhcGhp Y3NUeXBlcy5oIgorI2luY2x1ZGUgIktVUkwuaCIKICNpbmNsdWRlICJQYXRoLmgiCiAjaW5jbHVk ZSAiUGxhdGZvcm1TdHJpbmcuaCIKKyNpbmNsdWRlIDx3dGYvTGlzdEhhc2hTZXQuaD4KICNpbmNs dWRlIDx3dGYvVmVjdG9yLmg+CiAKICNpZiBQTEFURk9STShDRykKQEAgLTUxLDcgKzUzLDYgQEAg bmFtZXNwYWNlIFdlYkNvcmUgewogICAgIGNsYXNzIEhUTUxJbWFnZUVsZW1lbnQ7CiAgICAgY2xh c3MgSFRNTFZpZGVvRWxlbWVudDsKICAgICBjbGFzcyBJbWFnZURhdGE7Ci0gICAgY2xhc3MgS1VS TDsKICAgICBjbGFzcyBUZXh0TWV0cmljczsKIAogICAgIHR5cGVkZWYgaW50IEV4Y2VwdGlvbkNv ZGU7CkBAIC0yNjQsNiArMjY1LDkgQEAgbmFtZXNwYWNlIFdlYkNvcmUgewogI2VuZGlmCiAgICAg ICAgIAogICAgICAgICB2b2lkIHByZXBhcmVHcmFkaWVudEZvckRhc2hib2FyZChDYW52YXNHcmFk aWVudCogZ3JhZGllbnQpIGNvbnN0OworCisgICAgICAgIExpc3RIYXNoU2V0PEtVUkw+IG1fY2xl YW5PcmlnaW5zOworCiAgICAgICAgIHZvaWQgY2hlY2tPcmlnaW4oY29uc3QgS1VSTCYpOwogICAg ICAgICB2b2lkIGNoZWNrT3JpZ2luKGNvbnN0IFN0cmluZyYpOwogCg== 59456 2010-06-22 18:35:12 -0700 2010-06-25 12:15:17 -0700 Proposed patch v2 bug-41019-v2.diff text/plain 2191 kling ZGlmZiAtLWdpdCBhL1dlYkNvcmUvQ2hhbmdlTG9nIGIvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXgg OTJhNDM5ZS4uZTU1OGVkMyAxMDA2NDQKLS0tIGEvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvV2Vi Q29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxNyBAQAorMjAxMC0wNi0yMiAgQW5kcmVhcyBLbGlu ZyAgPGFuZHJlYXMua2xpbmdAbm9raWEuY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9E WSAoT09QUyEpLgorCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNn aT9pZD00MTAxOQorICAgICAgICBDYW52YXM6IFJlbWVtYmVyIHZlcmlmaWVkIGNsZWFuIG9yaWdp bnMgZm9yIGRyYXdJbWFnZSgpCisKKyAgICAgICAgTWFkZSBDYW52YXNSZW5kZXJpbmdDb250ZXh0 MkQgY2FjaGUgdGhlIEtVUkxzIG9mIGNsZWFuIG9yaWdpbnMKKyAgICAgICAgZm9yIGZhc3QgcmVw ZWF0ZWQgbG9va3VwLgorCisgICAgICAgICogaHRtbC9jYW52YXMvQ2FudmFzUmVuZGVyaW5nQ29u dGV4dDJELmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OkNhbnZhc1JlbmRlcmluZ0NvbnRleHQyRDo6 Y2hlY2tPcmlnaW4pOgorICAgICAgICAqIGh0bWwvY2FudmFzL0NhbnZhc1JlbmRlcmluZ0NvbnRl eHQyRC5oOgorCiAyMDEwLTA2LTIyICBUb255IEdlbnRpbGNvcmUgIDx0b255Z0BjaHJvbWl1bS5v cmc+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgRXJpYyBTZWlkZWwuCmRpZmYgLS1naXQgYS9XZWJD b3JlL2h0bWwvY2FudmFzL0NhbnZhc1JlbmRlcmluZ0NvbnRleHQyRC5jcHAgYi9XZWJDb3JlL2h0 bWwvY2FudmFzL0NhbnZhc1JlbmRlcmluZ0NvbnRleHQyRC5jcHAKaW5kZXggNTIyZTUxZC4uNjI4 NGU1MCAxMDA2NDQKLS0tIGEvV2ViQ29yZS9odG1sL2NhbnZhcy9DYW52YXNSZW5kZXJpbmdDb250 ZXh0MkQuY3BwCisrKyBiL1dlYkNvcmUvaHRtbC9jYW52YXMvQ2FudmFzUmVuZGVyaW5nQ29udGV4 dDJELmNwcApAQCAtOTU4LDEyICs5NTgsMjAgQEAgc3RhdGljIGlubGluZSBGbG9hdFJlY3Qgbm9y bWFsaXplUmVjdChjb25zdCBGbG9hdFJlY3QmIHJlY3QpCiAKIHZvaWQgQ2FudmFzUmVuZGVyaW5n Q29udGV4dDJEOjpjaGVja09yaWdpbihjb25zdCBLVVJMJiB1cmwpCiB7CisgICAgaWYgKG1fY2xl YW5PcmlnaW5zLmNvbnRhaW5zKHVybC5zdHJpbmcoKSkpCisgICAgICAgIHJldHVybjsKKwogICAg IGlmIChjYW52YXMoKS0+c2VjdXJpdHlPcmlnaW4oKS50YWludHNDYW52YXModXJsKSkKICAgICAg ICAgY2FudmFzKCktPnNldE9yaWdpblRhaW50ZWQoKTsKKyAgICBlbHNlCisgICAgICAgIG1fY2xl YW5PcmlnaW5zLmFkZCh1cmwuc3RyaW5nKCkpOwogfQogCiB2b2lkIENhbnZhc1JlbmRlcmluZ0Nv bnRleHQyRDo6Y2hlY2tPcmlnaW4oY29uc3QgU3RyaW5nJiB1cmwpCiB7CisgICAgaWYgKG1fY2xl YW5PcmlnaW5zLmNvbnRhaW5zKHVybCkpCisgICAgICAgIHJldHVybjsKKwogICAgIGNoZWNrT3Jp Z2luKEtVUkwoS1VSTCgpLCB1cmwpKTsKIH0KIApkaWZmIC0tZ2l0IGEvV2ViQ29yZS9odG1sL2Nh bnZhcy9DYW52YXNSZW5kZXJpbmdDb250ZXh0MkQuaCBiL1dlYkNvcmUvaHRtbC9jYW52YXMvQ2Fu dmFzUmVuZGVyaW5nQ29udGV4dDJELmgKaW5kZXggZDZiMGM4YS4uZmNhODE4YyAxMDA2NDQKLS0t IGEvV2ViQ29yZS9odG1sL2NhbnZhcy9DYW52YXNSZW5kZXJpbmdDb250ZXh0MkQuaAorKysgYi9X ZWJDb3JlL2h0bWwvY2FudmFzL0NhbnZhc1JlbmRlcmluZ0NvbnRleHQyRC5oCkBAIC0zNCw2ICsz NCw3IEBACiAjaW5jbHVkZSAiR3JhcGhpY3NUeXBlcy5oIgogI2luY2x1ZGUgIlBhdGguaCIKICNp bmNsdWRlICJQbGF0Zm9ybVN0cmluZy5oIgorI2luY2x1ZGUgPHd0Zi90ZXh0L1N0cmluZ0hhc2gu aD4KICNpbmNsdWRlIDx3dGYvVmVjdG9yLmg+CiAKICNpZiBQTEFURk9STShDRykKQEAgLTI2NCw2 ICsyNjUsOSBAQCBuYW1lc3BhY2UgV2ViQ29yZSB7CiAjZW5kaWYKICAgICAgICAgCiAgICAgICAg IHZvaWQgcHJlcGFyZUdyYWRpZW50Rm9yRGFzaGJvYXJkKENhbnZhc0dyYWRpZW50KiBncmFkaWVu dCkgY29uc3Q7CisKKyAgICAgICAgSGFzaFNldDxTdHJpbmc+IG1fY2xlYW5PcmlnaW5zOworCiAg ICAgICAgIHZvaWQgY2hlY2tPcmlnaW4oY29uc3QgS1VSTCYpOwogICAgICAgICB2b2lkIGNoZWNr T3JpZ2luKGNvbnN0IFN0cmluZyYpOwogCg==