64954 2011-07-21 08:59:04 -0700 Unpaired calls to Document::didAddWheelEventHandler / Document::didRemoveWheelEventHandler 2012-05-01 14:51:39 -0700 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) Unspecified Unspecified UNCONFIRMED P2 Normal --- 0 rob jonlee andersca eric hyatt jochen jonlee levin rakuco oldest_to_newest 440439 0 rob 2011-07-21 08:59:04 -0700 With the EFL backend and some Chromium use cases (according to https://bugs.webkit.org/show_bug.cgi?id=60779#c27) we trigger the assertion below: void Document::didRemoveWheelEventHandler() { ASSERT(m_wheelEventHandlerCount > 0); --m_wheelEventHandlerCount; Frame* mainFrame = page() ? page()->mainFrame() : 0; if (mainFrame) mainFrame->notifyChromeClientWheelEventHandlerCountChanged(); } When navigating from the page. I observe the following from the backtraces #0 WebCore::Document::didAddWheelEventHandler (this=0x74b240) at /home/rob/src/efl-webkit/source/WebKit/Source/WebCore/dom/Document.cpp:5050 and then #0 WebCore::Document::didRemoveWheelEventHandler (this=0x856fa0) at /home/rob/src/efl-webkit/source/WebKit/Source/WebCore/dom/Document.cpp:5059 and then #0 WebCore::Document::didAddWheelEventHandler (this=0x856fa0) at /home/rob/src/efl-webkit/source/WebKit/Source/WebCore/dom/Document.cpp:5050 So from my observations the Document inside the Frame has changed before the code that's doing the tidy up to remove the handlers gets called. i.e. we would expect the call to didRemoveWheelEventHandler to act on 0x74b240. 440456 1 jonlee 2011-07-21 09:46:17 -0700 This is a dupe of 60931. *** This bug has been marked as a duplicate of bug 60931 *** 450192 2 jonlee 2011-08-11 17:37:42 -0700 Sorry, this should not have been marked as a dupe. Back to unconfirmed, and assigning to me. 450340 3 103748 rob 2011-08-12 00:51:04 -0700 Created attachment 103748 Proposed patch Here is the patch I proposed on the other bug. 523989 4 103748 eric 2011-12-19 10:38:36 -0800 Comment on attachment 103748 Proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=103748&action=review > Source/WebCore/page/FrameView.h:143 > + void detachScrollbars(); Señor Levin wrote http://trac.webkit.org/changeset/44940, so it seems he should be involved in this review as well. 542946 5 103748 jochen 2012-01-26 01:15:01 -0800 Comment on attachment 103748 Proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=103748&action=review > Source/WebCore/dom/Document.cpp:1793 > + view->detachScrollbars(); you're dropping the call to detachCustomScrollbars here. Should detachScrollbars() invoke detachCustomScrollbars() then? 543076 6 levin 2012-01-26 05:24:12 -0800 (In reply to comment #4) > (From update of attachment 103748 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=103748&action=review > > > Source/WebCore/page/FrameView.h:143 > > + void detachScrollbars(); > > Señor Levin wrote http://trac.webkit.org/changeset/44940, so it seems he should be involved in this review as well. 2 1/2 years ago :( To my memory I was fixing a crash and consulted with several other folks about the right fix. Given all this my expertise is about none for this. 613691 7 103748 ojan 2012-05-01 14:51:39 -0700 Comment on attachment 103748 Proposed patch Is this visible to the web? If so, can you add layout tests for this? All WebKit patches need a test unless there's a good reason why it can't be tested. 103748 2011-08-12 00:51:04 -0700 2012-05-01 14:51:38 -0700 Proposed patch updated-patch.diff text/plain 2310 rob ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCA3ZTU1MDk0Li43OWUxZmQxIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29y ZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTYg QEAKKzIwMTEtMDctMjIgIFJvYiBCcmFkZm9yZCAgIDxyb2JAbGludXguaW50ZWwuY29tPgorCisg ICAgICAgIEVuc3VyZSB0aGUgc2Nyb2xsYmFycyBhcmUgYWx3YXlzIHJlbW92ZWQgd2hlbiB0aGUg cGFnZSBpcyBkZXRhY2hlZAorICAgICAgICBvdGhlcndpc2UgdW5wYWlyZWQgY2FsbHMgdG8gRG9j dW1lbnQ6OmRpZEFkZFdoZWVsRXZlbnRIYW5kbGVyIGFuZAorICAgICAgICBEb2N1bWVudDo6ZGlk UmVtb3ZlV2hlZWxFdmVudEhhbmRsZXIgYXJlIG1hZGUuCisgICAgICAgIGh0dHBzOi8vYnVncy53 ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD02MDkzMQorCisgICAgICAgICogcGFnZS9GcmFtZVZp ZXcuY3BwOgorICAgICAgICAqIHBhZ2UvRnJhbWVWaWV3Lmg6CisgICAgICAgIEFkZCBkZXRhY2hT Y3JvbGxiYXJzIG1ldGhvZC4KKyAgICAgICAgKiBkb20vRG9jdW1lbnQuaDoKKyAgICAgICAgRGV0 YWNoIHNjcm9sbGJhcnMgd2hlbiB0aGUgZG9jdW1lbnQgaXMgZGV0YWNoZWQuCisKIDIwMTEtMDct MjEgIFBhdmVsIEZlbGRtYW4gIDxwZmVsZG1hbkBnb29nbGUuY29tPgogCiAgICAgICAgIFdlYiBJ bnNwZWN0b3I6IFtwcm90b2NvbF0gZXh0ZW5kIHNldEJyZWFrcG9pbnRCeVVybCByZXF1ZXN0IHRv IGFjY2VwdCBpbmNvbXBsZXRlIFVSTCAocHJlZGljYXRlcykKZGlmZiAtLWdpdCBhL1NvdXJjZS9X ZWJDb3JlL2RvbS9Eb2N1bWVudC5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9kb20vRG9jdW1lbnQuY3Bw CmluZGV4IGQyZTA5NDQuLjE2NDc0ZmYgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL2RvbS9E b2N1bWVudC5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvZG9tL0RvY3VtZW50LmNwcApAQCAtMTc5 MCw3ICsxNzkwLDcgQEAgdm9pZCBEb2N1bWVudDo6ZGV0YWNoKCkKICAgICBpZiAobV9mcmFtZSkg ewogICAgICAgICBGcmFtZVZpZXcqIHZpZXcgPSBtX2ZyYW1lLT52aWV3KCk7CiAgICAgICAgIGlm ICh2aWV3KQotICAgICAgICAgICAgdmlldy0+ZGV0YWNoQ3VzdG9tU2Nyb2xsYmFycygpOworICAg ICAgICAgICAgdmlldy0+ZGV0YWNoU2Nyb2xsYmFycygpOwogCiAgICAgfQogCmRpZmYgLS1naXQg YS9Tb3VyY2UvV2ViQ29yZS9wYWdlL0ZyYW1lVmlldy5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9wYWdl L0ZyYW1lVmlldy5jcHAKaW5kZXggYmM4ODgzMi4uODZmNGE0NCAxMDA2NDQKLS0tIGEvU291cmNl L1dlYkNvcmUvcGFnZS9GcmFtZVZpZXcuY3BwCisrKyBiL1NvdXJjZS9XZWJDb3JlL3BhZ2UvRnJh bWVWaWV3LmNwcApAQCAtMzEwLDYgKzMxMCwyMCBAQCB2b2lkIEZyYW1lVmlldzo6ZGV0YWNoQ3Vz dG9tU2Nyb2xsYmFycygpCiAgICAgfQogfQogCit2b2lkIEZyYW1lVmlldzo6ZGV0YWNoU2Nyb2xs YmFycygpCit7CisgICAgaWYgKCFtX2ZyYW1lKQorICAgICAgICByZXR1cm47CisKKyAgICBzZXRI YXNIb3Jpem9udGFsU2Nyb2xsYmFyKGZhbHNlKTsKKyAgICBzZXRIYXNWZXJ0aWNhbFNjcm9sbGJh cihmYWxzZSk7CisKKyAgICBpZiAobV9zY3JvbGxDb3JuZXIpIHsKKyAgICAgICAgbV9zY3JvbGxD b3JuZXItPmRlc3Ryb3koKTsKKyAgICAgICAgbV9zY3JvbGxDb3JuZXIgPSAwOworICAgIH0KK30K Kwogdm9pZCBGcmFtZVZpZXc6OmRpZEFkZEhvcml6b250YWxTY3JvbGxiYXIoU2Nyb2xsYmFyKiBz Y3JvbGxiYXIpCiB7CiAgICAgaWYgKG1fZnJhbWUgJiYgbV9mcmFtZS0+ZG9jdW1lbnQoKSkKZGlm ZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL3BhZ2UvRnJhbWVWaWV3LmggYi9Tb3VyY2UvV2ViQ29y ZS9wYWdlL0ZyYW1lVmlldy5oCmluZGV4IDAzNmI1OGIuLjhiZTRlZTcgMTAwNjQ0Ci0tLSBhL1Nv dXJjZS9XZWJDb3JlL3BhZ2UvRnJhbWVWaWV3LmgKKysrIGIvU291cmNlL1dlYkNvcmUvcGFnZS9G cmFtZVZpZXcuaApAQCAtMTQwLDYgKzE0MCw3IEBAIHB1YmxpYzoKICAgICB2b2lkIHJlc2V0U2Ny b2xsYmFycygpOwogICAgIHZvaWQgcmVzZXRTY3JvbGxiYXJzQW5kQ2xlYXJDb250ZW50c1NpemUo KTsKICAgICB2b2lkIGRldGFjaEN1c3RvbVNjcm9sbGJhcnMoKTsKKyAgICB2b2lkIGRldGFjaFNj cm9sbGJhcnMoKTsKICAgICB2aXJ0dWFsIHZvaWQgcmVjYWxjdWxhdGVTY3JvbGxiYXJPdmVybGF5 U3R5bGUoKTsKIAogICAgIHZvaWQgY2xlYXIoKTsK