r86584 relies on didAddWheelEventHandler() and willRemoveWheelEventHandler() getting called the same number of times. We're getting errors on a few tests after this CL, and I think the implication is that willRemoveWheelEventHandler() is being called too often - I haven't been able to debug this after a few hours of trying, so I'm not sure of the cause: ASSERTION FAILED: m_wheelEventHandlerCount > 0 Backtrace: WebCore::Document::didRemoveWheelEventHandler [0x01428ED6+70] (e:\b\build\slave\webkit_win__dbg__2_\build\src\third_party\webkit\source\webcore\dom\document.cpp:5095) WebCore::FrameView::willRemoveHorizontalScrollbar [0x013D9242+98] (e:\b\build\slave\webkit_win__dbg__2_\build\src\third_party\webkit\source\webcore\page\frameview.cpp:323) WebCore::ScrollView::setHasHorizontalScrollbar [0x018CEF26+310] (e:\b\build\slave\webkit_win__dbg__2_\build\src\third_party\webkit\source\webcore\platform\scrollview.cpp:100) WebCore::ScrollView::updateScrollbars [0x018D088B+315] (e:\b\build\slave\webkit_win__dbg__2_\build\src\third_party\webkit\source\webcore\platform\scrollview.cpp:477) WebCore::ScrollView::setScrollbarModes [0x018CF271+193] (e:\b\build\slave\webkit_win__dbg__2_\build\src\third_party\webkit\source\webcore\platform\scrollview.cpp:163) WebCore::ScrollView::setHorizontalScrollbarMode [0x013DB774+52] (e:\b\build\slave\webkit_win__dbg__2_\build\src\third_party\webkit\source\webcore\platform\scrollview.h:92) WebCore::FrameView::layout [0x013DB1AA+1754] (e:\b\build\slave\webkit_win__dbg__2_\build\src\third_party\webkit\source\webcore\page\frameview.cpp:942) WebCore::Document::implicitClose [0x0141C023+851] (e:\b\build\slave\webkit_win__dbg__2_\build\src\third_party\webkit\source\webcore\dom\document.cpp:2162) WebCore::FrameLoader::checkCallImplicitClose [0x013BF994+132] (e:\b\build\slave\webkit_win__dbg__2_\build\src\third_party\webkit\source\webcore\loader\frameloader.cpp:980) WebCore::FrameLoader::checkCompleted [0x013BF6FD+237] (e:\b\build\slave\webkit_win__dbg__2_\build\src\third_party\webkit\source\webcore\loader\frameloader.cpp:929) WebCore::FrameLoader::finishedParsing [0x013BF468+152] (e:\b\build\slave\webkit_win__dbg__2_\build\src\third_party\webkit\source\webcore\loader\frameloader.cpp:863) WebCore::Document::finishedParsing [0x0142538C+332] (e:\b\build\slave\webkit_win__dbg__2_\build\src\third_party\webkit\source\webcore\dom\document.cpp:4273) WebCore::HTMLTreeBuilder::finished [0x0219C6B4+100] (e:\b\build\slave\webkit_win__dbg__2_\build\src\third_party\webkit\source\webcore\html\parser\htmltreebuilder.cpp:2803) WebCore::HTMLDocumentParser::end [0x0213C963+131] (e:\b\build\slave\webkit_win__dbg__2_\build\src\third_party\webkit\source\webcore\html\parser\htmldocumentparser.cpp:379) WebCore::HTMLDocumentParser::attemptToRunDeferredScriptsAndEnd [0x0213CA36+182] (e:\b\build\slave\webkit_win__dbg__2_\build\src\third_party\webkit\source\webcore\html\parser\htmldocumentparser.cpp:388) WebCore::HTMLDocumentParser::prepareToStopParsing [0x0213B7AC+188] (e:\b\build\slave\webkit_win__dbg__2_\build\src\third_party\webkit\source\webcore\html\parser\htmldocumentparser.cpp:152) WebCore::HTMLDocumentParser::attemptToEnd [0x0213CAA9+57] (e:\b\build\slave\webkit_win__dbg__2_\build\src\third_party\webkit\source\webcore\html\parser\htmldocumentparser.cpp:399) WebCore::HTMLDocumentParser::finish [0x0213CBF3+51] (e:\b\build\slave\webkit_win__dbg__2_\build\src\third_party\webkit\source\webcore\html\parser\htmldocumentparser.cpp:428) WebCore::Document::finishParsing [0x0141C61B+75] (e:\b\build\slave\webkit_win__dbg__2_\build\src\third_party\webkit\source\webcore\dom\document.cpp:2285) WebCore::DocumentWriter::endIfNotLoadingMainResource [0x0141342D+125] (e:\b\build\slave\webkit_win__dbg__2_\build\src\third_party\webkit\source\webcore\loader\documentwriter.cpp:225) WebCore::DocumentWriter::end [0x01413397+39] (e:\b\build\slave\webkit_win__dbg__2_\build\src\third_party\webkit\source\webcore\loader\documentwriter.cpp:210) WebCore::DocumentWriter::replaceDocument [0x014123D6+230] (e:\b\build\slave\webkit_win__dbg__2_\build\src\third_party\webkit\source\webcore\loader\documentwriter.cpp:85) WebCore::ScriptController::executeIfJavaScriptURL [0x014C2B97+391] (e:\b\build\slave\webkit_win__dbg__2_\build\src\third_party\webkit\source\webcore\bindings\scriptcontrollerbase.cpp:114) WebCore::FrameLoader::urlSelected [0x013BCAB7+135] (e:\b\build\slave\webkit_win__dbg__2_\build\src\third_party\webkit\source\webcore\loader\frameloader.cpp:288) WebCore::FrameLoader::changeLocation [0x013BC8FD+157] (e:\b\build\slave\webkit_win__dbg__2_\build\src\third_party\webkit\source\webcore\loader\frameloader.cpp:271) WebCore::DOMWindow::createWindow [0x014B9995+677] (e:\b\build\slave\webkit_win__dbg__2_\build\src\third_party\webkit\source\webcore\page\domwindow.cpp:1752) WebCore::DOMWindow::open [0x014B9EA7+983] (e:\b\build\slave\webkit_win__dbg__2_\build\src\third_party\webkit\source\webcore\page\domwindow.cpp:1821) WebCore::V8DOMWindow::openCallback [0x016305CF+463] (e:\b\build\slave\webkit_win__dbg__2_\build\src\third_party\webkit\source\webcore\bindings\v8\custom\v8domwindowcustom.cpp:456) v8::internal::HandleApiCallHelper<0> [0x0064EE3C+892] (e:\b\build\slave\webkit_win__dbg__2_\build\src\v8\src\builtins.cc:1126) v8::internal::Builtin_Impl_HandleApiCall [0x00649CD4+20] (e:\b\build\slave\webkit_win__dbg__2_\build\src\v8\src\builtins.cc:1143) v8::internal::Builtin_HandleApiCall [0x00649C52+66] (e:\b\build\slave\webkit_win__dbg__2_\build\src\v8\src\builtins.cc:1142) (No symbol) [0x0CC300B6] (No symbol) [0x3DBC413C] (No symbol) [0x0CC3115F] (No symbol) [0x0CC438D1] (No symbol) [0x0CC30B22] v8::internal::Invoke [0x004DAD1C+412] (e:\b\build\slave\webkit_win__dbg__2_\build\src\v8\src\execution.cc:122) v8::internal::Execution::Call [0x004DAB62+34] (e:\b\build\slave\webkit_win__dbg__2_\build\src\v8\src\execution.cc:153) v8::Function::Call [0x00478AD7+407] (e:\b\build\slave\webkit_win__dbg__2_\build\src\v8\src\api.cc:3297) WebCore::V8Proxy::callFunction [0x01381190+464] (e:\b\build\slave\webkit_win__dbg__2_\build\src\third_party\webkit\source\webcore\bindings\v8\v8proxy.cpp:483) WebCore::V8EventListener::callListenerFunction [0x01633907+167] (e:\b\build\slave\webkit_win__dbg__2_\build\src\third_party\webkit\source\webcore\bindings\v8\v8eventlistener.cpp:77) WebCore::V8AbstractEventListener::invokeEventHandler [0x01818E6C+332] (e:\b\build\slave\webkit_win__dbg__2_\build\src\third_party\webkit\source\webcore\bindings\v8\v8abstracteventlistener.cpp:154) WebCore::V8AbstractEventListener::handleEvent [0x01818B43+275] (e:\b\build\slave\webkit_win__dbg__2_\build\src\third_party\webkit\source\webcore\bindings\v8\v8abstracteventlistener.cpp:99) WebCore::EventTarget::fireEventListeners [0x01544F75+261] (e:\b\build\slave\webkit_win__dbg__2_\build\src\third_party\webkit\source\webcore\dom\eventtarget.cpp:360) WebCore::EventTarget::fireEventListeners [0x01544E06+262] (e:\b\build\slave\webkit_win__dbg__2_\build\src\third_party\webkit\source\webcore\dom\eventtarget.cpp:331) WebCore::DOMWindow::dispatchEvent [0x014B8E4E+238] (e:\b\build\slave\webkit_win__dbg__2_\build\src\third_party\webkit\source\webcore\page\domwindow.cpp:1588) WebCore::DOMWindow::dispatchTimedEvent [0x014B9066+150] (e:\b\build\slave\webkit_win__dbg__2_\build\src\third_party\webkit\source\webcore\page\domwindow.cpp:1601) WebCore::DOMWindow::dispatchLoadEvent [0x014B8BB2+242] (e:\b\build\slave\webkit_win__dbg__2_\build\src\third_party\webkit\source\webcore\page\domwindow.cpp:1564) WebCore::Document::dispatchWindowLoadEvent [0x0142145F+95] (e:\b\build\slave\webkit_win__dbg__2_\build\src\third_party\webkit\source\webcore\dom\document.cpp:3523) WebCore::Document::implicitClose [0x0141BE66+406] (e:\b\build\slave\webkit_win__dbg__2_\build\src\third_party\webkit\source\webcore\dom\document.cpp:2119) WebCore::FrameLoader::checkCallImplicitClose [0x013BF994+132] (e:\b\build\slave\webkit_win__dbg__2_\build\src\third_party\webkit\source\webcore\loader\frameloader.cpp:980) WebCore::FrameLoader::checkCompleted [0x013BF6FD+237] (e:\b\build\slave\webkit_win__dbg__2_\build\src\third_party\webkit\source\webcore\loader\frameloader.cpp:929) WebCore::FrameLoader::finishedParsing [0x013BF468+152] (e:\b\build\slave\webkit_win__dbg__2_\build\src\third_party\webkit\source\webcore\loader\frameloader.cpp:863) WebCore::Document::finishedParsing [0x0142538C+332] (e:\b\build\slave\webkit_win__dbg__2_\build\src\third_party\webkit\source\webcore\dom\document.cpp:4273) WebCore::HTMLTreeBuilder::finished [0x0219C6B4+100] (e:\b\build\slave\webkit_win__dbg__2_\build\src\third_party\webkit\source\webcore\html\parser\htmltreebuilder.cpp:2803) WebCore::HTMLDocumentParser::end [0x0213C963+131] (e:\b\build\slave\webkit_win__dbg__2_\build\src\third_party\webkit\source\webcore\html\parser\htmldocumentparser.cpp:379) WebCore::HTMLDocumentParser::attemptToRunDeferredScriptsAndEnd [0x0213CA36+182] (e:\b\build\slave\webkit_win__dbg__2_\build\src\third_party\webkit\source\webcore\html\parser\htmldocumentparser.cpp:388) WebCore::HTMLDocumentParser::prepareToStopParsing [0x0213B7AC+188] (e:\b\build\slave\webkit_win__dbg__2_\build\src\third_party\webkit\source\webcore\html\parser\htmldocumentparser.cpp:152) WebCore::HTMLDocumentParser::attemptToEnd [0x0213CAA9+57] (e:\b\build\slave\webkit_win__dbg__2_\build\src\third_party\webkit\source\webcore\html\parser\htmldocumentparser.cpp:399) WebCore::HTMLDocumentParser::finish [0x0213CBF3+51] (e:\b\build\slave\webkit_win__dbg__2_\build\src\third_party\webkit\source\webcore\html\parser\htmldocumentparser.cpp:428) WebCore::Document::finishParsing [0x0141C61B+75] (e:\b\build\slave\webkit_win__dbg__2_\build\src\third_party\webkit\source\webcore\dom\document.cpp:2285) WebCore::DocumentWriter::endIfNotLoadingMainResource [0x0141342D+125] (e:\b\build\slave\webkit_win__dbg__2_\build\src\third_party\webkit\source\webcore\loader\documentwriter.cpp:225) It doesn't seem to fail on the webkit.org bots, nor can I get it to fail in a chromium build, so I think that it may just be a chromium DRT/TestShell issue. I'm going to disable the tests that break as a result of this so I can roll forward.
<rdar://problem/9449486>
*** Bug 64954 has been marked as a duplicate of this bug. ***
My notes from the duplicated bug: 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.
Created attachment 101733 [details] Patch to detach scrollbars The problem is that when the document gets detached from the frame the scrollbars don't get removed. One solution that seems to work for me is to always detach the scrollbars when you detach the the document from the frame. Here is a patch to do that. First patch to WebKit, be gentle. :-)
Comment on attachment 101733 [details] Patch to detach scrollbars Patch should have been review? not review+
Attachment 101733 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/dom/Document.cpp:1793: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 1 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 101882 [details] Updated patch to address automated review issue
This bug seems to be the cause of some tests crashing in the GTK bots too. See the backtrace here: 0x00002b12b17b1d2b in WebCore::Document::didRemoveWheelEventHandler (this=0x728d780) at ../../Source/WebCore/dom/Document.cpp:5167 #1 0x00002b12b1bf13b0 in WebCore::FrameView::willRemoveHorizontalScrollbar (this=0x7330890, scrollbar=0x73ed700) at ../../Source/WebCore/page/FrameView.cpp:337 #2 0x00002b12b1d1a12c in WebCore::ScrollView::setHasHorizontalScrollbar (this=0x7330890, hasBar=false) at ../../Source/WebCore/platform/ScrollView.cpp:98 #3 0x00002b12b1d1b3f9 in WebCore::ScrollView::updateScrollbars (this=0x7330890, desiredOffset=...) at ../../Source/WebCore/platform/ScrollView.cpp:468 #4 0x00002b12b254a61a in WebCore::ScrollView::setScrollbarModes (this=0x7330890, horizontalMode=WebCore::ScrollbarAlwaysOff, verticalMode=WebCore::ScrollbarAlwaysOn, horizontalLock=false, verticalLock=false) at ../../Source/WebCore/platform/gtk/ScrollViewGtk.cpp:117 #5 0x00002b12b1bfa735 in WebCore::ScrollView::setHorizontalScrollbarMode (this=0x7330890, mode=WebCore::ScrollbarAlwaysOff, lock=false) at ../../Source/WebCore/platform/ScrollView.h:93 #6 0x00002b12b1bf2dfd in WebCore::FrameView::layout (this=0x7330890, allowSubtree=true) at ../../Source/WebCore/page/FrameView.cpp:1059 #7 0x00002b12b17a62ac in WebCore::Document::implicitClose (this=0x728d780) at ../../Source/WebCore/dom/Document.cpp:2231 #8 0x00002b12b1b2d519 in WebCore::FrameLoader::checkCallImplicitClose (this=0x740a1d8) at ../../Source/WebCore/loader/FrameLoader.cpp:796 #9 0x00002b12b1b2d2ec in WebCore::FrameLoader::checkCompleted (this=0x740a1d8) at ../../Source/WebCore/loader/FrameLoader.cpp:744 #10 0x00002b12b1b2d053 in WebCore::FrameLoader::finishedParsing (this=0x740a1d8) at ../../Source/WebCore/loader/FrameLoader.cpp:678 #11 0x00002b12b17ae8da in WebCore::Document::finishedParsing (this=0x728d780) at ../../Source/WebCore/dom/Document.cpp:4306 #12 0x00002b12b1a07c86 in WebCore::HTMLTreeBuilder::finished (this=0x738dc80) at ../../Source/WebCore/html/parser/HTMLTreeBuilder.cpp:2826 #13 0x00002b12b19dd5dc in WebCore::HTMLDocumentParser::end (this=0x74b4f00) at ../../Source/WebCore/html/parser/HTMLDocumentParser.cpp:381 #14 0x00002b12b19dd6d9 in WebCore::HTMLDocumentParser::attemptToRunDeferredScriptsAndEnd (this=0x74b4f00) at ../../Source/WebCore/html/parser/HTMLDocumentParser.cpp:390 #15 0x00002b12b19dc7ef in WebCore::HTMLDocumentParser::prepareToStopParsing (this=0x74b4f00) at ../../Source/WebCore/html/parser/HTMLDocumentParser.cpp:153 #16 0x00002b12b19dd71e in WebCore::HTMLDocumentParser::attemptToEnd (this=0x74b4f00) at ../../Source/WebCore/html/parser/HTMLDocumentParser.cpp:402 #17 0x00002b12b19dd7d7 in WebCore::HTMLDocumentParser::finish (this=0x74b4f00) at ../../Source/WebCore/html/parser/HTMLDocumentParser.cpp:429 #18 0x00002b12b1b28062 in WebCore::DocumentWriter::endIfNotLoadingMainResource (this=0x73210a0) at ../../Source/WebCore/loader/DocumentWriter.cpp:235 #19 0x00002b12b1b27f79 in WebCore::DocumentWriter::end (this=0x73210a0) at ../../Source/WebCore/loader/DocumentWriter.cpp:214 #20 0x00002b12b1b273f7 in WebCore::DocumentWriter::replaceDocument (this=0x73210a0, source="bar") at ../../Source/WebCore/loader/DocumentWriter.cpp:84 #21 0x00002b12b1627b37 in WebCore::ScriptController::executeIfJavaScriptURL (this=0x740a5f0, url=..., shouldReplaceDocumentIfJavaScriptURL=WebCore::ReplaceDocumentIfJavaScriptURL) at ../../Source/WebCore/bindings/ScriptControllerBase.cpp:128 #22 0x00002b12b1b2b54e in WebCore::FrameLoader::urlSelected (this=0x740a1d8, passedRequest=..., triggeringEvent=..., lockHistory=true, lockBackForwardList=false, referrerPolicy=WebCore::SendReferrer, shouldReplaceDocumentIfJavaScriptURL=WebCore::ReplaceDocumentIfJavaScriptURL) at ../../Source/WebCore/loader/FrameLoader.cpp:284 #23 0x00002b12b1b2b2f1 in WebCore::FrameLoader::changeLocation (this=0x740a1d8, securityOrigin=0x73f1c10, url=..., referrer="http://127.0.0.1:8000/security/cookies/cookie-theft-with-javascript-doc.html", lockHistory=true, lockBackForwardList=false, refresh=false) at ../../Source/WebCore/loader/FrameLoader.cpp:267 #24 0x00002b12b1b71378 in WebCore::ScheduledURLNavigation::fire (this=0x736f630, frame=0x740a120) at ../../Source/WebCore/loader/NavigationScheduler.cpp:109 #25 0x00002b12b1b70ba1 in WebCore::NavigationScheduler::timerFired (this=0x740a538) at ../../Source/WebCore/loader/NavigationScheduler.cpp:418 #26 0x00002b12b1b72d60 in WebCore::Timer<WebCore::NavigationScheduler>::fired (this=0x740a540) at ../../Source/WebCore/platform/Timer.h:100 #27 0x00002b12b1d40f70 in WebCore::ThreadTimers::sharedTimerFiredInternal (this=0xbe4fc0) at ../../Source/WebCore/platform/ThreadTimers.cpp:115 #28 0x00002b12b1d40ea7 in WebCore::ThreadTimers::sharedTimerFired () at ../../Source/WebCore/platform/ThreadTimers.cpp:93 #29 0x00002b12b254ac16 in WebCore::timeout_cb () at ../../Source/WebCore/platform/gtk/SharedTimerGtk.cpp:49 #30 0x00002b12b66afddb in g_timeout_dispatch (source=0x7470510, callback=<optimized out>, user_data=<optimized out>) at /tmp/buildd/glib2.0-2.28.6/./glib/gmain.c:3882 #31 0x00002b12b66ae4a3 in g_main_dispatch (context=0xb11e60) at /tmp/buildd/glib2.0-2.28.6/./glib/gmain.c:2440 #32 g_main_context_dispatch (context=0xb11e60) at /tmp/buildd/glib2.0-2.28.6/./glib/gmain.c:3013 #33 0x00002b12b66aec80 in g_main_context_iterate (context=0xb11e60, block=1, dispatch=1, self=<optimized out>) at /tmp/buildd/glib2.0-2.28.6/./glib/gmain.c:3091 #34 0x00002b12b66af2f2 in g_main_loop_run (loop=0x713b750) at /tmp/buildd/glib2.0-2.28.6/./glib/gmain.c:3299 #35 0x00002b12b445c4cd in gtk_main () from /usr/lib/libgtk-3.so.0 #36 0x000000000042f2e5 in runTest (testPathOrURL=...) at ../../Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:710 #37 0x000000000042e91d in runTestingServerLoop () at ../../Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:502 #38 0x0000000000430c58 in main (argc=2, argv=0x7fff16b2f508) at ../../Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:1205 Skipping in GTK in the meanwhile.
Comment on attachment 101882 [details] Updated patch to address automated review issue View in context: https://bugs.webkit.org/attachment.cgi?id=101882&action=review > Source/WebCore/ChangeLog:7 > + It's unfortunate that there is no test case. The bug refers to 3 ASSERT in Chromium's test_expectations.txt, have you checked that your patch solves them? If it does not solve them, it looks like this is not a fix for the right bug. You would need an explanation as to why there is no test in this case. > Source/WebCore/dom/Document.cpp:1793 > + view->detachScrollbars(); There are 2 other callers to detachCustomScrollbars, wouldn't they also need to be converted to avoid such a bug too? > Source/WebCore/page/FrameView.cpp:313 > +void FrameView::detachScrollbars() It would also be neat to change FrameView::~FrameView to use this method for consistency. > Source/WebCore/page/FrameView.cpp:316 > + return; I wonder if this check is needed. In detachCustomScrollbars, it looks like a legacy as none of the following code requires a Frame.
(In reply to comment #9) > (From update of attachment 101882 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=101882&action=review > > > Source/WebCore/ChangeLog:7 > > + > > It's unfortunate that there is no test case. The bug refers to 3 ASSERT in Chromium's test_expectations.txt, have you checked that your patch solves them? If it does not solve them, it looks like this is not a fix for the right bug. You would need an explanation as to why there is no test in this case. These tests haven't crashed since the end of October, perhaps fixed by http://trac.webkit.org/changeset/98778/ (it's the only one in the span that mentions wheel events. Any further info on this?
Comment on attachment 101882 [details] Updated patch to address automated review issue View in context: https://bugs.webkit.org/attachment.cgi?id=101882&action=review The change looks sane to me but I think Andersca should give the final review as he knows the code a lot more than I do. >>> Source/WebCore/ChangeLog:7 >>> + >> >> It's unfortunate that there is no test case. The bug refers to 3 ASSERT in Chromium's test_expectations.txt, have you checked that your patch solves them? If it does not solve them, it looks like this is not a fix for the right bug. You would need an explanation as to why there is no test in this case. > > These tests haven't crashed since the end of October, perhaps fixed by http://trac.webkit.org/changeset/98778/ (it's the only one in the span that mentions wheel events. Any further info on this? Fine enough, this change would need a test case then or an explanation as to why it lacks one as nothing seems to be checking that the calls are properly paired.
(In reply to comment #11) > Fine enough, this change would need a test case then or an explanation as to why it lacks one as nothing seems to be checking that the calls are properly paired. One idea to implement the test is to add a method to LayoutTestController to enable/disable log of adding/removing scrollbar. By the way, the related tests still crashed according to my investigation.
Seems this bug hasn't attracted attention for a while. After reading the comments for Rob's patch, I was curious why there was only customized scrollbar got removed before, so I dug the old related bug, which is https://bugs.webkit.org/show_bug.cgi?id=26326. In the comment 9 of bug 26326, Dave said "You don't want to always destroy the scrollbars. You only want to nuke them if the document supplied them. If they were custom basically.". @Dave,would you like give the context about why we should not always destroy the scrollbars when the view's document gets detached. This issue is just because we don't do that. @Rob, will you have time to continue working on this issue?
Comment on attachment 101882 [details] Updated patch to address automated review issue View in context: https://bugs.webkit.org/attachment.cgi?id=101882&action=review > Source/WebCore/ChangeLog:5 > + Ensure the scrollbars are always removed when the page is detached > + otherwise unpaired calls to Document::didAddWheelEventHandler and > + Document::didRemoveWheelEventHandler are made. This doesn't match the bug summary. Please move the description to below the bug url followed by a blank line. Also, you're missing "Reviewed by" below. r- because of this.