21:06:24.901 12251 worker/17 media/media-continues-playing-after-replace-source.html crashed, (stderr lines): 21:06:24.901 12251 ASSERTION FAILED: m_wrapper || !m_jsFunction 21:06:24.901 12251 /home/buildslave-1/webkit-buildslave/efl-linux-64-debug-wk2/build/Source/WebCore/bindings/js/JSEventListener.h(90) : JSC::JSObject* WebCore::JSEventListener::jsFunction(WebCore::ScriptExecutionContext*) const 21:06:24.901 12251 1 0x7fd440decf63 WebCore::JSEventListener::jsFunction(WebCore::ScriptExecutionContext*) const 21:06:24.901 12251 2 0x7fd440df6a42 WebCore::JSEventListener::handleEvent(WebCore::ScriptExecutionContext*, WebCore::Event*) 21:06:24.901 12251 3 0x7fd4403326ca WebCore::EventTarget::fireEventListeners(WebCore::Event*, WebCore::EventTargetData*, WTF::Vector<WebCore::RegisteredEventListener, 1ul>&) 21:06:24.901 12251 4 0x7fd44033248a WebCore::EventTarget::fireEventListeners(WebCore::Event*) 21:06:24.901 12251 5 0x7fd44035dd8a WebCore::Node::handleLocalEvents(WebCore::Event*) 21:06:24.901 12251 6 0x7fd4403267b9 WebCore::EventContext::handleLocalEvents(WebCore::Event*) const 21:06:24.901 12251 7 0x7fd4403292f1 WebCore::EventDispatcher::dispatchEventAtTarget(WTF::PassRefPtr<WebCore::Event>) 21:06:24.901 12251 8 0x7fd4403286f7 WebCore::EventDispatcher::dispatchEvent(WTF::PassRefPtr<WebCore::Event>) 21:06:24.901 12251 9 0x7fd440326b4c WebCore::EventDispatchMediator::dispatchEvent(WebCore::EventDispatcher*) const 21:06:24.901 12251 10 0x7fd440327814 WebCore::EventDispatcher::dispatchEvent(WebCore::Node*, WTF::PassRefPtr<WebCore::EventDispatchMediator>) 21:06:24.901 12251 11 0x7fd44035de84 WebCore::Node::dispatchEvent(WTF::PassRefPtr<WebCore::Event>) 21:06:24.901 12251 12 0x7fd440501663 WebCore::HTMLMediaElement::dispatchEvent(WTF::PassRefPtr<WebCore::Event>) 21:06:24.901 12251 13 0x7fd44033444a WebCore::GenericEventQueue::timerFired(WebCore::Timer<WebCore::GenericEventQueue>*) 21:06:24.901 12251 14 0x7fd440335470 WebCore::Timer<WebCore::GenericEventQueue>::fired() 21:06:24.901 12251 15 0x7fd4408c04fa WebCore::ThreadTimers::sharedTimerFiredInternal() 21:06:24.901 12251 16 0x7fd4408c041b WebCore::ThreadTimers::sharedTimerFired() 21:06:24.901 12251 17 0x7fd441268fa5 21:06:24.901 12251 18 0x7fd43d76f33e _ecore_timer_expired_call 21:06:24.901 12251 19 0x7fd43d76f50b _ecore_timer_expired_timers_call 21:06:24.901 12251 20 0x7fd43d76c421 21:06:24.901 12251 21 0x7fd43d76cab7 ecore_main_loop_begin 21:06:24.902 12251 22 0x7fd441267919 WebCore::RunLoop::run() 21:06:24.902 12251 23 0x7fd44456aa22 WebProcessMainEfl 21:06:24.902 12251 24 0x4007f4 main 21:06:24.902 12251 25 0x7fd44392c76d __libc_start_main 21:06:24.902 12251 26 0x400719
I'd say r133633 is at blame here. http://trac.webkit.org/changeset/133633 At least wo media failures on GTK builders seem related, both time out: media/audio-concurrent-supported.html media/media-continues-playing-after-replace-source.html There are also random crashes and increased flakiness on Mac in (again, at least) these two tests. http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40ToT%20-%20webkit.org&tests=media%2Faudio-concurrent-supported.html%2Cmedia%2Fmedia-continues-playing-after-replace-source.html
What is the regression range?
(In reply to comment #2) > What is the regression range? media/media-continues-playing-after-replace-source.html is crashing on EFL builders consistently, after build with the following range: http://trac.webkit.org/log/?verbose=on&rev=133635&stop_rev=133630 http://build.webkit.org/builders/EFL%20Linux%2064-bit%20Debug%20WK2/builds/5560
http://trac.webkit.org/log/trunk/Source?verbose=on&rev=133635&stop_rev=133630 I agree that my patch is the only likely candidate. Perhaps we should roll it out to see if that heals the ASSERT?
This assertion usually means that JS wrapper was garbage collected too early. Perhaps some code still tries to look for the wrapper in the HashMap only, and thus fails to protect the wrapper from GC.
Adam, do you plan to look into this soon? I think that this is a pretty bad assertion to fail.
> Adam, do you plan to look into this soon? I think that this is a pretty bad assertion to fail. It's on my list for Wednesday. Is that soon enough?
Certainly! Thank you.
The assertion has been essentially removed in <http://trac.webkit.org/changeset/134495>. But the fact that it was firing presumably means that the events fail to be dispatched sometimes
(In reply to comment #9) > The assertion has been essentially removed in <http://trac.webkit.org/changeset/134495>. But the fact that it was firing presumably means that the events fail to be dispatched sometimes FYI, the assertion has been "re-installed" for the normal world in <http://trac.webkit.org/changeset/134508>.
Created attachment 174532 [details] Makes ASSERT reliable on apple-mac
I have verified that reverting http://trac.webkit.org/changeset/133633 locally makes the ASSERT not occur.
Created attachment 174563 [details] Reduction of patch that still exhibits the problem
Here's the definition of wrapperOwner from JSNode.h: inline JSC::WeakHandleOwner* wrapperOwner(DOMWrapperWorld*, Node*) { DEFINE_STATIC_LOCAL(JSNodeOwner, jsNodeOwner, ()); return &jsNodeOwner; } What's interesting about this definition is that it's declaring a static local variable in an inline function in a header. I wonder if we're getting a separate instance of the static for each compilation unit?
Created attachment 174572 [details] Reduction of patch that still exhibits the problem
> I wonder if we're getting a separate instance of the static for each compilation unit? I moved the static out of line, but that didn't help.
The issue is that Node and HTMLAudioElement are fighting about whether to use JSNodeOwner or JSHTMLAudioElementOwner as the owner object. Previously, we used JSNodeOwner in the main world and JSHTMLAudioElementOwner in isolated worlds. After my patch, we use JSHTMLAudioElementOwner in both kinds of worlds.
bug 102217 seems to be related
*** Bug 102217 has been marked as a duplicate of this bug. ***
(In reply to comment #17) > The issue is that Node and HTMLAudioElement are fighting about whether to use JSNodeOwner or JSHTMLAudioElementOwner as the owner object. Nice debugging. The behavior we want is both: We should use JSHTMLAudioElementOwner (to get ActiveDOMObject behavior), and that owner should delegate (possibly through inheritance) to JSNodeOwner.
> The behavior we want is both: We should use JSHTMLAudioElementOwner (to get ActiveDOMObject behavior), and that owner should delegate (possibly through inheritance) to JSNodeOwner. Thanks. I'll try to work up a patch that does that.
Created attachment 174744 [details] Patch
Comment on attachment 174744 [details] Patch Attachment 174744 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/14868357 New failing tests: fast/dom/shadow/shadow-dom-event-dispatching.html
@ggaren: Any interest in reviewing this patch? I believe I've taken the approach you recommended.
Comment on attachment 174744 [details] Patch r=me
Comment on attachment 174744 [details] Patch Clearing flags on attachment: 174744 Committed r135063: <http://trac.webkit.org/changeset/135063>
All reviewed patches have been landed. Closing bug.
Comment on attachment 174744 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=174744&action=review > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:1060 > + push(@headerContent, "public:\n"); Why did we add this?
(In reply to comment #28) > (From update of attachment 174744 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=174744&action=review > > > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:1060 > > + push(@headerContent, "public:\n"); > > Why did we add this? So that we could call these functions from their overrides in the derived class. I guess could have made them protected instead. The functions are basically public anyway since they're virtual and declared public in the base class.
This broke bindings tests (probably need new results): http://build.webkit.org/builders/Apple%20MountainLion%20Debug%20WK1%20%28Tests%29/builds/2930/steps/bindings-generation-tests/logs/stdio
Fixenated in http://trac.webkit.org/changeset/135081