|Summary:||REGRESSION(r125251): It made svg/custom/use-instanceRoot-as-event-target.xhtml assert and flakey|
|Product:||WebKit||Reporter:||Csaba Osztrogonác <ossy>|
|Severity:||Major||CC:||abarth, ap, barraclough, buildbot, commit-queue, darin, dglazkov, dominicc, ggaren, guijemont, hayato, kling, koivisto, ossy, rniwa, zan|
|Priority:||P2||Keywords:||Gtk, LayoutTestFailure, Qt, QtTriaged|
|Version:||528+ (Nightly build)|
|Bug Depends on:||132471|
|Bug Blocks:||79668, 88232|
Description Csaba Osztrogonác 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.
Comment 2 Csaba Osztrogonác 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.
Comment 3 Hayato Ito 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.
Comment 4 Csaba Osztrogonác 2012-08-13 06:24:55 PDT
*** Bug 93819 has been marked as a duplicate of this bug. ***
Comment 5 Csaba Osztrogonác 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)
Comment 6 Csaba Osztrogonác 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
Comment 7 Dominic Cooney 2012-08-13 23:47:23 PDT
This is my mess. I will try to clean it up.
Comment 8 Alexey Proskuryakov 2012-09-13 17:19:15 PDT
Also flaky on Mac.
Comment 9 Alexey Proskuryakov 2012-09-13 17:19:59 PDT
This was already skipped on platform/mac, I'll re-classify the failure.
Comment 10 Zan Dobersek 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
Comment 11 Alexey Proskuryakov 2013-09-24 16:14:07 PDT
We still have <svg/custom/use-instanceRoot-as-event-target.xhtml> skipped because of this.
Comment 12 Alexey Proskuryakov 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 ***
Comment 14 Alexey Proskuryakov 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
Comment 15 Darin Adler 2014-05-03 20:01:19 PDT
It’s entirely possible that my fix for bug 132148 didn’t actually solve the problem!
Comment 17 Guillaume Emont 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.
Comment 18 Build Bot 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
Comment 19 Build Bot 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
Comment 20 Andreas Kling 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.
Comment 21 Darin Adler 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.
Comment 22 Darin Adler 2015-01-18 09:05:15 PST
*** This bug has been marked as a duplicate of bug 132148 ***
Comment 23 Darin Adler 2015-01-18 09:06:07 PST
This time for sure. I actually understand what’s going on now!