before: http://build.webkit.org/builders/EFL%20Linux%2064-bit%20Release%20WK1/builds/5723 after: http://build.webkit.org/builders/EFL%20Linux%2064-bit%20Release%20WK1/builds/5724 crash log: STDERR: 1 0x7fa0359401c0 STDERR: 2 0x7fa030671ff0 STDERR: 3 0x7fa033b1de27 WebCore::HTMLElement::ieForbidsInsertHTML() const STDERR: 4 0x7fa033b209ad WebCore::HTMLElement::setInnerText(WTF::String const&, int&) STDERR: 5 0x7fa033b76d8b WebCore::HTMLTextFormControlElement::setInnerTextValue(WTF::String const&) STDERR: 6 0x7fa033b9125c WebCore::TextFieldInputType::updateInnerTextValue() STDERR: 7 0x7fa0339c10f9 WebCore::Element::attributeChanged(WebCore::QualifiedName const&, WTF::AtomicString const&, WTF::AtomicString const&, WebCore::Element::AttributeModificationReason) STDERR: 8 0x7fa0339bf4fc WebCore::Element::didAddAttribute(WebCore::QualifiedName const&, WTF::AtomicString const&) STDERR: 9 0x7fa0339c1d7d WebCore::Element::addAttributeInternal(WebCore::QualifiedName const&, WTF::AtomicString const&, WebCore::Element::SynchronizationOfLazyAttribute) STDERR: 10 0x7fa0339c3dad WebCore::Element::setAttributeWithoutSynchronization(WebCore::QualifiedName const&, WTF::AtomicString const&) STDERR: 11 0x7fa033c1b6cb WebCore::MediaControlRewindButtonElement::create(WebCore::Document&) STDERR: 12 0x7fa03436128e WebCore::MediaControlsApple::createControls(WebCore::Document&) STDERR: 13 0x7fa034361b65 WebCore::MediaControls::create(WebCore::Document&) STDERR: 14 0x7fa033b422e4 WebCore::HTMLMediaElement::createMediaControls() STDERR: 15 0x7fa033b424d8 WebCore::HTMLMediaElement::configureMediaControls() STDERR: 16 0x7fa033b4dde4 WebCore::HTMLMediaElement::insertedInto(WebCore::ContainerNode&) STDERR: 17 0x7fa0339842ad WebCore::ContainerNode::parserAppendChild(WTF::PassRefPtr<WebCore::Node>) STDERR: 18 0x7fa033bcd9e9 WebCore::HTMLConstructionSite::executeQueuedTasks() STDERR: 19 0x7fa033bd7594 WebCore::HTMLDocumentParser::constructTreeFromHTMLToken(WebCore::HTMLToken&) STDERR: 20 0x7fa033bd7bf4 WebCore::HTMLDocumentParser::pumpTokenizer(WebCore::HTMLDocumentParser::SynchronousMode) STDERR: 21 0x7fa033bd88d2 WebCore::HTMLDocumentParser::append(WTF::PassRefPtr<WTF::StringImpl>) STDERR: 22 0x7fa03398ee2e WebCore::DecodedDataDocumentParser::flush(WebCore::DocumentWriter&) STDERR: 23 0x7fa033ccfe03 WebCore::DocumentWriter::end() STDERR: 24 0x7fa033cc5e07 WebCore::DocumentLoader::finishedLoading(double) STDERR: 25 0x7fa033d46841 WebCore::CachedResource::checkNotify() STDERR: 26 0x7fa033d449c0 WebCore::CachedRawResource::finishLoading(WebCore::ResourceBuffer*) STDERR: 27 0x7fa033d0db1e WebCore::SubresourceLoader::didFinishLoading(double) STDERR: 28 0x7fa0343ad371 STDERR: 29 0x7fa02e55704a STDERR: 30 0x7fa02e5765db STDERR: 31 0x7fa02e5765f9 Mark it as blocker/P1, because EFL bots exit early because of this regression, so they have near 0 test coverage since 5 days / 200 revisions. The bug should be fixed as soon as possible or media tests should be skipped if you are interested in catching new regressions by the bots.
Created attachment 228389 [details] backtrace Here is a gdb backtrace for media/audio-concurrent-supported.html, it might help someone interested in fixing this regression. My question is why EFL uses MediaControlsApple.cpp?
(In reply to comment #1) > Created an attachment (id=228389) [details] > backtrace > > Here is a gdb backtrace for media/audio-concurrent-supported.html, > it might help someone interested in fixing this regression. More information, This crash is started from r166395 ( Bug 130882 ). But I am not sure why it occurs in EFL because it is general code. > > My question is why EFL uses MediaControlsApple.cpp? EFL may need to have own MediaControls but it looks not related to this issue.
We have the same problem in the Haiku port (also using MediaControlsApple.cpp). Moving the setPseudo call to the create methods, after the call to setType, seems to work around this, but I don't know if that's the proper fix.
Here is our patch for this: https://github.com/haiku/webkit/commit/1a7810025da941981a6e39cc4b199eb47615e0c2
(In reply to comment #2) > (In reply to comment #1) > > Created an attachment (id=228389) [details] [details] > > backtrace > > > > Here is a gdb backtrace for media/audio-concurrent-supported.html, > > it might help someone interested in fixing this regression. > > More information, > This crash is started from r166395 ( Bug 130882 ). > But I am not sure why it occurs in EFL because it is general code. At least, I realized that GTK and (maybe) MAC does not use this crash path because they use MEDIA_CONTROLS_SCRIPT. Just for test, I tried to use the code of GTK port. And media/audio-concurrent-supported.html is passed. (control is bit ugly for efl port) IMO, we should turn on the MEDIA_CONTROLS_SCRIPT. but I am not sure whether it is correct solution for this crash. Anyway, I will prepare proper css/js for EFL port. (It may takes more time)
It seems the EFL bots are still broken from this regression. (during ~500 revisions and 11 days) Is there any plan to fix them to be able catch new regressions again?
(In reply to comment #6) > It seems the EFL bots are still broken from this regression. > (during ~500 revisions and 11 days) > > Is there any plan to fix them to be able catch new regressions again? Yes, EFL bots have broken too long time. I couldn't take a look this problem due to my various works. Let me try to fix it as soon as possible.
Almost media tests are being passed on EFL buildbot. However, unfortunately I failed to find what revision fixed this issue. To maintain those test on EFL port again, I unskip it by r173864. http://trac.webkit.org/changeset/173864
(In reply to comment #8) > Almost media tests are being passed on EFL buildbot. However, unfortunately I failed to find what revision fixed this issue. To maintain those test on EFL port again, I unskip it by r173864. > > http://trac.webkit.org/changeset/173864 Not at all, all media tests still fail on the EFL bot and this unskipping made the runtime from 22mins to 3.5hours and the number of failing test from 30-40 to 300+. :-/
(In reply to comment #9) > (In reply to comment #8) > > Almost media tests are being passed on EFL buildbot. However, unfortunately I failed to find what revision fixed this issue. To maintain those test on EFL port again, I unskip it by r173864. > > > > http://trac.webkit.org/changeset/173864 > > Not at all, all media tests still fail on the EFL bot and this > unskipping made the runtime from 22mins to 3.5hours and the > number of failing test from 30-40 to 300+. :-/ Ossy, something is weird. When I test it locally, there were dozens of failing tests. Let me check it again.
Created attachment 239541 [details] Patch
Brent, Eric, Jer, could one of you review this? One thing I am noticing is that if the setPseudo calls are now moved after the elements are completely constructed, we could possibly go back to calling shadowPseudoId() rather than separately hardcoding each ID. I suspect the reason that didn’t work before is that we were calling it during construction.
I think it would be helpful if someone would state what the fix is meant to do. It looks like the issue is that the 'setPseudo' call in the constructor causes an error when the m_innerText of a TextFieldInputType class is accessed. Apparently this does not yet exist during construction time. It seems strange to have to set these attributes of the class after construction, but I guess it's no worse than the existing need to call 'setType' or 'ensureUserAgentShadowRoot' after the object is constructed. This seems like clunky design, but I'm not sure how to address it at this point.
(In reply to comment #13) > I think it would be helpful if someone would state what the fix is meant to do. > > It looks like the issue is that the 'setPseudo' call in the constructor causes an error when the m_innerText of a TextFieldInputType class is accessed. Apparently this does not yet exist during construction time. Yeah, if you're calling a virtual method (Element::attributeChanged()) from a constructor you're going to have a bad time. (<meme>) > It seems strange to have to set these attributes of the class after construction, but I guess it's no worse than the existing need to call 'setType' or 'ensureUserAgentShadowRoot' after the object is constructed. > > This seems like clunky design, but I'm not sure how to address it at this point. I don't think there's a better way.
(In reply to comment #12) > Brent, Eric, Jer, could one of you review this? > > One thing I am noticing is that if the setPseudo calls are now moved after the elements are completely constructed, we could possibly go back to calling shadowPseudoId() rather than separately hardcoding each ID. I suspect the reason that didn’t work before is that we were calling it during construction. Someone added a FIXME to shadowPseudoId() asking clients not to overload it. So I don't think we should go back to using it instead.
Comment on attachment 239541 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=239541&action=review > Source/WebCore/ChangeLog:9 > + No new tests: fixes existing tests. > + The pseudo id cannot be set before the elements get a type. The patch is not necessarily wrong, but the changelog is unsatisfactory. Here you should explain exactly why the crash occur and why this is the right fix for it.
The media tests are still skipped. Is EFL port interested in fixing this old regression?
(In reply to comment #17) > The media tests are still skipped. Is EFL port interested in fixing this old > regression? Now media tests are passed on EFL port, except for 25 tests. I update this result in r182657 - http://trac.webkit.org/changeset/182657.