RESOLVED DUPLICATE of bug 132148 93812
REGRESSION(r125251): It made svg/custom/use-instanceRoot-as-event-target.xhtml assert and flakey
https://bugs.webkit.org/show_bug.cgi?id=93812
Summary REGRESSION(r125251): It made svg/custom/use-instanceRoot-as-event-target.xhtm...
Csaba Osztrogonác
Reported 2012-08-13 02:46:45 PDT
Qt crash log attached It seems https://bugs.webkit.org/show_bug.cgi?id=93727 is a similar bug. This test started to crash from r125133 On GTK, but from r125251 on Qt.
Attachments
crash log (5.35 KB, text/plain)
2012-08-13 02:47 PDT, Csaba Osztrogonác
no flags
Patch (6.19 KB, patch)
2014-08-27 13:21 PDT, Guillaume Emont
darin: review-
buildbot: commit-queue-
Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2 (625.83 KB, application/zip)
2014-08-27 22:38 PDT, Build Bot
no flags
Csaba Osztrogonác
Comment 1 2012-08-13 02:47:23 PDT
Created attachment 157949 [details] crash log
Csaba Osztrogonác
Comment 2 2012-08-13 04:18:17 PDT
I marked it as crash on Qt in debug mode - https://trac.webkit.org/changeset/125404/trunk/LayoutTests/platform/qt/TestExpectations Please remove this expectation with the proper fix. Thanks.
Hayato Ito
Comment 3 2012-08-13 04:23:41 PDT
As for svg/custom/use-instanceRoot-as-event-target.xhtml, It seems r125251 is the cause. I am now taking a look at other crashes on https://bugs.webkit.org/show_bug.cgi?id=93727.
Csaba Osztrogonác
Comment 4 2012-08-13 06:24:55 PDT
*** Bug 93819 has been marked as a duplicate of this bug. ***
Csaba Osztrogonác
Comment 5 2012-08-13 06:26:41 PDT
This test asserts in debug mode, but it is flakey (TEXT PASS) in relase mode. After r125251 svg/custom/use-instanceRoot-as-event-target.xhtml is very flakey on Qt platforms with the following error: FAIL: Timed out waiting for notifyDone to be called On Qt WK1 it was flakey between r125251-125396 and it fails always from r125398. And it fails always (not flakey) on Qt WK2 from r125251. And it is flakey again on Qt WK1 after r125404 (skip an unrelated test)
Csaba Osztrogonác
Comment 6 2012-08-13 06:34:43 PDT
(In reply to comment #2) > I marked it as crash on Qt in debug mode - https://trac.webkit.org/changeset/125404/trunk/LayoutTests/platform/qt/TestExpectations > > Please remove this expectation with the proper fix. Thanks. I moved it to the Skipped list instead of TestExpectations because of this complex bug - https://trac.webkit.org/changeset/125416
Dominic Cooney
Comment 7 2012-08-13 23:47:23 PDT
This is my mess. I will try to clean it up.
Alexey Proskuryakov
Comment 8 2012-09-13 17:19:15 PDT
Also flaky on Mac.
Alexey Proskuryakov
Comment 9 2012-09-13 17:19:59 PDT
This was already skipped on platform/mac, I'll re-classify the failure.
Zan Dobersek
Comment 10 2013-01-17 03:42:08 PST
The test is still exhibiting flaky crashing in debug builds and flaky timeouts in release builds. The Mac and Qt ports now skip the test. It's not failing on EFL, though. Dashboard: http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40ToT%20-%20webkit.org&showAllRuns=true&tests=svg%2Fcustom%2Fuse-instanceRoot-as-event-target.xhtml
Alexey Proskuryakov
Comment 11 2013-09-24 16:14:07 PDT
We still have <svg/custom/use-instanceRoot-as-event-target.xhtml> skipped because of this.
Alexey Proskuryakov
Comment 12 2014-05-01 22:20:45 PDT
I expect that Darin fixed this one too, unmarked in <http://trac.webkit.org/r168150>. *** This bug has been marked as a duplicate of bug 132148 ***
WebKit Commit Bot
Comment 13 2014-05-02 10:24:45 PDT
Re-opened since this is blocked by bug 132471
Alexey Proskuryakov
Comment 14 2014-05-02 10:26:31 PDT
Surprisingly, this test is still failing/asserting. Rolling the expectations back in. ASSERTION FAILED: !m_isolatedWorld->isNormal() || m_wrapper || !m_jsFunction /Volumes/Data/slave/mavericks-debug/build/Source/WebCore/bindings/js/JSEventListener.h(96) : JSC::JSObject *WebCore::JSEventListener::jsFunction(WebCore::ScriptExecutionContext *) const 1 0x10e107b80 WTFCrash 2 0x10ffd9f20 WebCore::JSEventListener::jsFunction(WebCore::ScriptExecutionContext*) const 3 0x11029ec3b WebCore::JSEventListener::handleEvent(WebCore::ScriptExecutionContext*, WebCore::Event*) 4 0x10fa6748f WebCore::EventTarget::fireEventListeners(WebCore::Event*, WebCore::EventTargetData*, WTF::Vector<WebCore::RegisteredEventListener, 1ul, WTF::CrashOnOverflow>&) 5 0x10fa66d5e WebCore::EventTarget::fireEventListeners(WebCore::Event*) 6 0x11091ccec WebCore::Node::handleLocalEvents(WebCore::Event&) 7 0x10fa31281 WebCore::EventContext::handleLocalEvents(WebCore::Event&) const 8 0x10fa31597 WebCore::MouseOrFocusEventContext::handleLocalEvents(WebCore::Event&) const
Darin Adler
Comment 15 2014-05-03 20:01:19 PDT
It’s entirely possible that my fix for bug 132148 didn’t actually solve the problem!
Guillaume Emont
Comment 16 2014-08-27 13:21:26 PDT
Guillaume Emont
Comment 17 2014-08-27 13:26:25 PDT
While I could not reproduce the issue with svg/custom/use-instanceRoot-as-event-target.xhtml, I hit the same ASSERT in an (oldish) qt branch I am working on, and I could reproduce the issue in an up to date WebkitGtk+ build. The test I provide seem to be flakey if I test without my patch (though it always fails at least once if I --repeat-each=2), and it also fails (in the same flakey manner) in Release, even though it doesn't hit the ASSERT.
Build Bot
Comment 18 2014-08-27 22:38:24 PDT
Comment on attachment 237244 [details] Patch Attachment 237244 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/5584579922493440 New failing tests: storage/indexeddb/delete-closed-database-object.html
Build Bot
Comment 19 2014-08-27 22:38:28 PDT
Created attachment 237290 [details] Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-11 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Andreas Kling
Comment 20 2014-09-03 10:43:01 PDT
Does this mean that registering an event listener on an object will cause that object to stay alive forever? That seems bad.
Darin Adler
Comment 21 2014-09-03 23:37:17 PDT
Comment on attachment 237244 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=237244&action=review Geoff, what’s your analysis about what’s happening here? > Source/WebCore/bindings/js/JSEventListener.h:72 > + mutable JSC::Strong<JSC::JSObject> m_wrapper; I don’t think we can just change this from Weak to Strong; doing that would lead to cycles and every object with an event listener that captured the object would leak. We should write a test case that checks this does not happen. > LayoutTests/ChangeLog:9 > + There are cases where the wrapper is not referenced from anywhere any more at > + the time when the event needs to be fired, which prevents that firing. We can’t fix this bug without understanding more about those cases.
Darin Adler
Comment 22 2015-01-18 09:05:15 PST
*** This bug has been marked as a duplicate of bug 132148 ***
Darin Adler
Comment 23 2015-01-18 09:06:07 PST
This time for sure. I actually understand what’s going on now!
Note You need to log in before you can comment on or make changes to this bug.