RESOLVED FIXED 105353
Minor improvements to HTMLMediaElement 
https://bugs.webkit.org/show_bug.cgi?id=105353
Summary Minor improvements to HTMLMediaElement
Joseph Pecoraro
Reported 2012-12-18 14:52:36 PST
While reading through HTMLMediaElement I noticed some cases where there could be string improvements: <http://trac.webkit.org/wiki/EfficientStrings> Patch to follow.
Attachments
[PATCH] Proposed Improvements (4.90 KB, patch)
2012-12-18 15:10 PST, Joseph Pecoraro 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Joseph Pecoraro
Comment 1 2012-12-18 14:53:33 PST
There was also a case (HTMLMediaElement::setLoop) which was doing extra work.
Joseph Pecoraro
Comment 2 2012-12-18 15:10:46 PST
Created attachment 180039 [details] [PATCH] Proposed Improvements
Joseph Pecoraro
Comment 3 2012-12-18 15:37:52 PST
This would be the path from setBooleanAttribute -> parseAttribute, which handles loopAttr: * thread #1: tid = 0x1c03, 0x000000011216bc42 WebCore`WebCore::HTMLMediaElement::parseAttribute(this=0x00007fa62523e500, name=0x00000001143cf820, value=0x0000000110fcb358) + 34 at HTMLMediaElement.cpp:372, stop reason = breakpoint 2.1 frame #0: 0x000000011216bc42 WebCore`WebCore::HTMLMediaElement::parseAttribute(this=0x00007fa62523e500, name=0x00000001143cf820, value=0x0000000110fcb358) + 34 at HTMLMediaElement.cpp:372 frame #1: 0x00000001121f06fb WebCore`WebCore::HTMLVideoElement::parseAttribute(this=0x00007fa62523e500, name=0x00000001143cf820, value=0x0000000110fcb358) + 379 at HTMLVideoElement.cpp:124 frame #2: 0x0000000111edb1e2 WebCore`WebCore::Element::attributeChanged(this=0x00007fa62523e500, name=0x00000001143cf820, newValue=0x0000000110fcb358) + 130 at Element.cpp:776 frame #3: 0x0000000112fea845 WebCore`WebCore::StyledElement::attributeChanged(this=0x00007fa62523e500, name=0x00000001143cf820, newValue=0x0000000110fcb358) + 181 at StyledElement.cpp:168 frame #4: 0x0000000111edfad9 WebCore`WebCore::Element::didAddAttribute(this=0x00007fa62523e500, name=0x00000001143cf820, value=0x0000000110fcb358) + 73 at Element.cpp:2497 frame #5: 0x0000000111edfa86 WebCore`WebCore::Element::addAttributeInternal(this=0x00007fa62523e500, name=0x00000001143cf820, value=0x0000000110fcb358, inSynchronizationOfLazyAttribute=NotInSynchronizationOfLazyAttribute) + 150 at Element.cpp:1706 frame #6: 0x0000000111ee481f WebCore`WebCore::Element::setAttributeInternal(this=0x00007fa62523e500, index=18446744073709551615, name=0x00000001143cf820, newValue=0x0000000110fcb358, inSynchronizationOfLazyAttribute=NotInSynchronizationOfLazyAttribute) + 127 at Element.cpp:731 frame #7: 0x0000000111ed961a WebCore`WebCore::Element::setAttribute(this=0x00007fa62523e500, name=0x00000001143cf820, value=0x0000000110fcb358) + 74 at Element.cpp:714 frame #8: 0x0000000111ed95ad WebCore`WebCore::Element::setBooleanAttribute(this=0x00007fa62523e500, name=0x00000001143cf820, value=true) + 61 at Element.cpp:288
Eric Carlson
Comment 4 2012-12-18 17:30:08 PST
Comment on attachment 180039 [details] [PATCH] Proposed Improvements Nice, thank you!
WebKit Review Bot
Comment 5 2012-12-18 18:15:10 PST
Comment on attachment 180039 [details] [PATCH] Proposed Improvements Clearing flags on attachment: 180039 Committed r138097: <http://trac.webkit.org/changeset/138097>
WebKit Review Bot
Comment 6 2012-12-18 18:15:13 PST
All reviewed patches have been landed. Closing bug.
Filip Pizlo
Comment 7 2012-12-18 19:28:29 PST
I think you guys just set my machine on fire: [pizlo@bigmac OpenSource] DYLD_FRAMEWORK_PATH=WebKitBuild/Debug/ WebKitBuild/Debug/DumpRenderTree LayoutTests/media/media-can-play-webm.html ASSERTION FAILED: m_length /Volumes/Data/pizlo/quartary/OpenSource/Source/WTF/wtf/text/StringImpl.h(252) : WTF::StringImpl::StringImpl(const char *, unsigned int, WTF::StringImpl::ConstructFromLiteralTag) 1 0x10ebcba44 WTF::StringImpl::StringImpl(char const*, unsigned int, WTF::StringImpl::ConstructFromLiteralTag) 2 0x10ebc9be9 WTF::StringImpl::StringImpl(char const*, unsigned int, WTF::StringImpl::ConstructFromLiteralTag) 3 0x10ebc1a16 WTF::StringImpl::createFromLiteral(char const*) 4 0x10ebd4840 WTF::String::String(WTF::ASCIILiteral) 5 0x10ebd47fd WTF::String::String(WTF::ASCIILiteral) 6 0x11060dd86 WebCore::HTMLMediaElement::canPlayType(WTF::String const&, WTF::String const&, WebCore::KURL const&) const 7 0x110a98029 WebCore::jsHTMLMediaElementPrototypeFunctionCanPlayType(JSC::ExecState*) 8 0x460869801265 9 0x10e8e8184 JSC::JITCode::execute(JSC::JSStack*, JSC::ExecState*, JSC::JSGlobalData*) 10 0x10e8e08a9 JSC::Interpreter::execute(JSC::EvalExecutable*, JSC::ExecState*, JSC::JSValue, JSC::JSScope*) 11 0x10e8dfff7 JSC::eval(JSC::ExecState*) 12 0x10eae7d75 llint_slow_path_call_eval 13 0x10eaf00a7 llint_op_call_eval 14 0x10e8e8184 JSC::JITCode::execute(JSC::JSStack*, JSC::ExecState*, JSC::JSGlobalData*) 15 0x10e8e53ff JSC::Interpreter::executeCall(JSC::ExecState*, JSC::JSObject*, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) 16 0x10e763a12 JSC::call(JSC::ExecState*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) 17 0x11089ad92 WebCore::JSMainThreadExecState::call(JSC::ExecState*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) 18 0x1109dc796 WebCore::JSEventListener::handleEvent(WebCore::ScriptExecutionContext*, WebCore::Event*) 19 0x1103cf363 WebCore::EventTarget::fireEventListeners(WebCore::Event*, WebCore::EventTargetData*, WTF::Vector<WebCore::RegisteredEventListener, 1ul>&) 20 0x1103cf135 WebCore::EventTarget::fireEventListeners(WebCore::Event*) 21 0x110319bd0 WebCore::DOMWindow::dispatchEvent(WTF::PassRefPtr<WebCore::Event>, WTF::PassRefPtr<WebCore::EventTarget>) 22 0x110320be8 WebCore::DOMWindow::dispatchLoadEvent() 23 0x11017a0bf WebCore::Document::dispatchWindowLoadEvent() 24 0x110177bf0 WebCore::Document::implicitClose() 25 0x11049572b WebCore::FrameLoader::checkCallImplicitClose() 26 0x1104953f3 WebCore::FrameLoader::checkCompleted() 27 0x110495595 WebCore::FrameLoader::loadDone() 28 0x10fec68e2 WebCore::CachedResourceLoader::loadDone(WebCore::CachedResource*) 29 0x1114eb28f WebCore::SubresourceLoader::releaseResources() 30 0x1112be579 WebCore::ResourceLoader::didFinishLoading(double) 31 0x1114eae75 WebCore::SubresourceLoader::didFinishLoading(double)
WebKit Review Bot
Comment 8 2012-12-18 21:37:39 PST
Re-opened since this is blocked by bug 105386
Joseph Pecoraro
Comment 9 2012-12-19 10:47:33 PST
Ahh, from the backtrace above it sounds like this was ASCIILiteral("") in HTMLMediaElement::canPlayType. StringImpl ends up doing strlen, gets a length of 0, and uses a constructor that asserts a non-0 length.
Oliver Hunt
Comment 10 2012-12-19 12:20:47 PST
Fixed the zero length ASCIILiteral bug in r138187 You should be able to roll back in now.
Joseph Pecoraro
Comment 11 2013-01-07 15:59:58 PST
Rolling back in after Oliver's change. Landed r139006: <http://trac.webkit.org/changeset/139006>
Note You need to log in before you can comment on or make changes to this bug.