Summary: | CSS: Slow parsing of rgb() with percent values | ||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Eric Seidel (no email) <eric> | ||||||||||||||||||||
Component: | CSS | Assignee: | Andras Becsi <abecsi> | ||||||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||||||
Severity: | Normal | CC: | abecsi, darin, dglazkov, excors, jberlin, jon, kling, mdelaney7, oliver, ossy, psolanki, webkit.review.bot | ||||||||||||||||||||
Priority: | P2 | Keywords: | Performance | ||||||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||||||||
Hardware: | All | ||||||||||||||||||||||
OS: | All | ||||||||||||||||||||||
URL: | http://canvex.lazyilluminati.com/misc/3d.html | ||||||||||||||||||||||
Bug Depends on: | 46591 | ||||||||||||||||||||||
Bug Blocks: | |||||||||||||||||||||||
Attachments: |
|
Description
Eric Seidel (no email)
2008-01-02 05:52:18 PST
I only see 7.6fps on TOT. I took a shark sample (which I can send to anyone who cares). High level: 13.5% 13.5% WebCore WebCore::CanvasRenderingContext2D::fill() 5.9% 5.9% JavaScriptCore KJS::BracketAccessorNode::evaluateToNumber(KJS::ExecState*) 5.6% 5.6% WebCore WebCore::ScrollView::updateContents(WebCore::IntRect const&, bool) 3.2% 3.2% JavaScriptCore kjs_dtoa 2.9% 2.9% JavaScriptCore WTF::fastMalloc(unsigned long) 2.5% 2.5% JavaScriptCore KJS::ArrayInstance::getOwnPropertySlot(KJS::ExecState*, unsigned, KJS::PropertySlot&) 2.4% 2.4% JavaScriptCore kjs_strtod 2.4% 2.4% WebCore WebCore::CanvasStyle::applyFillColor(WebCore::GraphicsContext*) 2.2% 2.2% JavaScriptCore WTF::fastFree(void*) 2.0% 2.0% WebCore cssyyparse(void*) 1.9% 1.9% WebCore WebCore::CSSParser::lex() 1.9% 1.9% JavaScriptCore KJS::AddNode::evaluate(KJS::ExecState*) 1.9% 1.9% JavaScriptCore quorem 1.6% 1.6% JavaScriptCore diff 1.6% 1.6% JavaScriptCore KJS::ActivationImp::~ActivationImp [in-charge]() 1.6% 1.6% JavaScriptCore KJS::ArrayInstance::mark() 1.5% 1.5% JavaScriptCore void* KJS::Collector::heapAllocate<(KJS::Collector::HeapType)1>(unsigned long) 1.4% 1.4% JavaScriptCore KJS::LocalVarAccessNode::evaluate(KJS::ExecState*) 1.3% 1.3% JavaScriptCore KJS::MultNode::evaluateToNumber(KJS::ExecState*) 1.3% 1.3% JavaScriptCore KJS::FunctionBodyNode::execute(KJS::ExecState*) 1.2% 1.2% JavaScriptCore unsigned long KJS::Collector::sweep<(KJS::Collector::HeapType)1>(bool) 1.1% 1.1% WebCore WebCore::HTMLCanvasElement::paint(WebCore::GraphicsContext*, WebCore::IntRect const&) 1.0% 1.0% WebCore WebCore::ScrollView::visibleContentRect() const 1.0% 1.0% JavaScriptCore KJS::NumberImp::toNumber(KJS::ExecState*) const 0.9% 0.9% WebCore WebCore::CSSParser::text(int*) 0.9% 0.9% JavaScriptCore KJS::AssignBracketNode::evaluate(KJS::ExecState*) 0.9% 0.9% JavaScriptCore void* KJS::Collector::heapAllocate<(KJS::Collector::HeapType)0>(unsigned long) 0.9% 0.9% JavaScriptCore KJS::ImmediateNumberNode::evaluate(KJS::ExecState*) 0.8% 0.8% JavaScriptCore KJS::BracketAccessorNode::evaluate(KJS::ExecState*) 0.8% 0.8% WebCore WebCore::Path::clear() 0.8% 0.8% JavaScriptCore WTF::fastRealloc(void*, unsigned long) 0.7% 0.7% JavaScriptCore KJS::ArrayInstance::put(KJS::ExecState*, unsigned, KJS::JSValue*, int) 0.7% 0.7% WebKit -[WebHTMLView visibleRect] 0.7% 0.7% JavaScriptCore unsigned long KJS::Collector::sweep<(KJS::Collector::HeapType)0>(bool) 0.7% 0.7% JavaScriptCore KJS::JSObject::toObject(KJS::ExecState*) const 0.7% 0.7% JavaScriptCore WTF::TCMalloc_Central_FreeList::RemoveRange(void**, void**, int*) 0.7% 0.7% WebCore WebCore::ScrollView::getDocumentView() const 0.7% 0.7% JavaScriptCore KJS::AddNumbersNode::evaluate(KJS::ExecState*) 0.7% 0.7% WebCore WebCore::DeprecatedStringData::makeAscii() 0.6% 0.6% JavaScriptCore KJS::jsNumberCell(double) 0.6% 0.6% JavaScriptCore KJS::ExecState::ExecState[not-in-charge](KJS::JSGlobalObject*, KJS::JSObject*, KJS::FunctionBodyNode*, KJS::ExecState*, KJS::FunctionImp*, KJS::List const&) 0.6% 0.6% JavaScriptCore KJS::UString::UString[not-in-charge](KJS::UString const&, KJS::UString const&) 0.5% 0.5% JavaScriptCore KJS::FunctionCallResolveNode::evaluate(KJS::ExecState*) 0.5% 0.5% JavaScriptCore KJS::AddNumbersNode::evaluateToNumber(KJS::ExecState*) 0.5% 0.5% WebCore WebCore::CSSParser::setupParser(char const*, WebCore::String const&, char const*) 0.5% 0.5% JavaScriptCore KJS::ExprStatementNode::execute(KJS::ExecState*) 0.5% 0.5% JavaScriptCore KJS::Lookup::findEntry(KJS::HashTable const*, KJS::Identifier const&) 0.5% 0.5% JavaScriptCore KJS::FunctionCallDotNode::evaluate(KJS::ExecState*) 0.5% 0.5% JavaScriptCore KJS::MultNode::evaluate(KJS::ExecState*) 0.5% 0.5% JavaScriptCore KJS::PropertyMap::mark() const 0.4% 0.4% JavaScriptCore KJS::FunctionImp::callAsFunction(KJS::ExecState*, KJS::JSObject*, KJS::List const&) 0.4% 0.4% JavaScriptCore KJS::DivNode::evaluate(KJS::ExecState*) 0.4% 0.4% JavaScriptCore KJS::ArgumentListNode::evaluateList(KJS::ExecState*, KJS::List&) 0.4% 0.4% WebCore WebCore::StringImpl::lower() const 0.4% 0.4% JavaScriptCore KJS::LocalVarAccessNode::evaluateToNumber(KJS::ExecState*) 0.4% 0.4% JavaScriptCore KJS::BlockNode::execute(KJS::ExecState*) 0.4% 0.4% JavaScriptCore KJS::AddNode::evaluateToNumber(KJS::ExecState*) 0.4% 0.4% JavaScriptCore pow5mult 0.3% 0.3% JavaScriptCore WTF::TCMalloc_Central_FreeList::FetchFromSpansSafe() 0.3% 0.3% WebCore WebCore::CSSParser::lex(void*) 0.3% 0.3% JavaScriptCore KJS::StringImp::~StringImp [in-charge]() 0.3% 0.3% JavaScriptCore KJS::AssignLocalVarNode::evaluate(KJS::ExecState*) 0.3% 0.3% JavaScriptCore KJS::VarStatementNode::execute(KJS::ExecState*) 0.3% 0.3% WebCore WebCore::GraphicsContext::fillRect(WebCore::IntRect const&, WebCore::Color const&) 0.3% 0.3% JavaScriptCore WTF::TCMalloc_Central_FreeList::ReleaseListToSpans(void*) 0.3% 0.3% WebCore WebCore::CanvasRenderingContext2D::fillRect(float, float, float, float, int&) 0.3% 0.3% JavaScriptCore KJS::SubNode::evaluate(KJS::ExecState*) 0.3% 0.3% JavaScriptCore KJS::ActivationImp::getOwnPropertySlot(KJS::ExecState*, KJS::Identifier const&, KJS::PropertySlot&) 0.3% 0.3% WebCore WebCore::RenderView::repaintViewRectangle(WebCore::IntRect const&, bool) 0.3% 0.3% JavaScriptCore d2b 0.3% 0.3% JavaScriptCore KJS::JSValue::toFloat(KJS::ExecState*) const 0.3% 0.3% JavaScriptCore KJS::ElementNode::evaluate(KJS::ExecState*) 0.3% 0.3% JavaScriptCore KJS::compareWithCompareFunctionForQSort(void const*, void const*) 0.2% 0.2% WebCore WebCore::String::String[not-in-charge](KJS::UString const&) 0.2% 0.2% JavaScriptCore KJS::ForNode::execute(KJS::ExecState*) 0.2% 0.2% WebCore void WTF::deleteAllValues<WebCore::ValueList*, WTF::HashTable<int, int, WTF::IdentityExtractor<int>, WTF::IntHash<int>, WTF::HashTraits<int>, WTF::HashTraits<int> > const>(WTF::HashTable<int, int, WTF::IdentityExtractor<int>, WTF::IntHash<int>, WTF::HashTraits<int>, WTF::HashTraits<int> > const&) 0.2% 0.2% JavaScriptCore KJS::PreIncLocalVarNode::evaluate(KJS::ExecState*) 0.2% 0.2% JavaScriptCore KJS::ArrayInstance::sort(KJS::ExecState*, KJS::JSObject*) 0.2% 0.2% JavaScriptCore KJS::UString::UString[not-in-charge](char const*) 0.2% 0.2% JavaScriptCore KJS::UString::from(double) 0.2% 0.2% JavaScriptCore KJS::NumberImp::toString(KJS::ExecState*) const 0.2% 0.2% JavaScriptCore KJS::ArrayInstance::~ArrayInstance [not-in-charge]() 0.2% 0.2% WebCore void WTF::deleteAllValues<WebCore::Function*, WTF::HashTable<int, int, WTF::IdentityExtractor<int>, WTF::IntHash<int>, WTF::HashTraits<int>, WTF::HashTraits<int> > const>(WTF::HashTable<int, int, WTF::IdentityExtractor<int>, WTF::IntHash<int>, WTF::HashTraits<int>, WTF::HashTraits<int> > const&) 0.2% 0.2% JavaScriptCore KJS::Collector::markStackObjectsConservatively(void*, void*) 0.1% 0.1% WebCore WebCore::RenderBox::absoluteClippedOverflowRect() 0.1% 0.1% JavaScriptCore KJS::PropertyMap::get(KJS::Identifier const&) const 0.1% 0.1% JavaScriptCore KJS::NumberNode::evaluateToNumber(KJS::ExecState*) 0.1% 0.1% WebCore WebCore::RenderBox::computeAbsoluteRepaintRect(WebCore::IntRect&, bool) 0.1% 0.1% JavaScriptCore KJS::NumberImp::type() const 0.1% 0.1% JavaScriptCore KJS::JSCell::mark() 0.1% 0.1% WebCore WebCore::JSDOMWindow::customGetOwnPropertySlot(KJS::ExecState*, KJS::Identifier const&, KJS::PropertySlot&) 0.1% 0.1% WebCore WebCore::CSSParser::parseColorFromValue(WebCore::Value*, unsigned&, bool) 0.1% 0.1% WebCore std::pair<WTF::HashTableIterator<int, int, WTF::IdentityExtractor<int>, WTF::IntHash<int>, WTF::HashTraits<int>, WTF::HashTraits<int> >, bool> WTF::HashTable<int, int, WTF::IdentityExtractor<int>, WTF::IntHash<int>, WTF::HashTraits<int>, WTF::HashTraits<int> >::add<WebCore::ValueList*, WebCore::ValueList*, WTF::HashSetTranslator<(bool)1, WebCore::ValueList*, WTF::HashTraits<WebCore::ValueList*>, WTF::HashTraits<int>, WTF::PtrHash<WebCore::ValueList*> > >(WebCore::ValueList* const&, WebCore::ValueList* const&) 0.1% 0.1% JavaScriptCore WTF::HashMap<WTF::RefPtr<KJS::UString::Rep>, unsigned long, KJS::IdentifierRepHash, KJS::IdentifierRepHashTraits, KJS::SymbolTableIndexHashTraits>::get(KJS::UString::Rep*) const 0.1% 0.1% WebCore WebCore::JSDOMWindow::getOwnPropertySlot(KJS::ExecState*, KJS::Identifier const&, KJS::PropertySlot&) 0.1% 0.1% WebCore WebCore::JSCanvasRenderingContext2DPrototypeFunctionFill::callAsFunction(KJS::ExecState*, KJS::JSObject*, KJS::List const&) 0.1% 0.1% WebCore findProp(char const*, unsigned) 0.1% 0.1% WebCore WebCore::ValueList::~ValueList [not-in-charge]() 0.1% 0.1% JavaScriptCore KJS::ReturnNode::execute(KJS::ExecState*) 0.1% 0.1% JavaScriptCore KJS::PropertyMap::~PropertyMap [not-in-charge]() 0.1% 0.1% WebCore WebCore::RenderObject::repaint(bool) 0.1% 0.1% WebCore WebCore::DeprecatedString::isAllASCII() const 0.1% 0.1% JavaScriptCore KJS::StringImp::toString(KJS::ExecState*) const 0.1% 0.1% WebCore WebCore::CSSParser::parseColorParameters(WebCore::Value*, int*, bool) 0.1% 0.1% JavaScriptCore KJS::LessEqNode::evaluateToBoolean(KJS::ExecState*) 0.1% 0.1% WebCore WTF::HashTableIterator<int, int, WTF::IdentityExtractor<int>, WTF::IntHash<int>, WTF::HashTraits<int>, WTF::HashTraits<int> > WTF::HashTable<int, int, WTF::IdentityExtractor<int>, WTF::IntHash<int>, WTF::HashTraits<int>, WTF::HashTraits<int> >::find<int, WTF::IdentityHashTranslator<int, int, WTF::IntHash<int> > >(int const&) 0.1% 0.1% WebCore WebCore::toHTMLCanvasStyle(KJS::ExecState*, KJS::JSValue*) 0.1% 0.1% WebCore WebCore::DeprecatedString::~DeprecatedString [not-in-charge]() 0.1% 0.1% WebCore WebCore::CSSParser::~CSSParser [not-in-charge]() 0.1% 0.1% JavaScriptCore KJS::JSObject::type() const 0.1% 0.1% JavaScriptCore WTF::fastZeroedMalloc(unsigned long) 0.1% 0.1% WebCore WebCore::JSCanvasRenderingContext2D::putValueProperty(KJS::ExecState*, int, KJS::JSValue*, int) 0.1% 0.1% WebCore WebCore::Frame::settings() const 0.1% 0.1% WebCore WebCore::CanvasRenderingContext2D::setFillStyle(WTF::PassRefPtr<WebCore::CanvasStyle>) 0.1% 0.1% JavaScriptCore KJS::AssignDotNode::evaluate(KJS::ExecState*) 0.1% 0.1% WebCore WebCore::Widget::getView() const 0.1% 0.1% WebCore WebCore::CSSParser::parseValue(int, bool) 0.1% 0.1% WebCore KJS::Window::allowsAccessFrom(KJS::JSGlobalObject const*) const 0.1% 0.1% JavaScriptCore KJS::ExecState::lexicalGlobalObject() const 0.1% 0.1% WebCore WebCore::RenderView::printing() const 0.1% 0.1% WebCore WebCore::RenderObject::container() const 0.1% 0.1% JavaScriptCore KJS::PropertyMap::getLocation(KJS::Identifier const&) 0.1% 0.1% JavaScriptCore KJS::jsString(KJS::UString const&) 0.1% 0.1% JavaScriptCore KJS::JSObject::mark() 0.1% 0.1% JavaScriptCore KJS::ArrayInstance::getOwnPropertySlot(KJS::ExecState*, KJS::Identifier const&, KJS::PropertySlot&) 0.1% 0.1% WebCore WebCore::JSCanvasRenderingContext2D::getOwnPropertySlot(KJS::ExecState*, KJS::Identifier const&, KJS::PropertySlot&) 0.1% 0.1% JavaScriptCore KJS::Collector::markProtectedObjects() 0.1% 0.1% WebCore WebCore::Document::settings() const 0.1% 0.1% WebCore WebCore::CSSParser::parseColor(WebCore::String const&, unsigned&, bool) 0.1% 0.1% JavaScriptCore KJS::MathProtoFuncSqrt::callAsFunction(KJS::ExecState*, KJS::JSObject*, KJS::List const&) 0.1% 0.1% JavaScriptCore KJS::LessNode::evaluateToBoolean(KJS::ExecState*) 0.1% 0.1% JavaScriptCore KJS::JSGlobalObject::getOwnPropertySlot(KJS::ExecState*, KJS::Identifier const&, KJS::PropertySlot&) 0.1% 0.1% JavaScriptCore KJS::FunctionCallDotNode::evaluateToNumber(KJS::ExecState*) 0.1% 0.1% WebCore WebCore::JSCanvasRenderingContext2DPrototypeFunctionBeginPath::callAsFunction(KJS::ExecState*, KJS::JSObject*, KJS::List const&) 0.1% 0.1% WebCore WebCore::JSCanvasRenderingContext2DPrototype::getOwnPropertySlot(KJS::ExecState*, KJS::Identifier const&, KJS::PropertySlot&) 0.1% 0.1% WebCore WebCore::DeprecatedString::toDouble(bool*) const 0.1% 0.1% WebCore WebCore::DeprecatedString::DeprecatedString[not-in-charge](WebCore::DeprecatedChar const*, unsigned) 0.1% 0.1% WebCore WebCore::Color::setNamedColor(WebCore::String const&) 0.1% 0.1% JavaScriptCore KJS::DotAccessorNode::evaluate(KJS::ExecState*) 0.1% 0.1% JavaScriptCore WTF::TCMalloc_Central_FreeList::InsertRange(void*, void*, int) 0.1% 0.1% WebCore WebCore::String::length() const 0.1% 0.1% WebCore WebCore::Path::addLineTo(WebCore::FloatPoint const&) 0.1% 0.1% WebCore WebCore::CSSParser::CSSParser[not-in-charge](bool) 0.1% 0.1% WebCore std::pair<WTF::HashTableIterator<int, int, WTF::IdentityExtractor<int>, WTF::IntHash<int>, WTF::HashTraits<int>, WTF::HashTraits<int> >, bool> WTF::HashTable<int, int, WTF::IdentityExtractor<int>, WTF::IntHash<int>, WTF::HashTraits<int>, WTF::HashTraits<int> >::add<WebCore::Function*, WebCore::Function*, WTF::HashSetTranslator<(bool)1, WebCore::Function*, WTF::HashTraits<WebCore::Function*>, WTF::HashTraits<int>, WTF::PtrHash<WebCore::Function*> > >(WebCore::Function* const&, WebCore::Function* const&) 0.1% 0.1% WebCore KJS::staticFunctionGetter(KJS::ExecState*, KJS::JSObject*, KJS::Identifier const&, KJS::PropertySlot const&) 0.1% 0.1% JavaScriptCore KJS::jsOwnedString(KJS::UString const&) 0.1% 0.1% JavaScriptCore KJS::FunctionImp::mark() 0.1% 0.1% JavaScriptCore KJS::ArrayNode::evaluate(KJS::ExecState*) 0.1% 0.1% JavaScriptCore KJS::ArrayInstance::ArrayInstance[not-in-charge](KJS::JSObject*, KJS::List const&) 0.1% 0.1% WebCore WebCore::String::deprecatedString() const 0.1% 0.1% WebCore WebCore::ParseString::lower() 0.1% 0.1% WebCore WebCore::CSSPrimitiveValue::~CSSPrimitiveValue [in-charge deleting]() 0.1% 0.1% JavaScriptCore KJS::AddStringsNode::evaluate(KJS::ExecState*) 0.1% 0.1% WebKit -[WebHTMLView drawSingleRect:] 0.1% 0.1% WebCore WebCore::String::lower() const 0.1% 0.1% WebCore WebCore::setSharedTimerFireTime(double) 0.1% 0.1% WebCore WebCore::RenderReplaced::overflowRect(bool) const 0.1% 0.1% WebCore WebCore::DeprecatedStringData::DeprecatedStringData[not-in-charge]() 0.1% 0.1% WebCore WebCore::CSSParser::validUnit(WebCore::Value*, WebCore::CSSParser::Units, bool) 0.1% 0.1% WebCore WebCore::CSSParser::parseColor(unsigned&, WebCore::String const&, bool) 0.1% 0.1% WebCore WebCore::CanvasRenderingContext2D::clearPathForDashboardBackwardCompatibilityMode() 0.1% 0.1% JavaScriptCore KJS::ArrayObjectImp::construct(KJS::ExecState*, KJS::List const&) 0.1% 0.1% JavaScriptCore KJS::ActivationImp::mark() 0.1% 0.1% JavaScriptCore Balloc 0.0% 0.0% WebCore WTF::Vector<WebCore::Value, (unsigned long)16>::shrink(unsigned long) 0.0% 0.0% WebCore WTF::HashTable<int, int, WTF::IdentityExtractor<int>, WTF::IntHash<int>, WTF::HashTraits<int>, WTF::HashTraits<int> >::expand() 0.0% 0.0% WebCore WebCore::JSCanvasRenderingContext2D::put(KJS::ExecState*, KJS::Identifier const&, KJS::JSValue*, int) 0.0% 0.0% WebCore WebCore::JSCanvasRenderingContext2D::classInfo() const 0.0% 0.0% WebCore WebCore::HTMLCanvasElement::drawingContext() const 0.0% 0.0% WebCore WebCore::DeprecatedStringData::initialize(WebCore::DeprecatedChar const*, unsigned) 0.0% 0.0% WebCore WebCore::DeprecatedString::makeSharedNullHandle() 0.0% 0.0% JavaScriptCore KJS::InternalFunctionImp::implementsCall() const 0.0% 0.0% WebCore WTF::HashTable<int, int, WTF::IdentityExtractor<int>, WTF::IntHash<int>, WTF::HashTraits<int>, WTF::HashTraits<int> >::rehash(int) 0.0% 0.0% WebCore WebCore::StyleBase::stylesheet() 0.0% 0.0% WebCore WebCore::String::String[in-charge](unsigned short const*, unsigned) 0.0% 0.0% WebCore WebCore::RenderObject::view() const 0.0% 0.0% WebCore WebCore::RenderBlock::isBlockFlow() const 0.0% 0.0% WebCore WebCore::JSCanvasRenderingContext2DPrototypeFunctionMoveTo::callAsFunction(KJS::ExecState*, KJS::JSObject*, KJS::List const&) 0.0% 0.0% WebCore WebCore::JSCanvasRenderingContext2DPrototypeFunctionLineTo::callAsFunction(KJS::ExecState*, KJS::JSObject*, KJS::List const&) 0.0% 0.0% WebCore WebCore::DeprecatedStringData::DeprecatedStringData[in-charge]() 0.0% 0.0% WebCore WebCore::CSSParser::sinkFloatingValueList(WebCore::ValueList*) 0.0% 0.0% WebCore WebCore::CSSParser::parseColor(WebCore::CSSMutableStyleDeclaration*, WebCore::String const&) 0.0% 0.0% JavaScriptCore KJS::StringImp::type() const 0.0% 0.0% JavaScriptCore KJS::ReadModifyLocalVarNode::evaluate(KJS::ExecState*) 0.0% 0.0% JavaScriptCore KJS::AddStringLeftNode::evaluate(KJS::ExecState*) 0.0% 0.0% WebKit -[WebHTMLView(WebPrivate) _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] 0.0% 0.0% WebCore WebCore::StringImpl::~StringImpl [not-in-charge]() 0.0% 0.0% WebCore WebCore::StringImpl::StringImpl[in-charge](unsigned short const*, unsigned) 0.0% 0.0% WebCore WebCore::RenderBox::borderBox() const 0.0% 0.0% WebCore WebCore::Frame::ownerElement() const 0.0% 0.0% WebCore WebCore::DeprecatedString::toFloat(bool*) const 0.0% 0.0% WebCore WebCore::CSSStyleDeclaration::CSSStyleDeclaration[not-in-charge](WebCore::CSSRule*) 0.0% 0.0% WebCore WebCore::CSSParser::parseColor(WebCore::Value*) 0.0% 0.0% WebCore WebCore::CanvasRenderingContext2D::lineTo(float, float) 0.0% 0.0% JavaScriptCore KJS::ArrayInstance::~ArrayInstance [in-charge]() 0.0% 0.0% WebCore WebCore::Document::ownerElement() const 0.0% 0.0% WebCore WebCore::CSSPrimitiveValue::cleanup() 0.0% 0.0% WebCore WebCore::CSSParser::createFloatingValueList() 0.0% 0.0% WebCore KJS::JSGlobalObject::isGlobalObject() const 0.0% 0.0% JavaScriptCore KJS::ArrayInstance::lengthGetter(KJS::ExecState*, KJS::JSObject*, KJS::Identifier const&, KJS::PropertySlot const&) 0.0% 0.0% WebCore WebCore::String::String[not-in-charge](unsigned short const*, unsigned) 0.0% 0.0% WebCore WebCore::JSDOMWindow::impl() const 0.0% 0.0% WebCore WebCore::equal(WebCore::StringImpl const*, char const*) 0.0% 0.0% WebCore WebCore::DeprecatedString::DeprecatedString[not-in-charge]() 0.0% 0.0% WebCore WebCore::CSSPrimitiveValue::cssValueType() const 0.0% 0.0% WebCore WebCore::CSSMutableStyleDeclaration::~CSSMutableStyleDeclaration [in-charge deleting]() 0.0% 0.0% WebCore WebCore::CanvasRenderingContext2D::drawingContext() const 0.0% 0.0% WebCore KJS::ScriptInterpreter::markDOMNodesForDocument(WebCore::Document*) 0.0% 0.0% JavaScriptCore KJS::NumberNode::evaluate(KJS::ExecState*) 0.0% 0.0% WebCore WTF::HashSet<WebCore::ValueList*, WTF::PtrHash<WebCore::ValueList*>, WTF::HashTraits<WebCore::ValueList*> >::add(WebCore::ValueList* const&) 0.0% 0.0% WebCore WebCore::HTMLCanvasElement::willDraw(WebCore::FloatRect const&) 0.0% 0.0% WebCore WebCore::GraphicsContext::platformContext() const 0.0% 0.0% WebCore WebCore::FrameView::repaintRectangle(WebCore::IntRect const&, bool) 0.0% 0.0% WebCore WebCore::deprecatedString(WebCore::ParseString const&) 0.0% 0.0% WebCore WebCore::CSSParser::clearProperties() 0.0% 0.0% WebCore WebCore::CSSParser::addProperty(int, WTF::PassRefPtr<WebCore::CSSValue>, bool) 0.0% 0.0% JavaScriptCore KJS::JSVariableObject::mark() 0.0% 0.0% WebCore WTF::Vector<WTF::RefPtr<WebCore::CSSRuleList>, (unsigned long)0>::shrink(unsigned long) 0.0% 0.0% WebCore WTF::HashTable<int, int, WTF::IdentityExtractor<int>, WTF::IntHash<int>, WTF::HashTraits<int>, WTF::HashTraits<int> >::deallocateTable(int*, int) 0.0% 0.0% WebCore WebCore::StringImpl::init(unsigned short const*, unsigned) 0.0% 0.0% WebCore WebCore::RenderBox::height() const 0.0% 0.0% WebCore WebCore::FloatSize::FloatSize[not-in-charge](_NSSize const&) 0.0% 0.0% WebCore WebCore::DeprecatedValueListImpl::DeprecatedValueListImpl[not-in-charge](void (*)(WebCore::DeprecatedValueListImplNode*), WebCore::DeprecatedValueListImplNode* (*)(WebCore::DeprecatedValueListImplNode*)) 0.0% 0.0% WebCore WebCore::DeprecatedValueListImpl::DeprecatedValueListImpl[in-charge](void (*)(WebCore::DeprecatedValueListImplNode*), WebCore::DeprecatedValueListImplNode* (*)(WebCore::DeprecatedValueListImplNode*)) 0.0% 0.0% WebCore WebCore::DeprecatedString::DeprecatedString[in-charge](WebCore::DeprecatedChar const*, unsigned) 0.0% 0.0% WebCore WebCore::CSSParser::~CSSParser [in-charge]() 0.0% 0.0% WebCore WebCore::CSSMutableStyleDeclaration::CSSMutableStyleDeclaration[not-in-charge]() 0.0% 0.0% WebCore WebCore::CSSMutableStyleDeclaration::CSSMutableStyleDeclaration[in-charge]() 0.0% 0.0% WebCore WebCore::CanvasStyle::CanvasStyle[not-in-charge](WebCore::String const&) 0.0% 0.0% JavaScriptCore KJS::ResolveNode::evaluate(KJS::ExecState*) 0.0% 0.0% JavaScriptCore KJS::JSGlobalObject::mark() 0.0% 0.0% WebKit -[WebHTMLView isFlipped] 0.0% 0.0% WebCore WTF::Vector<WTF::RefPtr<WebCore::StyleBase>, (unsigned long)0>::shrink(unsigned long) 0.0% 0.0% JavaScriptCore WTF::TCMalloc_Central_FreeList::ShrinkCache(int, bool) 0.0% 0.0% WebCore WTF::HashSet<WebCore::Function*, WTF::PtrHash<WebCore::Function*>, WTF::HashTraits<WebCore::Function*> >::add(WebCore::Function* const&) 0.0% 0.0% WebCore WebCore::StyleBase::isStyleSheet() const 0.0% 0.0% WebCore WebCore::stopSharedTimer() 0.0% 0.0% WebCore WebCore::RenderObject::borderTopExtra() const 0.0% 0.0% WebCore WebCore::RenderFlow::hasColumns() const 0.0% 0.0% WebCore WebCore::IntRect::operator _NSRect() const 0.0% 0.0% WebCore WebCore::freeHandle(WebCore::DeprecatedStringData**) 0.0% 0.0% WebCore WebCore::FloatRect::FloatRect[not-in-charge](_NSRect const&) 0.0% 0.0% WebCore WebCore::FloatRect::FloatRect[in-charge](_NSRect const&) 0.0% 0.0% WebCore WebCore::DeprecatedStringData::~DeprecatedStringData [not-in-charge]() 0.0% 0.0% WebCore WebCore::DeprecatedStringData::~DeprecatedStringData [in-charge]() 0.0% 0.0% WebCore WebCore::CSSParser::sinkFloatingValue(WebCore::Value&) 0.0% 0.0% WebCore WebCore::CSSParser::createFloatingFunction() 0.0% 0.0% WebCore WebCore::CSSParser::checkForOrphanedUnits() 0.0% 0.0% WebCore void WTF::deleteAllValues<WebCore::CSSSelector*, WTF::HashTable<int, int, WTF::IdentityExtractor<int>, WTF::IntHash<int>, WTF::HashTraits<int>, WTF::HashTraits<int> > const>(WTF::HashTable<int, int, WTF::IdentityExtractor<int>, WTF::IntHash<int>, WTF::HashTraits<int>, WTF::HashTraits<int> > const&) 0.0% 0.0% WebCore KJS::JSObject::isActivationObject() 0.0% 0.0% WebKit -[WebClipView hasAdditionalClip] 0.0% 0.0% WebCore WTF::HashTable<int, int, WTF::IdentityExtractor<int>, WTF::IntHash<int>, WTF::HashTraits<int>, WTF::HashTraits<int> >::remove(int*) 0.0% 0.0% WebCore WebCore::String::String[in-charge](KJS::UString const&) 0.0% 0.0% WebCore WebCore::RenderView::computeAbsoluteRepaintRect(WebCore::IntRect&, bool) 0.0% 0.0% WebCore WebCore::RenderObject::isTable() const 0.0% 0.0% WebCore WebCore::Path::moveTo(WebCore::FloatPoint const&) 0.0% 0.0% WebCore WebCore::makeRGB(int, int, int) 0.0% 0.0% WebCore WebCore::JSCanvasRenderingContext2D::setFillStyle(KJS::ExecState*, KJS::JSValue*) 0.0% 0.0% WebCore WebCore::FloatRect::FloatRect[not-in-charge](CGRect const&) 0.0% 0.0% WebCore WebCore::DeprecatedValueListImpl::~DeprecatedValueListImpl [not-in-charge]() 0.0% 0.0% WebCore WebCore::DeprecatedValueListImpl::~DeprecatedValueListImpl [in-charge]() 0.0% 0.0% WebCore WebCore::DeprecatedString::~DeprecatedString [in-charge]() 0.0% 0.0% WebCore WebCore::CSSPrimitiveValue::CSSPrimitiveValue[in-charge](unsigned) 0.0% 0.0% WebCore WebCore::CanvasRenderingContext2D::willDraw(WebCore::FloatRect const&) 0.0% 0.0% JavaScriptCore KJS::StringNode::evaluate(KJS::ExecState*) 0.0% 0.0% WebKit -[WebHTMLView drawRect:] 0.0% 0.0% JavaScriptCore WTF::TCMalloc_PageHeap::IncrementalScavenge(unsigned long) 0.0% 0.0% WebCore WTF::HashTable<int, int, WTF::IdentityExtractor<int>, WTF::IntHash<int>, WTF::HashTraits<int>, WTF::HashTraits<int> >::allocateTable(int) 0.0% 0.0% WebCore WebCore::ScheduledAction::execute(KJS::Window*) 0.0% 0.0% WebCore WebCore::RenderView::isRenderView() const 0.0% 0.0% WebCore WebCore::RenderObject::borderBottomExtra() const 0.0% 0.0% WebCore WebCore::RenderBox::width() const 0.0% 0.0% WebCore WebCore::JSCanvasRenderingContext2DPrototypeFunctionSave::callAsFunction(KJS::ExecState*, KJS::JSObject*, KJS::List const&) 0.0% 0.0% WebCore WebCore::getPropertyID(char const*, int) 0.0% 0.0% WebCore WebCore::FloatRect::operator _NSRect() const 0.0% 0.0% WebCore WebCore::FloatRect::FloatRect[in-charge](CGRect const&) 0.0% 0.0% WebCore WebCore::FloatPoint::FloatPoint[not-in-charge](_NSPoint const&) 0.0% 0.0% WebCore WebCore::DeprecatedValueListImpl::Private::deleteList(WebCore::DeprecatedValueListImplNode*) 0.0% 0.0% WebCore WebCore::CSSPrimitiveValue::CSSPrimitiveValue[not-in-charge](unsigned) 0.0% 0.0% WebCore WebCore::CSSParser::CSSParser[in-charge](bool) 0.0% 0.0% WebCore WebCore::ContainerNode::virtualFirstChild() const 0.0% 0.0% WebCore WebCore::Color::parseHexColor(WebCore::String const&, unsigned&) 0.0% 0.0% WebCore WebCore::CanvasRenderingContext2D::moveTo(float, float) 0.0% 0.0% WebCore WebCore::CanvasRenderingContext2D::beginPath() 0.0% 0.0% JavaScriptCore KJS::NumberImp::toPrimitive(KJS::ExecState*, KJS::JSType) const 0.0% 0.0% JavaScriptCore KJS::MathObjectImp::getOwnPropertySlot(KJS::ExecState*, KJS::Identifier const&, KJS::PropertySlot&) 0.0% 0.0% JavaScriptCore KJS::ArrayInstance::compactForSorting() 0.0% 0.0% WebCore WebCore::ValueList::~ValueList [in-charge]() 0.0% 0.0% WebCore WebCore::timerFired(__CFRunLoopTimer*, void*) 0.0% 0.0% WebCore WebCore::StringImpl::~StringImpl [in-charge]() 0.0% 0.0% WebCore WebCore::StringImpl::StringImpl[not-in-charge](unsigned short const*, unsigned) 0.0% 0.0% WebCore WebCore::String::characters() const 0.0% 0.0% WebCore WebCore::setCGFillColor(CGContext*, WebCore::Color const&) 0.0% 0.0% WebCore WebCore::ScrollView::contentsHeight() const 0.0% 0.0% WebCore WebCore::RenderObject::hasControlClip() const 0.0% 0.0% WebCore WebCore::RenderFlow::paintLines(WebCore::RenderObject::PaintInfo&, int, int) 0.0% 0.0% WebCore WebCore::RenderBox::paintRootBoxDecorations(WebCore::RenderObject::PaintInfo&, int, int) 0.0% 0.0% WebCore WebCore::RenderBlock::overflowLeft(bool) const 0.0% 0.0% WebCore WebCore::RenderBlock::MarginInfo::MarginInfo[not-in-charge](WebCore::RenderBlock*, int, int) 0.0% 0.0% WebCore WebCore::Path::Path[not-in-charge](WebCore::Path const&) 0.0% 0.0% WebCore WebCore::Path::isEmpty() const 0.0% 0.0% WebCore WebCore::Node::virtualFirstChild() const 0.0% 0.0% WebCore WebCore::Node::traverseNextNode(WebCore::Node const*) const 0.0% 0.0% WebCore WebCore::JSNode::mark() 0.0% 0.0% WebCore WebCore::JSDocument::mark() 0.0% 0.0% WebCore WebCore::InlineBox::paint(WebCore::RenderObject::PaintInfo&, int, int) 0.0% 0.0% WebCore WebCore::HTMLTokenizer::write(WebCore::SegmentedString const&, bool) 0.0% 0.0% WebCore WebCore::FrameLoader::client() const 0.0% 0.0% WebCore WebCore::Frame::dragCaretController() const 0.0% 0.0% WebCore WebCore::Frame::document() const 0.0% 0.0% WebCore WebCore::Font::drawGlyphs(WebCore::GraphicsContext*, WebCore::FontData const*, WebCore::GlyphBuffer const&, int, int, WebCore::FloatPoint const&) const 0.0% 0.0% WebCore WebCore::FloatSize::FloatSize[in-charge](CGSize const&) 0.0% 0.0% WebCore WebCore::FloatSize::FloatSize[in-charge](_NSSize const&) 0.0% 0.0% WebCore WebCore::FloatPoint::FloatPoint[in-charge](CGPoint const&) 0.0% 0.0% WebCore WebCore::DeprecatedString::DeprecatedString[in-charge]() 0.0% 0.0% WebCore WebCore::CanvasStyle::CanvasStyle[in-charge](WebCore::String const&) 0.0% 0.0% JavaScriptCore KJS::NativeErrorImp::mark() 0.0% 0.0% JavaScriptCore KJS::MathProtoFuncSin::callAsFunction(KJS::ExecState*, KJS::JSObject*, KJS::List const&) 0.0% 0.0% JavaScriptCore KJS::JSWrapperObject::mark() 0.0% 0.0% JavaScriptCore KJS::JSLock::registerThread() 0.0% 0.0% JavaScriptCore KJS::FunctionImp::~FunctionImp [in-charge]() 0.0% 0.0% JavaScriptCore KJS::ExecState::mark() 0.0% 0.0% JavaScriptCore KJS::Collector::collect() 0.0% 0.0% WebKit core(WebFrame*) 0.0% 0.0% WebKit -[WebHTMLView(WebPrivate) viewWillDraw] 0.0% 0.0% WebKit -[WebHTMLView(WebHTMLViewFileInternal) _isTopHTMLView] 0.0% 0.0% WebKit -[WebHTMLView respondsToSelector:] 0.0% 0.0% WebKit -[WebFrameView documentView] 0.0% 0.0% WebKit -[_WebSafeForwarder forwardInvocation:] Indeed we are slow, but, well, by my measurements ToT is faster than firefox nightlies as of a couple of days ago, and Opera weeklies... After 2 years of inattention, we're up to 27fps on shipping Safari 5. Is that fast enough to close this bug out? I suspect we should sample this and get an up to date idea of where perf is being consumed, on my mac book pro this only gets 15-20fps. I am sure we can do better. The bottleneck here is CSSParser::parseColor()'s handling of rgb() colors with percent values. There's no fast-path for those so it goes through cssyyparse(). Circa 30% of runtime is spent doing this. Created attachment 85468 [details] proposed patch With QtWebKit trunk on my Core i5 Linux machine without the patch I get 23-24 fps, and 35-36 fps with it on http://canvex.lazyilluminati.com/misc/3d.html. In comparison on Chromium 9.0.597.107 I get 34-35 fps on Opera 11.01 I get 44-45 fps on this machine. Comment on attachment 85468 [details] proposed patch Quoth CSS ( http://www.w3.org/TR/css3-color/#rgb-def ): "The format of an RGB value in the functional notation is ‘rgb(’ followed by a comma-separated list of three numerical values (either three integer values or three percentage values) followed by ‘)’." (And the same is basically repeated for rgba().) Thus we should not allow mixing integer and percentage component values, i.e it should be either all integers, or all percentages. The in-tree test css2.1/t040306-syntax-01-f.html covers mixing of RGB component types, and should fail with this change. Attachment 85468 [details] did not build on mac: Build output: http://queues.webkit.org/results/8135486 Attachment 85468 [details] did not build on chromium: Build output: http://queues.webkit.org/results/8131530 Created attachment 85596 [details]
proposed patch
Do not allow mixing of percentage and integer values.
Thanks Andreas for pointing this out.
Attachment 85596 [details] did not build on mac: Build output: http://queues.webkit.org/results/8132729 Created attachment 85597 [details]
proposed patch
Fix the Mac build.
Attachment 85596 [details] did not build on chromium: Build output: http://queues.webkit.org/results/8141735 Comment on attachment 85597 [details] proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=85597&action=review I got excited about this patch, but I'm too tired to finish reviewing it. > Source/WebCore/css/CSSParser.cpp:3922 > + bytes[i] = string[i]; I would put a static_cast here to emphasize the change in type. Created attachment 85925 [details]
updated patch
Update patch to ToT and use static_cast for explicit type conversion in parseDoubleIfValid.
Thanks Adam for taking a look at it.
Comment on attachment 85925 [details]
updated patch
This patch looks correct to me, but I don't really know this code that well. I'd feel better if someone who knows the CSS parser better did the final review.
Created attachment 86172 [details]
updated patch
Comment on attachment 86172 [details] updated patch View in context: https://bugs.webkit.org/attachment.cgi?id=86172&action=review I think we need some more test cases beyond the ones added here. I don’t think the test cases cover all the branches in the code. For example, I don’t see any test cases with multiple periods and a percentage sign or with any other illegal characters. > Source/WebCore/css/CSSParser.cpp:3901 > +static inline int parseDoubleIfValid(const UChar* string, const UChar* end, UChar terminator, double& value) I think it’s unclear that this function result is the number of characters consumed. There should be a comment saying so. I don’t think it makes sense to mark this function inline. It’s called in three different places and it’s pretty long too. > Source/WebCore/css/CSSParser.cpp:3933 > + if (processedLength) { > + value = 0.0; > + bytes[processedLength] = static_cast<char>(terminator); > + bytes[processedLength + 1] = '\0'; > + char* foundTerminator; > + value = WTF::strtod(bytes.data(), &foundTerminator); > + return (*foundTerminator == static_cast<char>(terminator)) ? processedLength : 0; > + } > + return 0; This should use early return instead of putting all the code inside an if. It’s not a good idea to cast a UChar into a char. If the terminator is guaranteed to be an ASCII character, then it should be passed in to this function as a char, not a UChar. If it’s not, then this code won’t work. Does this code really need to use WTF::strtod? > Source/WebCore/css/CSSParser.cpp:3936 > +static inline bool parseColorIntOrPercentage(const UChar*& string, const UChar* end, UChar terminator, CSSPrimitiveValue::UnitTypes& expect, int& value) I don’t think it makes sense to mark a function inline that’s this long and called in so many places. > Source/WebCore/css/CSSParser.cpp:3939 > + double localValue = 0.0; We normally just say 0, not 0.0. > Source/WebCore/css/CSSParser.cpp:3982 > + localValue = localValue / 100.0 * 256.0; There’s an extra space here. Multiplying by 256 is wrong. You should multiply by nextafter(256, 0). > Source/WebCore/css/CSSParser.cpp:3984 > + if (localValue > 255) > + localValue = 255; This probably wouldn’t be necessary if we multiplied by the right value. Unless we allow percentage values over 100. > Source/WebCore/css/CSSParser.cpp:4034 > + double d; Can you use a word name for this local variable rather than just "d"? > Source/WebCore/css/CSSParser.cpp:4035 > + if (parseDoubleIfValid(string, end, terminator, d)) { This code does significant extra work that it didn’t before, including calling strtod. That doesn’t seem good for performance. > Source/WebCore/css/CSSParser.cpp:4062 > + double d = 0.0; > + if (parseDoubleIfValid(string, end, terminator, d)) { > + value = negative ? 0 : static_cast<int>(d * nextafter(256.0, 0.0)); > + string = end; > + return true; > + } > + return false; Can you use a word name for this local variable rather than just "d"? Not sure why you are initializing it. This should be early return style, not “nest the success case inside an if” style. Created attachment 86322 [details]
proposed patch v2
(In reply to comment #19) Thanks for the comprehensive review. > I think we need some more test cases beyond the ones added here. I don’t think the test cases cover all the branches in the code. For example, I don’t see any test cases with multiple periods and a percentage sign or with any other illegal characters. Added some cases to test consistency in case of illegal combinations. > > > Source/WebCore/css/CSSParser.cpp:3901 > > +static inline int parseDoubleIfValid(const UChar* string, const UChar* end, UChar terminator, double& value) > > I think it’s unclear that this function result is the number of characters consumed. There should be a comment saying so. Added comments. > > I don’t think it makes sense to mark this function inline. It’s called in three different places and it’s pretty long too. Removed inline. > It’s not a good idea to cast a UChar into a char. If the terminator is guaranteed to be an ASCII character, then it should be passed in to this function as a char, not a UChar. If it’s not, then this code won’t work. > > Does this code really need to use WTF::strtod? I think it is much more safe to use strtod than trying to implement it here. > > > Source/WebCore/css/CSSParser.cpp:3936 > > +static inline bool parseColorIntOrPercentage(const UChar*& string, const UChar* end, UChar terminator, CSSPrimitiveValue::UnitTypes& expect, int& value) > > I don’t think it makes sense to mark a function inline that’s this long and called in so many places. > Removed inline. > > Source/WebCore/css/CSSParser.cpp:3939 > > + double localValue = 0.0; > > We normally just say 0, not 0.0. > > > Source/WebCore/css/CSSParser.cpp:3982 > > + localValue = localValue / 100.0 * 256.0; > > There’s an extra space here. Multiplying by 256 is wrong. You should multiply by nextafter(256, 0). If I use nextafter here I get wrong values because the double is casted to int at the end. (For example for 50% using nextafter gives a value of 127 instead of 128) > > > Source/WebCore/css/CSSParser.cpp:3984 > > + if (localValue > 255) > > + localValue = 255; > > This probably wouldn’t be necessary if we multiplied by the right value. Unless we allow percentage values over 100. We need to check the value here because the percentageg need to be clamped to the 0-100 intervall to retain consistency. > > > Source/WebCore/css/CSSParser.cpp:4034 > > + double d; > > Can you use a word name for this local variable rather than just "d"? > > > Source/WebCore/css/CSSParser.cpp:4035 > > + if (parseDoubleIfValid(string, end, terminator, d)) { > > This code does significant extra work that it didn’t before, including calling strtod. That doesn’t seem good for performance. I changed the code to have a separate isValidDouble function. > > > Source/WebCore/css/CSSParser.cpp:4062 > > + double d = 0.0; > > + if (parseDoubleIfValid(string, end, terminator, d)) { > > + value = negative ? 0 : static_cast<int>(d * nextafter(256.0, 0.0)); > > + string = end; > > + return true; > > + } > > + return false; > > Can you use a word name for this local variable rather than just "d"? Not sure why you are initializing it. I need to initialize it because on chromium it fails to build without initializing the value, because of a compiler warning. Created attachment 86325 [details]
proposed patch v2
Using early return, in parseAlphaValue.
Created attachment 86326 [details]
proposed patch v2 with layout test
Mixing of percentage and numeric values is already covered by css2.1/t040306-syntax-01-f.html, as Andreas pointed out earlier, so the added cases should explicitly test invalid combinations.
Created attachment 86486 [details]
proposed patch v3
- Avoid using strtod, and add result of new test cases to the expected file.
(In reply to comment #24) > Created an attachment (id=86486) [details] > proposed patch v3 > > - Avoid using strtod, and add result of new test cases to the expected file. This patch still applies. Darin, could you take a look at this approach? Comment on attachment 86486 [details] proposed patch v3 View in context: https://bugs.webkit.org/attachment.cgi?id=86486&action=review > Source/WebCore/css/CSSParser.cpp:3904 > +// Returns the number of characters which form a valid double > +// and are terminated by the given terminator character > +static int isValidDouble(const UChar* string, const UChar* end, const char terminator) Given that this returns the number of characters, the function name should probably be something that doesn’t make it sound like it returns a boolean. > Source/WebCore/css/CSSParser.cpp:3964 > + unsigned scale = 1; > + > + while (position < length && scale < MAX_SCALE) { > + fraction = fraction * 10 + string[position++] - '0'; > + scale *= 10; > + } > + > + value = localValue + (fraction / static_cast<double>(scale)); It seems that "scale" should be a double here instead of unsigned. Comment on attachment 86486 [details]
proposed patch v3
Setting cq- to address Darins suggestions, landing by hand.
This change appears to have broken the Windows 7 Release Tests: http://build.webkit.org/builders/Windows%207%20Release%20%28Tests%29/builds/10932 http://build.webkit.org/results/Windows%207%20Release%20(Tests)/r82284%20(10933)/results.html And Snow Leopard: http://build.webkit.org/results/SnowLeopard%20Intel%20Release%20(Tests)/r82289%20(27421)/results.html Andras, are you going to fix the failing tests? I rolled out the original patch, the unreviewed fix and the chromium expected results: http://trac.webkit.org/changeset/82315 Andras can check this fail tomorrow. I'm investigating the roundig issue and relanding it with the fix later. Since we now calculate everything in double and cast to int at the very end, it seems that the value is rounded incorrectly. Thanks Ossy for taking care of the rollout. Re-landed in http://trac.webkit.org/changeset/82464. Closing bug. |