112450 2013-03-15 09:50:54 -0700 Clean up RenderFrameSet::nodeAtPoint 2013-03-18 05:01:41 -0700 1 1 1 Unclassified WebKit UI Events 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 allan.jensen allan.jensen darin eric esprehn+autocc ojan.autocc webkit.review.bot oldest_to_newest 856052 0 allan.jensen 2013-03-15 09:50:54 -0700 As part of my work to remove side-effects in hit-testing I want to also eliminate any tests on the HitTestRequest::isReadOnly(), only one such remain and is in a cryptic test in RenderFrameSet::nodeAtPoint. if (inside && frameSet()->noResize() && !request.readOnly() && !result.innerNode() && !request.touchMove()) Reseaching the history of the test shows that it has been invalid since r18333 in December 2006 where a part of the test was accidentally reversed from !element()->noResize to frameSet->noResize(). That has however had no effect since the check itself was dead code since r17770 in November 2006 where the newly introduced EventHandler started taking care of sending events the frameset being resized without needing a hit-test. The check can therefore be removed and RenderFrameSet::nodeAtPoint made both cleaner and sane. 856062 1 193328 allan.jensen 2013-03-15 10:04:34 -0700 Created attachment 193328 Patch 856157 2 193328 eric 2013-03-15 11:53:17 -0700 Comment on attachment 193328 Patch OK. I assume we have some tests for this? If not, we definitely need to add one. 856391 3 193328 darin 2013-03-15 16:29:12 -0700 Comment on attachment 193328 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=193328&action=review > Source/WebCore/rendering/RenderFrameSet.cpp:166 > if (action != HitTestForeground) > return false; Do we still need this part? What does this accomplish? I think we probably want to remove the entire function unless this has some benefit. 856982 4 allan.jensen 2013-03-18 04:27:46 -0700 (In reply to comment #3) > (From update of attachment 193328 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=193328&action=review > > > Source/WebCore/rendering/RenderFrameSet.cpp:166 > > if (action != HitTestForeground) > > return false; > > Do we still need this part? What does this accomplish? I think we probably want to remove the entire function unless this has some benefit. No, I guess not. RenderBox::nodeAtPoint also makes the check if (action == HitTestForeground). 856988 5 193527 allan.jensen 2013-03-18 04:40:08 -0700 Created attachment 193527 Patch for landing 857003 6 allan.jensen 2013-03-18 05:01:41 -0700 Committed r146056: <http://trac.webkit.org/changeset/146056> 193328 2013-03-15 10:04:34 -0700 2013-03-18 04:40:04 -0700 Patch bug-112450-20130315175937.patch text/plain 2047 allan.jensen U3VidmVyc2lvbiBSZXZpc2lvbjogMTQ1ODk5CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggYmViNDczNjAxY2U2YzMw ZjY3YjNkNzEwNTIyNDgxY2EzOGI4NTg4Yy4uMjAzMjMxYTI3MzgxYWZiYmZkNjQwYmNhOWJhMTM1 NmRmMzkzZTRkNCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE4IEBACisyMDEzLTAzLTE1ICBBbGxh biBTYW5kZmVsZCBKZW5zZW4gIDxhbGxhbi5qZW5zZW5AZGlnaWEuY29tPgorCisgICAgICAgIENs ZWFuIHVwIFJlbmRlckZyYW1lU2V0Ojpub2RlQXRQb2ludAorICAgICAgICBodHRwczovL2J1Z3Mu d2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTEyNDUwCisKKyAgICAgICAgUmV2aWV3ZWQgYnkg Tk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgUmVtb3ZlIGhhbmRsaW5nIG9mIHJlc2l6aW5nIGZy YW1lc2V0cyBmcm9tIFJlbmRlckZyYW1lU2V0Ojpub2RlQXRQb2ludC4KKyAgICAgICAgVGhlIGNv ZGUgaGFzIGJlZW4gaW5jb3JyZWN0IHNpbmNlIGEgbWlzdGFrZSBpbiByMTgzMzMgKERlY2VtYmVy IDIwMDYpLAorICAgICAgICBidXQgaGFzIGJlZW4gZGVhZCBjb2RlIHNpbmNlIHIxNzc3MCAoTm92 ZW1iZXIgMjAwNikgd2hlcmUgdGhlIHRoZW4gbmV3CisgICAgICAgIEV2ZW50SGFuZGxlciBzdGFy dGVkIHRha2luZyBjYXJlIG9mIHJvdXRpbmcgZXZlbnRzIHRvIHJlc2l6aW5nIEZyYW1lU2V0cy4K KworICAgICAgICAqIHJlbmRlcmluZy9SZW5kZXJGcmFtZVNldC5jcHA6CisgICAgICAgIChXZWJD b3JlOjpSZW5kZXJGcmFtZVNldDo6bm9kZUF0UG9pbnQpOgorCiAyMDEzLTAzLTE1ICBNYXJqYSBI w7ZsdHTDpCAgPG1hcmphQGNocm9taXVtLm9yZz4KIAogICAgICAgICBbVjhdIEFkZCBtYWNoaW5l cnkgZm9yIGdlbmVyYXRpbmcgc3BlY2lhbGl6ZWQgYmluZGluZ3MgZm9yIHRoZSBtYWluIHdvcmxk CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9yZW5kZXJpbmcvUmVuZGVyRnJhbWVTZXQuY3Bw IGIvU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL1JlbmRlckZyYW1lU2V0LmNwcAppbmRleCA3NmRh YmFkNGIyY2EyZTBmNzEzZGYzOWQ1NzA5ZTdiODcxZDEyODI3Li4yNzg5NGM2NDJlNDM1MzhmMGQx NTM3MTMzNzM5MjIwZmJmMDQzODI3IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9yZW5kZXJp bmcvUmVuZGVyRnJhbWVTZXQuY3BwCisrKyBiL1NvdXJjZS9XZWJDb3JlL3JlbmRlcmluZy9SZW5k ZXJGcmFtZVNldC5jcHAKQEAgLTE2NCwxNyArMTY0LDcgQEAgYm9vbCBSZW5kZXJGcmFtZVNldDo6 bm9kZUF0UG9pbnQoY29uc3QgSGl0VGVzdFJlcXVlc3QmIHJlcXVlc3QsIEhpdFRlc3RSZXN1bHQm IHIKIHsKICAgICBpZiAoYWN0aW9uICE9IEhpdFRlc3RGb3JlZ3JvdW5kKQogICAgICAgICByZXR1 cm4gZmFsc2U7Ci0KLSAgICBib29sIGluc2lkZSA9IFJlbmRlckJveDo6bm9kZUF0UG9pbnQocmVx dWVzdCwgcmVzdWx0LCBsb2NhdGlvbkluQ29udGFpbmVyLCBhY2N1bXVsYXRlZE9mZnNldCwgYWN0 aW9uKQotICAgICAgICB8fCBtX2lzUmVzaXppbmc7Ci0KLSAgICBpZiAoaW5zaWRlICYmIGZyYW1l U2V0KCktPm5vUmVzaXplKCkKLSAgICAgICAgICAgICYmICFyZXF1ZXN0LnJlYWRPbmx5KCkgJiYg IXJlc3VsdC5pbm5lck5vZGUoKSAmJiAhcmVxdWVzdC50b3VjaE1vdmUoKSkgewotICAgICAgICBy ZXN1bHQuc2V0SW5uZXJOb2RlKG5vZGUoKSk7Ci0gICAgICAgIHJlc3VsdC5zZXRJbm5lck5vblNo YXJlZE5vZGUobm9kZSgpKTsKLSAgICB9Ci0KLSAgICByZXR1cm4gaW5zaWRlIHx8IG1faXNDaGls ZFJlc2l6aW5nOworICAgIHJldHVybiBSZW5kZXJCb3g6Om5vZGVBdFBvaW50KHJlcXVlc3QsIHJl c3VsdCwgbG9jYXRpb25JbkNvbnRhaW5lciwgYWNjdW11bGF0ZWRPZmZzZXQsIGFjdGlvbik7CiB9 CiAKIHZvaWQgUmVuZGVyRnJhbWVTZXQ6OkdyaWRBeGlzOjpyZXNpemUoaW50IHNpemUpCg== 193527 2013-03-18 04:40:08 -0700 2013-03-18 04:40:08 -0700 Patch for landing bug-112450-20130318123507.patch text/plain 3067 allan.jensen U3VidmVyc2lvbiBSZXZpc2lvbjogMTQ2MDUyCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNmYzNzM2YzNhM2YzNjNh N2JhNGE5NGRhN2YzZmE2NjUwZTNmNWQ0OS4uNWFjMDQ3YjAxMzRiOGMwZjBiMTEzMDczMWQ5ODFj Mzg3YzE0MzVkMyAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIyIEBACisyMDEzLTAzLTE4ICBBbGxh biBTYW5kZmVsZCBKZW5zZW4gIDxhbGxhbi5qZW5zZW5AZGlnaWEuY29tPgorCisgICAgICAgIENs ZWFuIHVwIFJlbmRlckZyYW1lU2V0Ojpub2RlQXRQb2ludAorICAgICAgICBodHRwczovL2J1Z3Mu d2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTEyNDUwCisKKyAgICAgICAgUmV2aWV3ZWQgYnkg RXJpYyBTZWlkZWwuCisKKyAgICAgICAgUmVtb3ZlIGhhbmRsaW5nIG9mIHJlc2l6aW5nIGZyYW1l c2V0cyBmcm9tIFJlbmRlckZyYW1lU2V0Ojpub2RlQXRQb2ludC4KKyAgICAgICAgVGhlIGNvZGUg aGFzIGJlZW4gaW5jb3JyZWN0IHNpbmNlIGEgbWlzdGFrZSBpbiByMTgzMzMgKERlY2VtYmVyIDIw MDYpLAorICAgICAgICBidXQgaGFzIGJlZW4gZGVhZCBjb2RlIHNpbmNlIHIxNzc3MCAoTm92ZW1i ZXIgMjAwNikgd2hlcmUgdGhlIHRoZW4gbmV3CisgICAgICAgIEV2ZW50SGFuZGxlciBzdGFydGVk IHRha2luZyBjYXJlIG9mIHJvdXRpbmcgZXZlbnRzIHRvIHJlc2l6aW5nIEZyYW1lU2V0cy4KKwor ICAgICAgICBTaW5jZSB0aGlzIHdhcyB0aGUgb25seSBzcGVjaWFsIGZlYXR1cmUgb2YgUmVuZGVy RnJhbWVTZXQ6Om5vZGVBdFBvaW50LAorICAgICAgICB0aGUgbWV0aG9kIGhhcyBiZWVuIHJlbW92 ZWQuCisKKyAgICAgICAgKiByZW5kZXJpbmcvUmVuZGVyRnJhbWVTZXQuY3BwOgorICAgICAgICAq IHJlbmRlcmluZy9SZW5kZXJGcmFtZVNldC5oOgorICAgICAgICAoUmVuZGVyRnJhbWVTZXQpOgor CiAyMDEzLTAzLTE4ICBLZWlzaGkgSGF0dG9yaSAgPGtlaXNoaUB3ZWJraXQub3JnPgogCiAgICAg ICAgIEFkZCB0b3VjaCBzdXBwb3J0IHRvIHRoZSBjYWxlbmRhciBwaWNrZXIKZGlmZiAtLWdpdCBh L1NvdXJjZS9XZWJDb3JlL3JlbmRlcmluZy9SZW5kZXJGcmFtZVNldC5jcHAgYi9Tb3VyY2UvV2Vi Q29yZS9yZW5kZXJpbmcvUmVuZGVyRnJhbWVTZXQuY3BwCmluZGV4IDc2ZGFiYWQ0YjJjYTJlMGY3 MTNkZjM5ZDU3MDllN2I4NzFkMTI4MjcuLjhiMDEzNWY3MTM4MGUxMDM4YTU3ZTM4NjAyMWE1MjU1 YTU1NzI0Y2YgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL3JlbmRlcmluZy9SZW5kZXJGcmFt ZVNldC5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL1JlbmRlckZyYW1lU2V0LmNw cApAQCAtMTU5LDI0ICsxNTksNiBAQCB2b2lkIFJlbmRlckZyYW1lU2V0OjpwYWludChQYWludElu Zm8mIHBhaW50SW5mbywgY29uc3QgTGF5b3V0UG9pbnQmIHBhaW50T2Zmc2V0KQogICAgIH0KIH0K IAotYm9vbCBSZW5kZXJGcmFtZVNldDo6bm9kZUF0UG9pbnQoY29uc3QgSGl0VGVzdFJlcXVlc3Qm IHJlcXVlc3QsIEhpdFRlc3RSZXN1bHQmIHJlc3VsdCwKLSAgICBjb25zdCBIaXRUZXN0TG9jYXRp b24mIGxvY2F0aW9uSW5Db250YWluZXIsIGNvbnN0IExheW91dFBvaW50JiBhY2N1bXVsYXRlZE9m ZnNldCwgSGl0VGVzdEFjdGlvbiBhY3Rpb24pCi17Ci0gICAgaWYgKGFjdGlvbiAhPSBIaXRUZXN0 Rm9yZWdyb3VuZCkKLSAgICAgICAgcmV0dXJuIGZhbHNlOwotCi0gICAgYm9vbCBpbnNpZGUgPSBS ZW5kZXJCb3g6Om5vZGVBdFBvaW50KHJlcXVlc3QsIHJlc3VsdCwgbG9jYXRpb25JbkNvbnRhaW5l ciwgYWNjdW11bGF0ZWRPZmZzZXQsIGFjdGlvbikKLSAgICAgICAgfHwgbV9pc1Jlc2l6aW5nOwot Ci0gICAgaWYgKGluc2lkZSAmJiBmcmFtZVNldCgpLT5ub1Jlc2l6ZSgpCi0gICAgICAgICAgICAm JiAhcmVxdWVzdC5yZWFkT25seSgpICYmICFyZXN1bHQuaW5uZXJOb2RlKCkgJiYgIXJlcXVlc3Qu dG91Y2hNb3ZlKCkpIHsKLSAgICAgICAgcmVzdWx0LnNldElubmVyTm9kZShub2RlKCkpOwotICAg ICAgICByZXN1bHQuc2V0SW5uZXJOb25TaGFyZWROb2RlKG5vZGUoKSk7Ci0gICAgfQotCi0gICAg cmV0dXJuIGluc2lkZSB8fCBtX2lzQ2hpbGRSZXNpemluZzsKLX0KLQogdm9pZCBSZW5kZXJGcmFt ZVNldDo6R3JpZEF4aXM6OnJlc2l6ZShpbnQgc2l6ZSkKIHsKICAgICBtX3NpemVzLnJlc2l6ZShz aXplKTsKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL3JlbmRlcmluZy9SZW5kZXJGcmFtZVNl dC5oIGIvU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL1JlbmRlckZyYW1lU2V0LmgKaW5kZXggMTkx ODBhNWFlZjgzYjIzOGRhZmMzOTNkY2IyNDFiZjVhZGYwOGRhYS4uMTYzOTg2ZmE4ZTI3NWU0ZWE1 MDQyNGUyMmNiYzkzMTFlNjAyMGJhOSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvcmVuZGVy aW5nL1JlbmRlckZyYW1lU2V0LmgKKysrIGIvU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL1JlbmRl ckZyYW1lU2V0LmgKQEAgLTEwNCw3ICsxMDQsNiBAQCBwcml2YXRlOgogICAgIHZpcnR1YWwgYm9v bCBpc0ZyYW1lU2V0KCkgY29uc3QgeyByZXR1cm4gdHJ1ZTsgfQogCiAgICAgdmlydHVhbCB2b2lk IGxheW91dCgpOwotICAgIHZpcnR1YWwgYm9vbCBub2RlQXRQb2ludChjb25zdCBIaXRUZXN0UmVx dWVzdCYsIEhpdFRlc3RSZXN1bHQmLCBjb25zdCBIaXRUZXN0TG9jYXRpb24mIGxvY2F0aW9uSW5D b250YWluZXIsIGNvbnN0IExheW91dFBvaW50JiBhY2N1bXVsYXRlZE9mZnNldCwgSGl0VGVzdEFj dGlvbikgT1ZFUlJJREU7CiAgICAgdmlydHVhbCB2b2lkIHBhaW50KFBhaW50SW5mbyYsIGNvbnN0 IExheW91dFBvaW50Jik7CiAgICAgdmlydHVhbCBib29sIGlzQ2hpbGRBbGxvd2VkKFJlbmRlck9i amVjdCosIFJlbmRlclN0eWxlKikgY29uc3Q7CiAgICAgdmlydHVhbCBDdXJzb3JEaXJlY3RpdmUg Z2V0Q3Vyc29yKGNvbnN0IExheW91dFBvaW50JiwgQ3Vyc29yJikgY29uc3Q7Cg==