113713 2013-04-01 12:15:18 -0700 [WK2] WebPageProxy's setViewNeedsDisplay() and viewSize() should check page validity. 2013-04-02 11:36:10 -0700 1 1 1 Unclassified WebKit WebKit2 528+ (Nightly build) Unspecified Unspecified NEW P2 Normal --- 1 marcelo.lira marcelo.lira andersca jesus sergio oldest_to_newest 867269 0 marcelo.lira 2013-04-01 12:15:18 -0700 WebPageProxy's setViewNeedsDisplay() and viewSize() should check page validity. Soon after WebProcess dies there's still a call to those methods, and at the time the PageClient the WebPageProxy holds is invalid. 867272 1 196002 marcelo.lira 2013-04-01 12:17:36 -0700 Created attachment 196002 Patch 868090 2 196002 benjamin 2013-04-02 11:21:00 -0700 Comment on attachment 196002 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=196002&action=review > Source/WebKit2/ChangeLog:4 > + WebPageProxy's setViewNeedsDisplay() and viewSize() should check page validity. > + https://bugs.webkit.org/show_bug.cgi?id=113713 No they shouldn't. You must explain _why_ the change is needed. And add a test unless you have a good reason. 868095 3 jesus 2013-04-02 11:23:27 -0700 (In reply to comment #2) > (From update of attachment 196002 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=196002&action=review > > > Source/WebKit2/ChangeLog:4 > > + WebPageProxy's setViewNeedsDisplay() and viewSize() should check page validity. > > + https://bugs.webkit.org/show_bug.cgi?id=113713 > > No they shouldn't. > > You must explain _why_ the change is needed. And add a test unless you have a good reason. Since you r- this, so why they shouldn't check the validity then? 868100 4 andersca 2013-04-02 11:30:51 -0700 (In reply to comment #3) > (In reply to comment #2) > > (From update of attachment 196002 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=196002&action=review > > > > > Source/WebKit2/ChangeLog:4 > > > + WebPageProxy's setViewNeedsDisplay() and viewSize() should check page validity. > > > + https://bugs.webkit.org/show_bug.cgi?id=113713 > > > > No they shouldn't. > > > > You must explain _why_ the change is needed. And add a test unless you have a good reason. > > > Since you r- this, so why they shouldn't check the validity then? Other ports rely on being able to call setViewNeedsDisplay even if the page is invalid. You mustn’t break this contract. 868114 5 jesus 2013-04-02 11:36:10 -0700 (In reply to comment #4) > (In reply to comment #3) > > (In reply to comment #2) > > > (From update of attachment 196002 [details] [details] [details]) > > > View in context: https://bugs.webkit.org/attachment.cgi?id=196002&action=review > > > > > > > Source/WebKit2/ChangeLog:4 > > > > + WebPageProxy's setViewNeedsDisplay() and viewSize() should check page validity. > > > > + https://bugs.webkit.org/show_bug.cgi?id=113713 > > > > > > No they shouldn't. > > > > > > You must explain _why_ the change is needed. And add a test unless you have a good reason. > > > > > > Since you r- this, so why they shouldn't check the validity then? > > Other ports rely on being able to call setViewNeedsDisplay even if the page is invalid. You mustn’t break this contract. Oh ok, thanks for the explanation! So I guess perhaps this is not the right way to check what Marcelo is looking for... Would checking if (m_pageClient) be acceptable here, since it is a raw pointer ? 196002 2013-04-01 12:17:36 -0700 2013-04-02 11:21:00 -0700 Patch bug-113713-20130401161728.patch text/plain 2408 marcelo.lira U3VidmVyc2lvbiBSZXZpc2lvbjogMTQ3MzQ4CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0Mi9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViS2l0Mi9DaGFuZ2VMb2cKaW5kZXggYzlhMTYxMGZlOTNjNzVk OWViZWQ1OTUwMTZmODY3MTcxNTZiYjYxOS4uN2Y2MzVhNzAxN2JjYTMwNjYzMmI3NWQ5NDRhNTk3 YzdjMTE1NzdlMiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJLaXQyL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE3IEBACisyMDEzLTA0LTAxICBNYXJj ZWxvIExpcmEgIDxtYXJjZWxvLmxpcmFAb3BlbmJvc3NhLm9yZz4KKworICAgICAgICBXZWJQYWdl UHJveHkncyBzZXRWaWV3TmVlZHNEaXNwbGF5KCkgYW5kIHZpZXdTaXplKCkgc2hvdWxkIGNoZWNr IHBhZ2UgdmFsaWRpdHkuCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVn LmNnaT9pZD0xMTM3MTMKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKwor ICAgICAgICAqIFVJUHJvY2Vzcy9XZWJQYWdlUHJveHkuY3BwOgorICAgICAgICAoV2ViS2l0OjpX ZWJQYWdlUHJveHk6OmlzVmFsaWQpOgorICAgICAgICAoV2ViS2l0OjpXZWJQYWdlUHJveHk6OnNl dFZpZXdOZWVkc0Rpc3BsYXkpOgorICAgICAgICAoV2ViS2l0OjpXZWJQYWdlUHJveHk6OnZpZXdT aXplKToKKyAgICAgICAgKiBVSVByb2Nlc3MvV2ViUGFnZVByb3h5Lmg6CisgICAgICAgIChXZWJQ YWdlUHJveHkpOgorCiAyMDEzLTA0LTAxICBBbGV4ZXkgUHJvc2t1cnlha292ICA8YXBAYXBwbGUu Y29tPgogCiAgICAgICAgIDxyZGFyOi8vcHJvYmxlbS8xMzQ5MDEyMz4gU2FuZGJveCBleHRlbnNp b24gdHJhY2tpbmcgaXMgaW5jb3JyZWN0IHdoZW4gcmVzdG9yaW5nIGEgc2Vzc2lvbgpkaWZmIC0t Z2l0IGEvU291cmNlL1dlYktpdDIvVUlQcm9jZXNzL1dlYlBhZ2VQcm94eS5jcHAgYi9Tb3VyY2Uv V2ViS2l0Mi9VSVByb2Nlc3MvV2ViUGFnZVByb3h5LmNwcAppbmRleCA4NTE3MzQwNTViZTI0OWYy ZTVkMDA5MjgzZWMwZDU1ZjZiNWNhMTE2Li5kYWI2ZmFmZGY4OGJkNTZhYjJhNjczYWVlZDRjNWQ3 ZTQ3MTczMWZlIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvV2ViUGFnZVBy b3h5LmNwcAorKysgYi9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvV2ViUGFnZVByb3h5LmNwcApA QCAtMzc0LDcgKzM3NCw3IEBAIFBsYXRmb3JtUHJvY2Vzc0lkZW50aWZpZXIgV2ViUGFnZVByb3h5 Ojpwcm9jZXNzSWRlbnRpZmllcigpIGNvbnN0CiAgICAgcmV0dXJuIG1fcHJvY2Vzcy0+cHJvY2Vz c0lkZW50aWZpZXIoKTsKIH0KIAotYm9vbCBXZWJQYWdlUHJveHk6OmlzVmFsaWQoKQorYm9vbCBX ZWJQYWdlUHJveHk6OmlzVmFsaWQoKSBjb25zdAogewogICAgIC8vIEEgcGFnZSB0aGF0IGhhcyBi ZWVuIGV4cGxpY2l0bHkgY2xvc2VkIGlzIG5ldmVyIHZhbGlkLgogICAgIGlmIChtX2lzQ2xvc2Vk KQpAQCAtOTU5LDYgKzk1OSw4IEBAIHZvaWQgV2ViUGFnZVByb3h5Ojp2aWV3V2lsbEVuZExpdmVS ZXNpemUoKQogCiB2b2lkIFdlYlBhZ2VQcm94eTo6c2V0Vmlld05lZWRzRGlzcGxheShjb25zdCBJ bnRSZWN0JiByZWN0KQogeworICAgIGlmICghaXNWYWxpZCgpKQorICAgICAgICByZXR1cm47CiAg ICAgbV9wYWdlQ2xpZW50LT5zZXRWaWV3TmVlZHNEaXNwbGF5KHJlY3QpOwogfQogCkBAIC0xMDQy LDYgKzEwNDQsOCBAQCB2b2lkIFdlYlBhZ2VQcm94eTo6dmlld1N0YXRlRGlkQ2hhbmdlKFZpZXdT dGF0ZUZsYWdzIGZsYWdzKQogCiBJbnRTaXplIFdlYlBhZ2VQcm94eTo6dmlld1NpemUoKSBjb25z dAogeworICAgIGlmICghaXNWYWxpZCgpKQorICAgICAgICByZXR1cm4gSW50U2l6ZSgpOwogICAg IHJldHVybiBtX3BhZ2VDbGllbnQtPnZpZXdTaXplKCk7CiB9CiAKZGlmZiAtLWdpdCBhL1NvdXJj ZS9XZWJLaXQyL1VJUHJvY2Vzcy9XZWJQYWdlUHJveHkuaCBiL1NvdXJjZS9XZWJLaXQyL1VJUHJv Y2Vzcy9XZWJQYWdlUHJveHkuaAppbmRleCAzYjMyZWVmODQxODIxZGVlZTNhZGU3NGY0N2Q5MjA0 Zjc4NzZkZmM4Li41M2I3YWVkMzljZmRlZjhjOTQwZTM0YjJlYWI0ODM5YzBjM2RjY2UyIDEwMDY0 NAotLS0gYS9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvV2ViUGFnZVByb3h5LmgKKysrIGIvU291 cmNlL1dlYktpdDIvVUlQcm9jZXNzL1dlYlBhZ2VQcm94eS5oCkBAIC02MjcsNyArNjI3LDcgQEAg cHVibGljOgogCiAgICAgV2ViUGFnZUdyb3VwKiBwYWdlR3JvdXAoKSBjb25zdCB7IHJldHVybiBt X3BhZ2VHcm91cC5nZXQoKTsgfQogCi0gICAgYm9vbCBpc1ZhbGlkKCk7CisgICAgYm9vbCBpc1Zh bGlkKCkgY29uc3Q7CiAKICAgICBQYXNzUmVmUHRyPEltbXV0YWJsZUFycmF5PiByZWxhdGVkUGFn ZXMoKSBjb25zdDsKIAo=