119231 2013-07-29 17:04:35 -0700 Null deref under PluginView::handlesPageScaleFactor() 2013-07-30 12:37:57 -0700 1 1 1 Unclassified WebKit Plug-ins 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 thorton thorton andersca oldest_to_newest 912630 0 thorton 2013-07-29 17:04:35 -0700 PluginView::handlesPageScaleFactor() doesn't null-check m_plugin, but it should. It should check isInitialized as well! <rdar://problem/14440207> 912632 1 207681 thorton 2013-07-29 17:07:21 -0700 Created attachment 207681 patch 912637 2 207681 simon.fraser 2013-07-29 17:15:30 -0700 Comment on attachment 207681 patch View in context: https://bugs.webkit.org/attachment.cgi?id=207681&action=review > Source/WebKit2/WebProcess/Plugins/PluginView.h:90 > + bool handlesPageScaleFactor(); Can this be const? 912638 3 thorton 2013-07-29 17:19:05 -0700 (In reply to comment #2) > (From update of attachment 207681 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=207681&action=review > > > Source/WebKit2/WebProcess/Plugins/PluginView.h:90 > > + bool handlesPageScaleFactor(); > > Can this be const? Yep. pageScaleFactor() too! Thanks! http://trac.webkit.org/changeset/153449 912668 4 207686 thorton 2013-07-29 22:11:46 -0700 Created attachment 207686 another potential null 912674 5 thorton 2013-07-29 22:35:50 -0700 Reopened for one more patch (so many things can be null). 912675 6 207689 thorton 2013-07-29 22:36:19 -0700 Created attachment 207689 try to make ews go 912979 7 207689 darin 2013-07-30 12:26:41 -0700 Comment on attachment 207689 try to make ews go View in context: https://bugs.webkit.org/attachment.cgi?id=207689&action=review > Source/WebKit2/WebProcess/WebPage/WebFrame.cpp:490 > PluginDocument* pluginDocument = static_cast<PluginDocument*>(m_coreFrame->document()); > - PluginView* pluginView = static_cast<PluginView*>(pluginDocument->pluginWidget()); > + if (PluginView* pluginView = static_cast<PluginView*>(pluginDocument->pluginWidget())) > + return pluginView->handlesPageScaleFactor(); > > - return pluginView->handlesPageScaleFactor(); > + return 0; I prefer early return to nesting the main line code in the if. Or using &&. return pluginView && pluginView->handlesPageScaleFactor(); 912987 8 thorton 2013-07-30 12:37:57 -0700 Thanks, Darin. I went with &&. http://trac.webkit.org/changeset/153486 207681 2013-07-29 17:07:21 -0700 2013-07-29 22:11:46 -0700 patch null.diff text/plain 1876 thorton ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQyL0No YW5nZUxvZwppbmRleCA2ZGZjNWNiLi44ZDVjZjc4IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0 Mi9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTYg QEAKKzIwMTMtMDctMjkgIFRpbSBIb3J0b24gIDx0aW1vdGh5X2hvcnRvbkBhcHBsZS5jb20+CisK KyAgICAgICAgTnVsbCBkZXJlZiB1bmRlciBQbHVnaW5WaWV3OjpoYW5kbGVzUGFnZVNjYWxlRmFj dG9yKCkKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTEx OTIzMQorICAgICAgICA8cmRhcjovL3Byb2JsZW0vMTQ0NDAyMDc+CisKKyAgICAgICAgUmV2aWV3 ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgKiBXZWJQcm9jZXNzL1BsdWdpbnMvUGx1 Z2luVmlldy5jcHA6CisgICAgICAgIChXZWJLaXQ6OlBsdWdpblZpZXc6OmhhbmRsZXNQYWdlU2Nh bGVGYWN0b3IpOgorICAgICAgICAqIFdlYlByb2Nlc3MvUGx1Z2lucy9QbHVnaW5WaWV3Lmg6Cisg ICAgICAgIE51bGwtY2hlY2sgbV9wbHVnaW4gYW5kIGNoZWNrIG1faXNJbml0aWFsaXplZC4KKwog MjAxMy0wNy0yOSAgQW5kZXJzIENhcmxzc29uICA8YW5kZXJzY2FAYXBwbGUuY29tPgogCiAgICAg ICAgIFJlbW92ZSBzZXRVbmRlcmxheVBhZ2UoKSBhbmQgYXNzb2NpYXRlZCBjb2RlCmRpZmYgLS1n aXQgYS9Tb3VyY2UvV2ViS2l0Mi9XZWJQcm9jZXNzL1BsdWdpbnMvUGx1Z2luVmlldy5jcHAgYi9T b3VyY2UvV2ViS2l0Mi9XZWJQcm9jZXNzL1BsdWdpbnMvUGx1Z2luVmlldy5jcHAKaW5kZXggZDM0 MzI2Yi4uZTIwZGIzOCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdDIvV2ViUHJvY2Vzcy9QbHVn aW5zL1BsdWdpblZpZXcuY3BwCisrKyBiL1NvdXJjZS9XZWJLaXQyL1dlYlByb2Nlc3MvUGx1Z2lu cy9QbHVnaW5WaWV3LmNwcApAQCAtNDY4LDYgKzQ2OCwxNCBAQCBkb3VibGUgUGx1Z2luVmlldzo6 cGFnZVNjYWxlRmFjdG9yKCkKICAgICByZXR1cm4gbV9wYWdlU2NhbGVGYWN0b3I7CiB9CiAKK2Jv b2wgUGx1Z2luVmlldzo6aGFuZGxlc1BhZ2VTY2FsZUZhY3RvcigpCit7CisgICAgaWYgKCFtX3Bs dWdpbiB8fCAhbV9pc0luaXRpYWxpemVkKQorICAgICAgICByZXR1cm4gZmFsc2U7CisKKyAgICBy ZXR1cm4gbV9wbHVnaW4tPmhhbmRsZXNQYWdlU2NhbGVGYWN0b3IoKTsKK30KKwogdm9pZCBQbHVn aW5WaWV3Ojp3ZWJQYWdlRGVzdHJveWVkKCkKIHsKICAgICBtX3dlYlBhZ2UgPSAwOwpkaWZmIC0t Z2l0IGEvU291cmNlL1dlYktpdDIvV2ViUHJvY2Vzcy9QbHVnaW5zL1BsdWdpblZpZXcuaCBiL1Nv dXJjZS9XZWJLaXQyL1dlYlByb2Nlc3MvUGx1Z2lucy9QbHVnaW5WaWV3LmgKaW5kZXggOWRkNTNi NS4uYjY0ZDYxMiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdDIvV2ViUHJvY2Vzcy9QbHVnaW5z L1BsdWdpblZpZXcuaAorKysgYi9Tb3VyY2UvV2ViS2l0Mi9XZWJQcm9jZXNzL1BsdWdpbnMvUGx1 Z2luVmlldy5oCkBAIC04Nyw3ICs4Nyw3IEBAIHB1YmxpYzoKICAgICAKICAgICB2b2lkIHNldFBh Z2VTY2FsZUZhY3Rvcihkb3VibGUgc2NhbGVGYWN0b3IsIFdlYkNvcmU6OkludFBvaW50IG9yaWdp bik7CiAgICAgZG91YmxlIHBhZ2VTY2FsZUZhY3RvcigpOwotICAgIGJvb2wgaGFuZGxlc1BhZ2VT Y2FsZUZhY3RvcigpIHsgcmV0dXJuIG1fcGx1Z2luLT5oYW5kbGVzUGFnZVNjYWxlRmFjdG9yKCk7 IH0KKyAgICBib29sIGhhbmRsZXNQYWdlU2NhbGVGYWN0b3IoKTsKIAogICAgIHZvaWQgcGFnZVNj YWxlRmFjdG9yRGlkQ2hhbmdlKCk7CiAgICAgdm9pZCB3ZWJQYWdlRGVzdHJveWVkKCk7Cg== 207686 2013-07-29 22:11:46 -0700 2013-07-29 22:36:19 -0700 another potential null null2.diff text/plain 1533 thorton ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQyL0No YW5nZUxvZwppbmRleCAzMDU0YmFhLi42YTY2Mzk4IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0 Mi9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCkBAIC00LDYgKzQsMTkg QEAKICAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTExOTIz MQogICAgICAgICA8cmRhcjovL3Byb2JsZW0vMTQ0NDAyMDc+CiAKKyAgICAgICAgUmV2aWV3ZWQg YnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgTnVsbC1jaGVjayB0aGUgUGx1Z2luVmlldyBp biB0aGUgY2FsbGVyIGFzIHdlbGwuCisKKyAgICAgICAgKiBXZWJQcm9jZXNzL1dlYlBhZ2UvV2Vi RnJhbWUuY3BwOgorICAgICAgICAoV2ViS2l0OjpXZWJGcmFtZTo6aGFuZGxlc1BhZ2VTY2FsZUdl c3R1cmUpOgorCisyMDEzLTA3LTI5ICBUaW0gSG9ydG9uICA8dGltb3RoeV9ob3J0b25AYXBwbGUu Y29tPgorCisgICAgICAgIE51bGwgZGVyZWYgdW5kZXIgUGx1Z2luVmlldzo6aGFuZGxlc1BhZ2VT Y2FsZUZhY3RvcigpCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNn aT9pZD0xMTkyMzEKKyAgICAgICAgPHJkYXI6Ly9wcm9ibGVtLzE0NDQwMjA3PgorCiAgICAgICAg IFJldmlld2VkIGJ5IFNpbW9uIEZyYXNlci4KIAogICAgICAgICAqIFdlYlByb2Nlc3MvUGx1Z2lu cy9QbHVnaW5WaWV3LmNwcDoKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL1dlYlByb2Nlc3Mv V2ViUGFnZS9XZWJGcmFtZS5jcHAgYi9Tb3VyY2UvV2ViS2l0Mi9XZWJQcm9jZXNzL1dlYlBhZ2Uv V2ViRnJhbWUuY3BwCmluZGV4IDlhOTQ4ZjYuLjIzODZjMmMgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9X ZWJLaXQyL1dlYlByb2Nlc3MvV2ViUGFnZS9XZWJGcmFtZS5jcHAKKysrIGIvU291cmNlL1dlYktp dDIvV2ViUHJvY2Vzcy9XZWJQYWdlL1dlYkZyYW1lLmNwcApAQCAtNDg0LDkgKzQ4NCwxMCBAQCBi b29sIFdlYkZyYW1lOjpoYW5kbGVzUGFnZVNjYWxlR2VzdHVyZSgpIGNvbnN0CiAgICAgICAgIHJl dHVybiAwOwogCiAgICAgUGx1Z2luRG9jdW1lbnQqIHBsdWdpbkRvY3VtZW50ID0gc3RhdGljX2Nh c3Q8UGx1Z2luRG9jdW1lbnQqPihtX2NvcmVGcmFtZS0+ZG9jdW1lbnQoKSk7Ci0gICAgUGx1Z2lu VmlldyogcGx1Z2luVmlldyA9IHN0YXRpY19jYXN0PFBsdWdpblZpZXcqPihwbHVnaW5Eb2N1bWVu dC0+cGx1Z2luV2lkZ2V0KCkpOworICAgIGlmIChQbHVnaW5WaWV3KiBwbHVnaW5WaWV3ID0gc3Rh dGljX2Nhc3Q8UGx1Z2luVmlldyo+KHBsdWdpbkRvY3VtZW50LT5wbHVnaW5XaWRnZXQoKSkpCisg ICAgICAgIHJldHVybiBwbHVnaW5WaWV3LT5oYW5kbGVzUGFnZVNjYWxlRmFjdG9yKCk7CiAKLSAg ICByZXR1cm4gcGx1Z2luVmlldy0+aGFuZGxlc1BhZ2VTY2FsZUZhY3RvcigpOworICAgIHJldHVy biAwOwogfQogCiBJbnRSZWN0IFdlYkZyYW1lOjpjb250ZW50Qm91bmRzKCkgY29uc3QK 207689 2013-07-29 22:36:19 -0700 2013-07-30 12:26:41 -0700 try to make ews go null2.diff text/plain 1533 thorton ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQyL0No YW5nZUxvZwppbmRleCAzMDU0YmFhLi42YTY2Mzk4IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0 Mi9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCkBAIC00LDYgKzQsMTkg QEAKICAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTExOTIz MQogICAgICAgICA8cmRhcjovL3Byb2JsZW0vMTQ0NDAyMDc+CiAKKyAgICAgICAgUmV2aWV3ZWQg YnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgTnVsbC1jaGVjayB0aGUgUGx1Z2luVmlldyBp biB0aGUgY2FsbGVyIGFzIHdlbGwuCisKKyAgICAgICAgKiBXZWJQcm9jZXNzL1dlYlBhZ2UvV2Vi RnJhbWUuY3BwOgorICAgICAgICAoV2ViS2l0OjpXZWJGcmFtZTo6aGFuZGxlc1BhZ2VTY2FsZUdl c3R1cmUpOgorCisyMDEzLTA3LTI5ICBUaW0gSG9ydG9uICA8dGltb3RoeV9ob3J0b25AYXBwbGUu Y29tPgorCisgICAgICAgIE51bGwgZGVyZWYgdW5kZXIgUGx1Z2luVmlldzo6aGFuZGxlc1BhZ2VT Y2FsZUZhY3RvcigpCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNn aT9pZD0xMTkyMzEKKyAgICAgICAgPHJkYXI6Ly9wcm9ibGVtLzE0NDQwMjA3PgorCiAgICAgICAg IFJldmlld2VkIGJ5IFNpbW9uIEZyYXNlci4KIAogICAgICAgICAqIFdlYlByb2Nlc3MvUGx1Z2lu cy9QbHVnaW5WaWV3LmNwcDoKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL1dlYlByb2Nlc3Mv V2ViUGFnZS9XZWJGcmFtZS5jcHAgYi9Tb3VyY2UvV2ViS2l0Mi9XZWJQcm9jZXNzL1dlYlBhZ2Uv V2ViRnJhbWUuY3BwCmluZGV4IDlhOTQ4ZjYuLjIzODZjMmMgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9X ZWJLaXQyL1dlYlByb2Nlc3MvV2ViUGFnZS9XZWJGcmFtZS5jcHAKKysrIGIvU291cmNlL1dlYktp dDIvV2ViUHJvY2Vzcy9XZWJQYWdlL1dlYkZyYW1lLmNwcApAQCAtNDg0LDkgKzQ4NCwxMCBAQCBi b29sIFdlYkZyYW1lOjpoYW5kbGVzUGFnZVNjYWxlR2VzdHVyZSgpIGNvbnN0CiAgICAgICAgIHJl dHVybiAwOwogCiAgICAgUGx1Z2luRG9jdW1lbnQqIHBsdWdpbkRvY3VtZW50ID0gc3RhdGljX2Nh c3Q8UGx1Z2luRG9jdW1lbnQqPihtX2NvcmVGcmFtZS0+ZG9jdW1lbnQoKSk7Ci0gICAgUGx1Z2lu VmlldyogcGx1Z2luVmlldyA9IHN0YXRpY19jYXN0PFBsdWdpblZpZXcqPihwbHVnaW5Eb2N1bWVu dC0+cGx1Z2luV2lkZ2V0KCkpOworICAgIGlmIChQbHVnaW5WaWV3KiBwbHVnaW5WaWV3ID0gc3Rh dGljX2Nhc3Q8UGx1Z2luVmlldyo+KHBsdWdpbkRvY3VtZW50LT5wbHVnaW5XaWRnZXQoKSkpCisg ICAgICAgIHJldHVybiBwbHVnaW5WaWV3LT5oYW5kbGVzUGFnZVNjYWxlRmFjdG9yKCk7CiAKLSAg ICByZXR1cm4gcGx1Z2luVmlldy0+aGFuZGxlc1BhZ2VTY2FsZUZhY3RvcigpOworICAgIHJldHVy biAwOwogfQogCiBJbnRSZWN0IFdlYkZyYW1lOjpjb250ZW50Qm91bmRzKCkgY29uc3QK