119776 2013-08-13 16:54:00 -0700 Don't use ScriptProfiler to find canvases for instrumentation 2013-08-13 18:03:03 -0700 1 1 1 Unclassified WebKit Web Inspector 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 dino dino commit-queue esprehn+autocc graouts joepeck timothy webkit-bug-importer oldest_to_newest 916943 0 dino 2013-08-13 16:54:00 -0700 InspectorCanvasAgent::findFramesWithUninstrumentedCanvases uses a ScriptProfiler to walk the tree looking for canvas elements. This is currently not implemented in JSC, but we can do this directly with DOM methods. We're only looking for Canvas elements that have a context, so there isn't a need for this abstract walking object. 916944 1 webkit-bug-importer 2013-08-13 16:54:20 -0700 <rdar://problem/14730401> 916948 2 208690 dino 2013-08-13 17:03:19 -0700 Created attachment 208690 Patch 916960 3 208690 joepeck 2013-08-13 17:26:00 -0700 Comment on attachment 208690 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=208690&action=review r=me > Source/WebCore/ChangeLog:10 > + InspectorCanvasAgent::findFramesWithUninstrumentedCanvases uses a ScriptProfiler to walk the tree > + looking for canvas elements. This is currently not implemented in JSC, but we can do this directly > + with DOM methods. We're only looking for Canvas elements that have a context, so there isn't a need > + for this abstract walking object. So we discussed this on IRC and this doesn't take into account -webkit-canvas(…) CSS Canvases or even Shadow DOM Canvases if they existed. I don't know if the original code handled those, but it is something that we should eventually handle. I think you could add a FIXME in source code. > Source/WebCore/html/HTMLCanvasElement.h:194 > +inline bool isHTMLCanvasElement(Node* node) > +{ > + return node->hasTagName(HTMLNames::canvasTag); > +} This one shouldn't be needed, the const one should work just fine with non-const Node*'s passed in. > Source/WebCore/inspector/InspectorCanvasAgent.cpp:270 > + if (!frame->document() || frame->page() != m_pageAgent->page()) I suspect the `page != pageAgent->page` check at the end of this can be removed now that we are walking this explicit page's frame tree. > Source/WebCore/inspector/InspectorCanvasAgent.cpp:278 > + if (canvas->renderingContext()) > + m_framesWithUninstrumentedCanvases.set(frame, true); You can `break;` inside this if statement to break out of this inner for loop. 916969 4 dino 2013-08-13 18:03:03 -0700 Committed r154033: <http://trac.webkit.org/changeset/154033> 208690 2013-08-13 17:03:19 -0700 2013-08-13 17:26:00 -0700 Patch bug-119776-20130814100317.patch text/plain 6153 dino U3VidmVyc2lvbiBSZXZpc2lvbjogMTU0MDI0CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggOTUzNGJjOTMxZmM1NGVh NzgwN2NiMDQzOGEwNWJiYTcyOTk4OTk3MC4uMWRkZDMxMGQxMTAxNTUxZmI2Y2Q0YzkzNzlkZDNj NDMxODdkNDVhOSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIyIEBACisyMDEzLTA4LTEzICBEZWFu IEphY2tzb24gIDxkaW5vQGFwcGxlLmNvbT4KKworICAgICAgICA8aHR0cHM6Ly93ZWJraXQub3Jn L2IvMTE5Nzc2PiBEb24ndCB1c2UgU2NyaXB0UHJvZmlsZXIgdG8gZmluZCBjYW52YXNlcyBmb3Ig aW5zdHJ1bWVudGF0aW9uCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisK KyAgICAgICAgSW5zcGVjdG9yQ2FudmFzQWdlbnQ6OmZpbmRGcmFtZXNXaXRoVW5pbnN0cnVtZW50 ZWRDYW52YXNlcyB1c2VzIGEgU2NyaXB0UHJvZmlsZXIgdG8gd2FsayB0aGUgdHJlZQorICAgICAg ICBsb29raW5nIGZvciBjYW52YXMgZWxlbWVudHMuIFRoaXMgaXMgY3VycmVudGx5IG5vdCBpbXBs ZW1lbnRlZCBpbiBKU0MsIGJ1dCB3ZSBjYW4gZG8gdGhpcyBkaXJlY3RseQorICAgICAgICB3aXRo IERPTSBtZXRob2RzLiBXZSdyZSBvbmx5IGxvb2tpbmcgZm9yIENhbnZhcyBlbGVtZW50cyB0aGF0 IGhhdmUgYSBjb250ZXh0LCBzbyB0aGVyZSBpc24ndCBhIG5lZWQKKyAgICAgICAgZm9yIHRoaXMg YWJzdHJhY3Qgd2Fsa2luZyBvYmplY3QuCisKKyAgICAgICAgKiBodG1sL0hUTUxDYW52YXNFbGVt ZW50Lmg6IEFkZCBuZXcgaGVscGVycyB0byBjYXN0IHRvIDxjYW52YXM+LgorICAgICAgICAoV2Vi Q29yZTo6aXNIVE1MQ2FudmFzRWxlbWVudCk6CisgICAgICAgIChXZWJDb3JlOjp0b0hUTUxDYW52 YXNFbGVtZW50KToKKyAgICAgICAgKiBpbnNwZWN0b3IvSW5zcGVjdG9yQ2FudmFzQWdlbnQuY3Bw OgorICAgICAgICAoV2ViQ29yZTo6SW5zcGVjdG9yQ2FudmFzQWdlbnQ6OmZpbmRGcmFtZXNXaXRo VW5pbnN0cnVtZW50ZWRDYW52YXNlcyk6IFNpbXBseSB3YWxrCisgICAgICAgIHRoZSBmcmFtZSB0 cmVlIGFuZCB1c2UgZ2V0RWxlbWVudHNCeVRhZ05hbWUgdG8gZmluZCBjYW52YXMgZWxlbWVudHMu CisgICAgICAgICogYmluZGluZ3MvanMvYmluZGluZ3MvanMvU2NyaXB0UHJvZmlsZXIuaDogUmVt b3ZlZCB1bnVzZWQgbWV0aG9kLgorCiAyMDEzLTA4LTEzICBBbmRlcnMgQ2FybHNzb24gIDxhbmRl cnNjYUBhcHBsZS5jb20+CiAKICAgICAgICAgU3RvcCB1c2luZyBERUZJTkVfU1RBVElDX0xPQ0FM IHdpdGggUmVmUHRyCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9iaW5kaW5ncy9qcy9TY3Jp cHRQcm9maWxlci5oIGIvU291cmNlL1dlYkNvcmUvYmluZGluZ3MvanMvU2NyaXB0UHJvZmlsZXIu aAppbmRleCBhYzYyN2ExZGUzYzEzZDYxNTc4M2Q2YTZkNjlkNTZkMGY1ZTY0YjQ1Li41M2EzZTY5 YjhlZjIwZjdkNDE2ZjcyZjIxZmY5MGRkZTFiZWFhY2MxIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2Vi Q29yZS9iaW5kaW5ncy9qcy9TY3JpcHRQcm9maWxlci5oCisrKyBiL1NvdXJjZS9XZWJDb3JlL2Jp bmRpbmdzL2pzL1NjcmlwdFByb2ZpbGVyLmgKQEAgLTczLDggKzczLDYgQEAgcHVibGljOgogICAg IHN0YXRpYyBib29sIGNhdXNlc1JlY29tcGlsYXRpb24oKSB7IHJldHVybiB0cnVlOyB9CiAgICAg c3RhdGljIGJvb2wgaXNTYW1wbGluZygpIHsgcmV0dXJuIGZhbHNlOyB9CiAgICAgc3RhdGljIGJv b2wgaGFzSGVhcFByb2ZpbGVyKCkgeyByZXR1cm4gZmFsc2U7IH0KLSAgICAvLyBGSVhNRTogSW1w bGVtZW50IHRoaXMgY291bnRlciBmb3IgSlNDLiBTZWUgYnVnIDczOTM2IGZvciBtb3JlIGRldGFp bHMuCi0gICAgc3RhdGljIHZvaWQgdmlzaXROb2RlV3JhcHBlcnMoV3JhcHBlZE5vZGVWaXNpdG9y KikgeyB9CiAgICAgLy8gRklYTUU6IFN1cHBvcnQgdGhlc2UgbWV0aG9kcyBmb3IgSlNDLiBTZWUg YnVnIDkwMzU4LgogICAgIHN0YXRpYyB2b2lkIHZpc2l0RXh0ZXJuYWxTdHJpbmdzKEV4dGVybmFs U3RyaW5nVmlzaXRvciopIHsgfQogICAgIHN0YXRpYyB2b2lkIHZpc2l0RXh0ZXJuYWxBcnJheXMo RXh0ZXJuYWxBcnJheVZpc2l0b3IqKSB7IH0KZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL2h0 bWwvSFRNTENhbnZhc0VsZW1lbnQuaCBiL1NvdXJjZS9XZWJDb3JlL2h0bWwvSFRNTENhbnZhc0Vs ZW1lbnQuaAppbmRleCA2MGVhMGNkZTkwNjJjOTc4NzU1NTk0YzhkMWE4ZTRlZDU4ODI4MDBlLi5j ZGYzYTYwZjVlMmE2MWRjNjc5NzhlYzFmNGJiN2U2OTQ1YjhjNGYyIDEwMDY0NAotLS0gYS9Tb3Vy Y2UvV2ViQ29yZS9odG1sL0hUTUxDYW52YXNFbGVtZW50LmgKKysrIGIvU291cmNlL1dlYkNvcmUv aHRtbC9IVE1MQ2FudmFzRWxlbWVudC5oCkBAIC0xODgsNiArMTg4LDMxIEBAIHByaXZhdGU6CiAg ICAgbXV0YWJsZSBSZWZQdHI8SW1hZ2U+IG1fY29waWVkSW1hZ2U7IC8vIEZJWE1FOiBUaGlzIGlz IHRlbXBvcmFyeSBmb3IgcGxhdGZvcm1zIHRoYXQgaGF2ZSB0byBjb3B5IHRoZSBpbWFnZSBidWZm ZXIgdG8gcmVuZGVyIChhbmQgZm9yIENTU0NhbnZhc1ZhbHVlKS4KIH07CiAKK2lubGluZSBib29s IGlzSFRNTENhbnZhc0VsZW1lbnQoTm9kZSogbm9kZSkKK3sKKyAgICByZXR1cm4gbm9kZS0+aGFz VGFnTmFtZShIVE1MTmFtZXM6OmNhbnZhc1RhZyk7Cit9CisKK2lubGluZSBib29sIGlzSFRNTENh bnZhc0VsZW1lbnQoY29uc3QgTm9kZSogbm9kZSkKK3sKKyAgICByZXR1cm4gbm9kZS0+aGFzVGFn TmFtZShIVE1MTmFtZXM6OmNhbnZhc1RhZyk7Cit9CisKK2lubGluZSBIVE1MQ2FudmFzRWxlbWVu dCogdG9IVE1MQ2FudmFzRWxlbWVudChOb2RlKiBub2RlKQoreworICAgIEFTU0VSVF9XSVRIX1NF Q1VSSVRZX0lNUExJQ0FUSU9OKCFub2RlIHx8IGlzSFRNTENhbnZhc0VsZW1lbnQobm9kZSkpOwor ICAgIHJldHVybiBzdGF0aWNfY2FzdDxIVE1MQ2FudmFzRWxlbWVudCo+KG5vZGUpOworfQorCitp bmxpbmUgY29uc3QgSFRNTENhbnZhc0VsZW1lbnQqIHRvSFRNTENhbnZhc0VsZW1lbnQoY29uc3Qg Tm9kZSogbm9kZSkKK3sKKyAgICBBU1NFUlRfV0lUSF9TRUNVUklUWV9JTVBMSUNBVElPTighbm9k ZSB8fCBpc0hUTUxDYW52YXNFbGVtZW50KG5vZGUpKTsKKyAgICByZXR1cm4gc3RhdGljX2Nhc3Q8 Y29uc3QgSFRNTENhbnZhc0VsZW1lbnQqPihub2RlKTsKK30KKworLy8gVGhpcyB3aWxsIGNhdGNo IGFueW9uZSBkb2luZyBhbiB1bm5lY2Vzc2FyeSBjYXN0Lgordm9pZCB0b0hUTUxDYW52YXNFbGVt ZW50KGNvbnN0IEhUTUxDYW52YXNFbGVtZW50Kik7CisKIH0gLy9uYW1lc3BhY2UKIAogI2VuZGlm CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9pbnNwZWN0b3IvSW5zcGVjdG9yQ2FudmFzQWdl bnQuY3BwIGIvU291cmNlL1dlYkNvcmUvaW5zcGVjdG9yL0luc3BlY3RvckNhbnZhc0FnZW50LmNw cAppbmRleCBjZjk5NDBkMmFiMGIwNTFlNzA3NjEwNjgwZTVkZDIxMTZhZGYzZjE0Li43OTJkZDNk ODNiZmVlZDY4YTYzMGY5ZDQyNzU5M2FhM2NlZjhiZGFlIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2Vi Q29yZS9pbnNwZWN0b3IvSW5zcGVjdG9yQ2FudmFzQWdlbnQuY3BwCisrKyBiL1NvdXJjZS9XZWJD b3JlL2luc3BlY3Rvci9JbnNwZWN0b3JDYW52YXNBZ2VudC5jcHAKQEAgLTQ2LDYgKzQ2LDcgQEAK ICNpbmNsdWRlICJJbnNwZWN0b3JQYWdlQWdlbnQuaCIKICNpbmNsdWRlICJJbnNwZWN0b3JTdGF0 ZS5oIgogI2luY2x1ZGUgIkluc3RydW1lbnRpbmdBZ2VudHMuaCIKKyNpbmNsdWRlICJOb2RlTGlz dC5oIgogI2luY2x1ZGUgIlBhZ2UuaCIKICNpbmNsdWRlICJTY3JpcHRPYmplY3QuaCIKICNpbmNs dWRlICJTY3JpcHRQcm9maWxlci5oIgpAQCAtMjYzLDM1ICsyNjQsMjEgQEAgSW5qZWN0ZWRTY3Jp cHRDYW52YXNNb2R1bGUgSW5zcGVjdG9yQ2FudmFzQWdlbnQ6OmluamVjdGVkU2NyaXB0Q2FudmFz TW9kdWxlKEVycm8KIAogdm9pZCBJbnNwZWN0b3JDYW52YXNBZ2VudDo6ZmluZEZyYW1lc1dpdGhV bmluc3RydW1lbnRlZENhbnZhc2VzKCkKIHsKLSAgICBjbGFzcyBOb2RlVmlzaXRvciA6IHB1Ymxp YyBXcmFwcGVkTm9kZVZpc2l0b3IgewotICAgIHB1YmxpYzoKLSAgICAgICAgTm9kZVZpc2l0b3Io UGFnZSogcGFnZSwgRnJhbWVzV2l0aFVuaW5zdHJ1bWVudGVkQ2FudmFzZXMmIHJlc3VsdCkKLSAg ICAgICAgICAgIDogbV9wYWdlKHBhZ2UpCi0gICAgICAgICAgICAsIG1fZnJhbWVzV2l0aFVuaW5z dHJ1bWVudGVkQ2FudmFzZXMocmVzdWx0KQotICAgICAgICB7Ci0gICAgICAgIH0KLQotICAgICAg ICB2aXJ0dWFsIHZvaWQgdmlzaXROb2RlKE5vZGUqIG5vZGUpIE9WRVJSSURFCi0gICAgICAgIHsK LSAgICAgICAgICAgIGlmICghbm9kZS0+aGFzVGFnTmFtZShIVE1MTmFtZXM6OmNhbnZhc1RhZykg fHwgIW5vZGUtPmRvY3VtZW50KCkgfHwgIW5vZGUtPmRvY3VtZW50KCktPmZyYW1lKCkpCi0gICAg ICAgICAgICAgICAgcmV0dXJuOwotICAgICAgICAgICAgCi0gICAgICAgICAgICBGcmFtZSogZnJh bWUgPSBub2RlLT5kb2N1bWVudCgpLT5mcmFtZSgpOwotICAgICAgICAgICAgaWYgKGZyYW1lLT5w YWdlKCkgIT0gbV9wYWdlKQotICAgICAgICAgICAgICAgIHJldHVybjsKLQotICAgICAgICAgICAg SFRNTENhbnZhc0VsZW1lbnQqIGNhbnZhcyA9IHN0YXRpY19jYXN0PEhUTUxDYW52YXNFbGVtZW50 Kj4obm9kZSk7Ci0gICAgICAgICAgICBpZiAoY2FudmFzLT5yZW5kZXJpbmdDb250ZXh0KCkpCi0g ICAgICAgICAgICAgICAgbV9mcmFtZXNXaXRoVW5pbnN0cnVtZW50ZWRDYW52YXNlcy5zZXQoZnJh bWUsIHRydWUpOwotICAgICAgICB9CisgICAgbV9mcmFtZXNXaXRoVW5pbnN0cnVtZW50ZWRDYW52 YXNlcy5jbGVhcigpOwogCi0gICAgcHJpdmF0ZToKLSAgICAgICAgUGFnZSogbV9wYWdlOwotICAg ICAgICBGcmFtZXNXaXRoVW5pbnN0cnVtZW50ZWRDYW52YXNlcyYgbV9mcmFtZXNXaXRoVW5pbnN0 cnVtZW50ZWRDYW52YXNlczsKLSAgICB9IG5vZGVWaXNpdG9yKG1fcGFnZUFnZW50LT5wYWdlKCks IG1fZnJhbWVzV2l0aFVuaW5zdHJ1bWVudGVkQ2FudmFzZXMpOworICAgIGZvciAoRnJhbWUqIGZy YW1lID0gbV9wYWdlQWdlbnQtPnBhZ2UoKS0+bWFpbkZyYW1lKCk7IGZyYW1lOyBmcmFtZSA9IGZy YW1lLT50cmVlKCktPnRyYXZlcnNlTmV4dCgpKSB7CisgICAgICAgIGlmICghZnJhbWUtPmRvY3Vt ZW50KCkgfHwgZnJhbWUtPnBhZ2UoKSAhPSBtX3BhZ2VBZ2VudC0+cGFnZSgpKQorICAgICAgICAg ICAgY29udGludWU7CiAKLSAgICBtX2ZyYW1lc1dpdGhVbmluc3RydW1lbnRlZENhbnZhc2VzLmNs ZWFyKCk7Ci0gICAgU2NyaXB0UHJvZmlsZXI6OnZpc2l0Tm9kZVdyYXBwZXJzKCZub2RlVmlzaXRv cik7CisgICAgICAgIFJlZlB0cjxOb2RlTGlzdD4gY2FudmFzZXMgPSBmcmFtZS0+ZG9jdW1lbnQo KS0+Z2V0RWxlbWVudHNCeVRhZ05hbWUoSFRNTE5hbWVzOjpjYW52YXNUYWcubG9jYWxOYW1lKCkp OworICAgICAgICBpZiAoY2FudmFzZXMpIHsKKyAgICAgICAgICAgIGZvciAodW5zaWduZWQgaSA9 IDAsIGxlbmd0aCA9IGNhbnZhc2VzLT5sZW5ndGgoKTsgaSA8IGxlbmd0aDsgaSsrKSB7CisgICAg ICAgICAgICAgICAgSFRNTENhbnZhc0VsZW1lbnQqIGNhbnZhcyA9IHRvSFRNTENhbnZhc0VsZW1l bnQoY2FudmFzZXMtPml0ZW0oaSkpOworICAgICAgICAgICAgICAgIGlmIChjYW52YXMtPnJlbmRl cmluZ0NvbnRleHQoKSkKKyAgICAgICAgICAgICAgICAgICAgbV9mcmFtZXNXaXRoVW5pbnN0cnVt ZW50ZWRDYW52YXNlcy5zZXQoZnJhbWUsIHRydWUpOworICAgICAgICAgICAgfQorICAgICAgICB9 CisgICAgfQogCiAgICAgZm9yIChGcmFtZXNXaXRoVW5pbnN0cnVtZW50ZWRDYW52YXNlczo6aXRl cmF0b3IgaXQgPSBtX2ZyYW1lc1dpdGhVbmluc3RydW1lbnRlZENhbnZhc2VzLmJlZ2luKCk7IGl0 ICE9IG1fZnJhbWVzV2l0aFVuaW5zdHJ1bWVudGVkQ2FudmFzZXMuZW5kKCk7ICsraXQpIHsKICAg ICAgICAgU3RyaW5nIGZyYW1lSWQgPSBtX3BhZ2VBZ2VudC0+ZnJhbWVJZChpdC0+a2V5KTsK