115498 2013-05-01 16:24:12 -0700 Null check plugInClient earlier in snapshotting path 2013-05-01 17:43:25 -0700 1 1 1 Unclassified WebKit Plug-ins 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 joepeck joepeck commit-queue dino esprehn+autocc thorton oldest_to_newest 883584 0 joepeck 2013-05-01 16:24:12 -0700 If autostartOriginPlugInSnapshottingEnabled is enabled and there is no pluginClient snapshotting crashes. Add a null check to avoid a crash. Later on in the function there is a null check for this pluginClient as well. 883587 1 200252 joepeck 2013-05-01 16:27:25 -0700 Created attachment 200252 [PATCH] Proposed Fix I'm not very familiar with this path, but this crash certainly seems possible (a port that doesn't set a plugin in client but has snapshotting enabled because the Setting is enabled by default). We should be resilient in such cases and not crash. Maybe it makes sense to ASSERT at some other point that a plugin client is set. 883608 2 200252 darin 2013-05-01 17:10:31 -0700 Comment on attachment 200252 [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=200252&action=review > Source/WebCore/html/HTMLPlugInImageElement.cpp:648 > - if (document()->page()->settings()->autostartOriginPlugInSnapshottingEnabled() && document()->page()->plugInClient()->shouldAutoStartFromOrigin(document()->page()->mainFrame()->document()->baseURL().host(), url.host(), loadedMimeType())) { > + if (document()->page()->settings()->autostartOriginPlugInSnapshottingEnabled() && document()->page()->plugInClient() && document()->page()->plugInClient()->shouldAutoStartFromOrigin(document()->page()->mainFrame()->document()->baseURL().host(), url.host(), loadedMimeType())) { I think this would be way better with some local variables. One for document()->page() and another for document()->page()->plugInClient(). 883683 3 200252 commit-queue 2013-05-01 17:39:26 -0700 Comment on attachment 200252 [PATCH] Proposed Fix Clearing flags on attachment: 200252 Committed r149469: <http://trac.webkit.org/changeset/149469> 883684 4 commit-queue 2013-05-01 17:39:28 -0700 All reviewed patches have been landed. Closing bug. 200252 2013-05-01 16:27:25 -0700 2013-05-01 17:39:26 -0700 [PATCH] Proposed Fix proposed-fix-snapshot-1.patch text/plain 1749 joepeck ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCBjMGU1YzMzLi5kOWJkYmU5IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29y ZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTMg QEAKKzIwMTMtMDUtMDEgIEpvc2VwaCBQZWNvcmFybyAgPHBlY29yYXJvQGFwcGxlLmNvbT4KKwor ICAgICAgICBOdWxsIGNoZWNrIHBsdWdJbkNsaWVudCBlYXJsaWVyIGluIHNuYXBzaG90dGluZyBw YXRoCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xMTU0 OTgKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIGh0 bWwvSFRNTFBsdWdJbkltYWdlRWxlbWVudC5jcHA6CisgICAgICAgIChXZWJDb3JlOjpIVE1MUGx1 Z0luSW1hZ2VFbGVtZW50OjpzdWJmcmFtZUxvYWRlcldpbGxDcmVhdGVQbHVnSW4pOgorCiAyMDEz LTA0LTMwICBKZXIgTm9ibGUgIDxqZXIubm9ibGVAYXBwbGUuY29tPgogCiAgICAgICAgIENsb3Nl ZCBjYXB0aW9uIGxpbmVzIG92ZXJsYXAKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL2h0bWwv SFRNTFBsdWdJbkltYWdlRWxlbWVudC5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9odG1sL0hUTUxQbHVn SW5JbWFnZUVsZW1lbnQuY3BwCmluZGV4IDMwZTQ2YmYuLjhlOTkzMmUgMTAwNjQ0Ci0tLSBhL1Nv dXJjZS9XZWJDb3JlL2h0bWwvSFRNTFBsdWdJbkltYWdlRWxlbWVudC5jcHAKKysrIGIvU291cmNl L1dlYkNvcmUvaHRtbC9IVE1MUGx1Z0luSW1hZ2VFbGVtZW50LmNwcApAQCAtNjQ1LDcgKzY0NSw3 IEBAIHZvaWQgSFRNTFBsdWdJbkltYWdlRWxlbWVudDo6c3ViZnJhbWVMb2FkZXJXaWxsQ3JlYXRl UGx1Z0luKGNvbnN0IEtVUkwmIHVybCkKICAgICAgICAgcmV0dXJuOwogICAgIH0KIAotICAgIGlm IChkb2N1bWVudCgpLT5wYWdlKCktPnNldHRpbmdzKCktPmF1dG9zdGFydE9yaWdpblBsdWdJblNu YXBzaG90dGluZ0VuYWJsZWQoKSAmJiBkb2N1bWVudCgpLT5wYWdlKCktPnBsdWdJbkNsaWVudCgp LT5zaG91bGRBdXRvU3RhcnRGcm9tT3JpZ2luKGRvY3VtZW50KCktPnBhZ2UoKS0+bWFpbkZyYW1l KCktPmRvY3VtZW50KCktPmJhc2VVUkwoKS5ob3N0KCksIHVybC5ob3N0KCksIGxvYWRlZE1pbWVU eXBlKCkpKSB7CisgICAgaWYgKGRvY3VtZW50KCktPnBhZ2UoKS0+c2V0dGluZ3MoKS0+YXV0b3N0 YXJ0T3JpZ2luUGx1Z0luU25hcHNob3R0aW5nRW5hYmxlZCgpICYmIGRvY3VtZW50KCktPnBhZ2Uo KS0+cGx1Z0luQ2xpZW50KCkgJiYgZG9jdW1lbnQoKS0+cGFnZSgpLT5wbHVnSW5DbGllbnQoKS0+ c2hvdWxkQXV0b1N0YXJ0RnJvbU9yaWdpbihkb2N1bWVudCgpLT5wYWdlKCktPm1haW5GcmFtZSgp LT5kb2N1bWVudCgpLT5iYXNlVVJMKCkuaG9zdCgpLCB1cmwuaG9zdCgpLCBsb2FkZWRNaW1lVHlw ZSgpKSkgewogICAgICAgICBMT0coUGx1Z2lucywgIiVwIFBsdWctaW4gZnJvbSAoJXMsICVzKSBp cyBtYXJrZWQgdG8gYXV0by1zdGFydCwgc2V0IHRvIHBsYXkiLCB0aGlzLCBkb2N1bWVudCgpLT5w YWdlKCktPm1haW5GcmFtZSgpLT5kb2N1bWVudCgpLT5iYXNlVVJMKCkuaG9zdCgpLnV0ZjgoKS5k YXRhKCksIHVybC5ob3N0KCkudXRmOCgpLmRhdGEoKSk7CiAgICAgICAgIG1fc25hcHNob3REZWNp c2lvbiA9IE5ldmVyU25hcHNob3Q7CiAgICAgICAgIHJldHVybjsK