Bug 52699 – Allow Fixed Length values to be floating point

• Component: CSS
• Status: RESOLVED FIXED

[Simon Fraser (smfr)]

All Fixed Length values are currently truncated to ints. This is an issue for transforms, where the Length arguments to the various transform functions should really be floating point. Since we also interpolate through Lengths, this means that interpolated values are rounded to integers too.

[Rik Cabanier]

*** Bug 53836 has been marked as a duplicate of this bug. ***

[Rik Cabanier]

Created attachment 81737 [details]
Extends the Length class so it can store floating point values.

The Length class now stores 8 bits for floating point and 16 for integer. This changes loses the range of values that this class takes and gives a limited range for floating. Another way of fixing this would be to add a union, but this would increase the memory footprint.

[Simon Fraser (smfr)]

Comment on attachment 81737 [details]
Extends the Length class so it can store floating point values.

View in context: https://bugs.webkit.org/attachment.cgi?id=81737&action=review

> Source/WebCore/ChangeLog:7
> +        No new tests. However, this fix can introduce differences in existing tests since it can make a small difference in layout

In that case the patch needs to include new results for all the affected tests.

This changelog entry needs to mention the changes in Length behavior.

> Source/WebCore/platform/Length.h:33
> -const int percentScaleFactor = 128;
> -const int intMaxForLength = 0x7ffffff; // max value for a 28-bit int
> -const int intMinForLength = (-0x7ffffff - 1); // min value for a 28-bit int
> +const int intMaxForLength = 0x7ffff; // max value for a 20-bit int
> +const int intMinForLength = (-0x7ffff - 1); // min value for a 20-bit int

I don't think it's acceptable to change the max and min int values that Length can store. That might break websites (which do things like z-index: 999999999).

[Rik Cabanier]

> 
> In that case the patch needs to include new results for all the affected tests.
Will do so

> 
> This changelog entry needs to mention the changes in Length behavior.
> 
> > Source/WebCore/platform/Length.h:33
> > -const int percentScaleFactor = 128;
> > -const int intMaxForLength = 0x7ffffff; // max value for a 28-bit int
> > -const int intMinForLength = (-0x7ffffff - 1); // min value for a 28-bit int
> > +const int intMaxForLength = 0x7ffff; // max value for a 20-bit int
> > +const int intMinForLength = (-0x7ffff - 1); // min value for a 20-bit int
> 
> I don't think it's acceptable to change the max and min int values that Length can store. That might break websites (which do things like z-index: 999999999).

That doesn't work today either. Length can store at most 134217472 which is less than 999999999. The class also doesn't do bounds checking, so the larger value will overflow and become garbage.

It seems that making it a full int or float is the correct solution. Is it acceptable that the class takes up 4 more bytes?

[Simon Fraser (smfr)]

(In reply to comment #4)
> > 
> > In that case the patch needs to include new results for all the affected tests.
> Will do so
> 
> > 
> > This changelog entry needs to mention the changes in Length behavior.
> > 
> > > Source/WebCore/platform/Length.h:33
> > > -const int percentScaleFactor = 128;
> > > -const int intMaxForLength = 0x7ffffff; // max value for a 28-bit int
> > > -const int intMinForLength = (-0x7ffffff - 1); // min value for a 28-bit int
> > > +const int intMaxForLength = 0x7ffff; // max value for a 20-bit int
> > > +const int intMinForLength = (-0x7ffff - 1); // min value for a 20-bit int
> > 
> > I don't think it's acceptable to change the max and min int values that Length can store. That might break websites (which do things like z-index: 999999999).
> 
> That doesn't work today either. Length can store at most 134217472 which is less than 999999999. The class also doesn't do bounds checking, so the larger value will overflow and become garbage.

Bounds checking should happen elsewhere (e.g. in CSSPrimitiveValue methods).

> It seems that making it a full int or float is the correct solution. Is it acceptable that the class takes up 4 more bytes?

I don't know; we pass Lengths by value all over the place.

[Rik Cabanier]

999).
> > 
> > That doesn't work today either. Length can store at most 134217472 which is less than 999999999. The class also doesn't do bounds checking, so the larger value will overflow and become garbage.
> 
> Bounds checking should happen elsewhere (e.g. in CSSPrimitiveValue methods).
> 
I'll check there too...
> > It seems that making it a full int or float is the correct solution. Is it acceptable that the class takes up 4 more bytes?
> 
> I don't know; we pass Lengths by value all over the place.

I wasn't thinking of stack space. Is that a concern?
It was more about heap space if a lot of length objects are stored.

I've reimplemented my changes and the browser seems to render content fine, but I can't get a clean layout test. I reverted all my changes and tests are still failing.

Do I need to do more than sync everything and get the fonts?

[Rik Cabanier]

My new implementation of length can store an int or float. 
It should behave the same for integer content.

[Rik Cabanier]

I think the falures are because I don't have the right fonts on my system.
I'll try to get them installed or start developing on my mac.

[Rik Cabanier]

Created attachment 82556 [details]
Working patch with fixes for 2 layout tests

[Rik Cabanier]

I have also have a test file that shows the fix but I'm unsure where it should go.

[Rik Cabanier]

Created attachment 82558 [details]
Working patch with fixes for 2 layout tests

[Dave Hyatt]

I would just make the Length always be a float and not have it be both.  Also the bitfield is completely useless now if you use the full 32 bits for the number (it's not saving you any space), so you can just turn those into bools.

Lots of style errors too.  You might want to flag for review just so you can see the style errors and fix them.

[Rik Cabanier]

(In reply to comment #12)
> I would just make the Length always be a float and not have it be both.  Also the bitfield is completely useless now if you use the full 32 bits for the number (it's not saving you any space), so you can just turn those into bools.
> 
> Lots of style errors too.  You might want to flag for review just so you can see the style errors and fix them.

I was trying to minimize changes to the code. By having it behave differently only when the object is constructed with a float, we keep existing behavior.

My first attempt was to move to just float, but it actually changes the result of a large number of operations. It resulted in a lot of changes all over the code and numerous problems and differences in the layout tests.
Float also has less significant digits than int...

The bitfield is actually still necessary because part of the table logic multiplies by 128. If the value was maxint, it results in overflows which cause incorrect layout.

I will look at the style errors.

[Dave Hyatt]

You can just ditch rawValue also instead of keeping it.  It has no meaning if the value isn't being hacked.

[Dave Hyatt]

I guess the union is fine for now.  The engine's eventually going to move to floats, though, but if you want to keep ints in the underlying storage temporarily I suppose that's fine.

[Rik Cabanier]

Created attachment 83031 [details]
new patch that fixes the style + removes setRawValue calls

[Rik Cabanier]

(In reply to comment #14)
> You can just ditch rawValue also instead of keeping it.  It has no meaning if the value isn't being hacked.

I removed the setRawValue calls. However, I didn't remove the getRawValue because parts of the code use this to extract percentage values in fixed notation.

[Rik Cabanier]

Created attachment 83572 [details]
split Length calculation in 2 seperate functions as not to disturb non-transform parameters

[Rik Cabanier]

Created attachment 83725 [details]
removed bit specifiers in Length structure

[Rik Cabanier]

Created attachment 83831 [details]
moved 'unsigned char' to 'bool' type

[Dave Hyatt]

Comment on attachment 83831 [details]
moved 'unsigned char' to 'bool' type

r=me

[WebKit Commit Bot]

Comment on attachment 83831 [details]
moved 'unsigned char' to 'bool' type

Clearing flags on attachment: 83831

Committed r79784: <http://trac.webkit.org/changeset/79784>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Pavel Feldman]

Since recently, I am seeing assertion failure upon opening Web Inspector's network panel:

(filed https://bugs.webkit.org/show_bug.cgi?id=55301 to track it).

#0	0x1015d9687 in WebCore::Length::setValue at Length.h:91
#1	0x1019104ab in WebCore::FixedTableLayout::calcWidthArray at FixedTableLayout.cpp:121
#2	0x10191095f in WebCore::FixedTableLayout::computePreferredLogicalWidths at FixedTableLayout.cpp:204
#3	0x1020794d3 in WebCore::RenderTable::computePreferredLogicalWidths at RenderTable.cpp:574
#4	0x101f8c035 in WebCore::RenderBox::minPreferredLogicalWidth at RenderBox.cpp:659
#5	0x10207b2dc in WebCore::RenderTable::computeLogicalWidth at RenderTable.cpp:219
#6	0x10207a5f4 in WebCore::RenderTable::layout at RenderTable.cpp:278
#7	0x101f5ed92 in WebCore::RenderBlock::layoutBlockChild at RenderBlock.cpp:1958
#8	0x101f6093f in WebCore::RenderBlock::layoutBlockChildren at RenderBlock.cpp:1896
#9	0x101f60fb4 in WebCore::RenderBlock::layoutBlock at RenderBlock.cpp:1224
#10	0x101f5fc6e in WebCore::RenderBlock::layout at RenderBlock.cpp:1120
#11	0x101f83337 in WebCore::RenderObject::layoutIfNeeded at RenderObject.h:520
#12	0x101f5df34 in WebCore::RenderBlock::layoutPositionedObjects at RenderBlock.cpp:2137
#13	0x101f6128f in WebCore::RenderBlock::layoutBlock at RenderBlock.cpp:1255
#14	0x101f5fc6e in WebCore::RenderBlock::layout at RenderBlock.cpp:1120
#15	0x101f83337 in WebCore::RenderObject::layoutIfNeeded at RenderObject.h:520
#16	0x101f5df34 in WebCore::RenderBlock::layoutPositionedObjects at RenderBlock.cpp:2137
#17	0x101f6128f in WebCore::RenderBlock::layoutBlock at RenderBlock.cpp:1255
#18	0x101f5fc6e in WebCore::RenderBlock::layout at RenderBlock.cpp:1120
#19	0x101f83337 in WebCore::RenderObject::layoutIfNeeded at RenderObject.h:520
#20	0x101f5df34 in WebCore::RenderBlock::layoutPositionedObjects at RenderBlock.cpp:2137
#21	0x101f6128f in WebCore::RenderBlock::layoutBlock at RenderBlock.cpp:1255
#22	0x101f5fc6e in WebCore::RenderBlock::layout at RenderBlock.cpp:1120
#23	0x101f83337 in WebCore::RenderObject::layoutIfNeeded at RenderObject.h:520
#24	0x101f5df34 in WebCore::RenderBlock::layoutPositionedObjects at RenderBlock.cpp:2137
#25	0x101f6128f in WebCore::RenderBlock::layoutBlock at RenderBlock.cpp:1255
#26	0x101f5fc6e in WebCore::RenderBlock::layout at RenderBlock.cpp:1120
#27	0x101f83337 in WebCore::RenderObject::layoutIfNeeded at RenderObject.h:520
#28	0x101f5df34 in WebCore::RenderBlock::layoutPositionedObjects at RenderBlock.cpp:2137
#29	0x101f5fe50 in WebCore::RenderBlock::layoutOnlyPositionedObjects at RenderBlock.cpp:2087
#30	0x101f60a4e in WebCore::RenderBlock::layoutBlock at RenderBlock.cpp:1135
#31	0x101f5fc6e in WebCore::RenderBlock::layout at RenderBlock.cpp:1120
#32	0x101f83337 in WebCore::RenderObject::layoutIfNeeded at RenderObject.h:520
#33	0x101f5df34 in WebCore::RenderBlock::layoutPositionedObjects at RenderBlock.cpp:2137
#34	0x101f5fe50 in WebCore::RenderBlock::layoutOnlyPositionedObjects at RenderBlock.cpp:2087
#35	0x101f60a4e in WebCore::RenderBlock::layoutBlock at RenderBlock.cpp:1135
#36	0x101f5fc6e in WebCore::RenderBlock::layout at RenderBlock.cpp:1120
#37	0x101f83337 in WebCore::RenderObject::layoutIfNeeded at RenderObject.h:520
#38	0x101f5df34 in WebCore::RenderBlock::layoutPositionedObjects at RenderBlock.cpp:2137
#39	0x101f5fe50 in WebCore::RenderBlock::layoutOnlyPositionedObjects at RenderBlock.cpp:2087
#40	0x101f60a4e in WebCore::RenderBlock::layoutBlock at RenderBlock.cpp:1135
#41	0x101f5fc6e in WebCore::RenderBlock::layout at RenderBlock.cpp:1120
#42	0x1020b6fc3 in WebCore::RenderView::layout at RenderView.cpp:130
#43	0x101966cfc in WebCore::FrameView::layout at FrameView.cpp:906
#44	0x1017a5ef9 in WebCore::Document::updateLayout at Document.cpp:1592
#45	0x1017a85b8 in WebCore::Document::updateLayoutIgnorePendingStylesheets at Document.cpp:1623
#46	0x1018cd774 in WebCore::Element::focus at Element.cpp:1498
#47	0x101c3e412 in WebCore::jsElementPrototypeFunctionFocus at JSElement.cpp:1761
#48	0x279468c001b8 in ??
#49	0x1007e21f7 in JSC::JITCode::execute at JITCode.h:77
#50	0x1007dcc56 in JSC::Interpreter::executeCall at Interpreter.cpp:851
#51	0x10079632d in JSC::call at CallData.cpp:38
#52	0x100847b89 in JSC::JSObject::put at JSObject.cpp:149
#53	0x100815fbd in JSC::JSValue::put at JSObject.h:780
#54	0x10080f0ab in cti_op_put_by_id at JITStubs.cpp:1351
#55	0x100803dcd in WTF::doubleHash at HashTable.h:447
#56	0x1007e21f7 in JSC::JITCode::execute at JITCode.h:77
#57	0x1007dcc56 in JSC::Interpreter::executeCall at Interpreter.cpp:851
#58	0x10079632d in JSC::call at CallData.cpp:38
#59	0x100847b89 in JSC::JSObject::put at JSObject.cpp:149
#60	0x100815fbd in JSC::JSValue::put at JSObject.h:780
#61	0x10080f0ab in cti_op_put_by_id at JITStubs.cpp:1351
#62	0x100803dcd in WTF::doubleHash at HashTable.h:447
#63	0x1007e21f7 in JSC::JITCode::execute at JITCode.h:77
#64	0x1007dcc56 in JSC::Interpreter::executeCall at Interpreter.cpp:851
#65	0x10079632d in JSC::call at CallData.cpp:38
#66	0x1022d2f41 in WebCore::JSMainThreadExecState::call at JSMainThreadExecState.h:48
#67	0x101c50447 in WebCore::JSEventListener::handleEvent at JSEventListener.cpp:123
#68	0x1018ed642 in WebCore::EventTarget::fireEventListeners at EventTarget.cpp:354
#69	0x1018edc71 in WebCore::EventTarget::fireEventListeners at EventTarget.cpp:323
#70	0x101ec7d8f in WebCore::Node::handleLocalEvents at Node.cpp:2543
#71	0x1018d78d6 in WebCore::EventContext::handleLocalEvents at EventContext.cpp:48
#72	0x101ecff58 in WebCore::Node::dispatchGenericEvent at Node.cpp:2694
#73	0x101ed0379 in WebCore::Node::dispatchEvent at Node.cpp:2612
#74	0x101eceb58 in WebCore::Node::dispatchMouseEvent at Node.cpp:2902
#75	0x101ecf183 in WebCore::Node::dispatchMouseEvent at Node.cpp:2799
#76	0x1018df9af in WebCore::EventHandler::dispatchMouseEvent at EventHandler.cpp:1910
#77	0x1018e0f90 in WebCore::EventHandler::handleMouseReleaseEvent at EventHandler.cpp:1623
#78	0x1018e6fd5 in WebCore::EventHandler::mouseUp at EventHandlerMac.mm:546
#79	0x100ff8b3c in -[WebHTMLView mouseUp:] at WebHTMLView.mm:3764
#80	0x7fff8568d7ed in -[NSWindow sendEvent:]
#81	0x100041bf5 in ??
#82	0x100041b81 in ??
#83	0x7fff855c2ee2 in -[NSApplication sendEvent:]
#84	0x1000388c2 in ??
#85	0x7fff85559922 in -[NSApplication run]
#86	0x7fff855525f8 in NSApplicationMain
#87	0x100009b18 in ??

[Alejandro G. Castro]

This commit is causing a lot of crashes in the gtk debug bots:

http://webkit-bots.igalia.com/amd64/svn_79817.core-when_1298824044-_-who_DumpRenderTree-_-why_11.trace.html

Thread 1 (Thread 17474):
#0  0x00007fde3407987e in WebCore::Length::setValue (this=0x1b54620, t=WebCore::Percent, value=2304) at ../../Source/WebCore/platform/Length.h:91
#1  0x00007fde3407e4fd in WebCore::FixedTableLayout::calcWidthArray (this=0x188c830) at ../../Source/WebCore/rendering/FixedTableLayout.cpp:121
#2  0x00007fde3407e9b7 in WebCore::FixedTableLayout::computePreferredLogicalWidths (this=0x188c830, minWidth=@0x1b3e578, maxWidth=@0x1b3e57c) at ../../Source/WebCore/rendering/FixedTableLayout.cpp:204
#3  0x00007fde3417b05e in WebCore::RenderTable::computePreferredLogicalWidths (this=0x1b3e518) at ../../Source/WebCore/rendering/RenderTable.cpp:574
#4  0x00007fde340ec421 in WebCore::RenderBox::minPreferredLogicalWidth (this=0x1b3e518) at ../../Source/WebCore/rendering/RenderBox.cpp:659
#5  0x00007fde34178e77 in WebCore::RenderTable::computeLogicalWidth (this=0x1b3e518) at ../../Source/WebCore/rendering/RenderTable.cpp:219
#6  0x00007fde3417951f in WebCore::RenderTable::layout (this=0x1b3e518) at ../../Source/WebCore/rendering/RenderTable.cpp:278
#7  0x00007fde340aaeea in WebCore::RenderBlock::layoutBlockChild (this=0x194a5a8, child=0x1b3e518, marginInfo=..., previousFloatLogicalBottom=@0x7fffbb9d376c, maxFloatLogicalBottom=@0x7fffbb9d38c4) at ../../Source/WebCore/rendering/RenderBlock.cpp:1961
#8  0x00007fde340aaaff in WebCore::RenderBlock::layoutBlockChildren (this=0x194a5a8, relayoutChildren=true, maxFloatLogicalBottom=@0x7fffbb9d38c4) at ../../Source/WebCore/rendering/RenderBlock.cpp:1899
#9  0x00007fde340a7a80 in WebCore::RenderBlock::layoutBlock (this=0x194a5a8, relayoutChildren=true, pageLogicalHeight=0) at ../../Source/WebCore/rendering/RenderBlock.cpp:1226
#10 0x00007fde340a7382 in WebCore::RenderBlock::layout (this=0x194a5a8) at ../../Source/WebCore/rendering/RenderBlock.cpp:1122
#11 0x00007fde340aaeea in WebCore::RenderBlock::layoutBlockChild (this=0x1b0b3a8, child=0x194a5a8, marginInfo=..., previousFloatLogicalBottom=@0x7fffbb9d3b7c, maxFloatLogicalBottom=@0x7fffbb9d3cd4) at ../../Source/WebCore/rendering/RenderBlock.cpp:1961
#12 0x00007fde340aaaff in WebCore::RenderBlock::layoutBlockChildren (this=0x1b0b3a8, relayoutChildren=false, maxFloatLogicalBottom=@0x7fffbb9d3cd4) at ../../Source/WebCore/rendering/RenderBlock.cpp:1899
#13 0x00007fde340a7a80 in WebCore::RenderBlock::layoutBlock (this=0x1b0b3a8, relayoutChildren=false, pageLogicalHeight=0) at ../../Source/WebCore/rendering/RenderBlock.cpp:1226
#14 0x00007fde340a7382 in WebCore::RenderBlock::layout (this=0x1b0b3a8) at ../../Source/WebCore/rendering/RenderBlock.cpp:1122
#15 0x00007fde340c001d in WebCore::RenderObject::layoutIfNeeded (this=0x1b0b3a8) at ../../Source/WebCore/rendering/RenderObject.h:520
#16 0x00007fde340abacc in WebCore::RenderBlock::layoutPositionedObjects (this=0x1801048, relayoutChildren=false) at ../../Source/WebCore/rendering/RenderBlock.cpp:2140
#17 0x00007fde340ab7a4 in WebCore::RenderBlock::layoutOnlyPositionedObjects (this=0x1801048) at ../../Source/WebCore/rendering/RenderBlock.cpp:2090
#18 0x00007fde340a74b1 in WebCore::RenderBlock::layoutBlock (this=0x1801048, relayoutChildren=false, pageLogicalHeight=0) at ../../Source/WebCore/rendering/RenderBlock.cpp:1137
#19 0x00007fde340a7382 in WebCore::RenderBlock::layout (this=0x1801048) at ../../Source/WebCore/rendering/RenderBlock.cpp:1122
#20 0x00007fde340c001d in WebCore::RenderObject::layoutIfNeeded (this=0x1801048) at ../../Source/WebCore/rendering/RenderObject.h:520
#21 0x00007fde340abacc in WebCore::RenderBlock::layoutPositionedObjects (this=0xc03988, relayoutChildren=false) at ../../Source/WebCore/rendering/RenderBlock.cpp:2140
#22 0x00007fde340ab7a4 in WebCore::RenderBlock::layoutOnlyPositionedObjects (this=0xc03988) at ../../Source/WebCore/rendering/RenderBlock.cpp:2090
#23 0x00007fde340a74b1 in WebCore::RenderBlock::layoutBlock (this=0xc03988, relayoutChildren=false, pageLogicalHeight=0) at ../../Source/WebCore/rendering/RenderBlock.cpp:1137
#24 0x00007fde340a7382 in WebCore::RenderBlock::layout (this=0xc03988) at ../../Source/WebCore/rendering/RenderBlock.cpp:1122
#25 0x00007fde340c001d in WebCore::RenderObject::layoutIfNeeded (this=0xc03988) at ../../Source/WebCore/rendering/RenderObject.h:520
#26 0x00007fde340abacc in WebCore::RenderBlock::layoutPositionedObjects (this=0x1d6d698, relayoutChildren=false) at ../../Source/WebCore/rendering/RenderBlock.cpp:2140
#27 0x00007fde340ab7a4 in WebCore::RenderBlock::layoutOnlyPositionedObjects (this=0x1d6d698) at ../../Source/WebCore/rendering/RenderBlock.cpp:2090
#28 0x00007fde340a74b1 in WebCore::RenderBlock::layoutBlock (this=0x1d6d698, relayoutChildren=false, pageLogicalHeight=0) at ../../Source/WebCore/rendering/RenderBlock.cpp:1137
#29 0x00007fde340a7382 in WebCore::RenderBlock::layout (this=0x1d6d698) at ../../Source/WebCore/rendering/RenderBlock.cpp:1122
#30 0x00007fde340c001d in WebCore::RenderObject::layoutIfNeeded (this=0x1d6d698) at ../../Source/WebCore/rendering/RenderObject.h:520
#31 0x00007fde340abacc in WebCore::RenderBlock::layoutPositionedObjects (this=0x1d6cff8, relayoutChildren=false) at ../../Source/WebCore/rendering/RenderBlock.cpp:2140
#32 0x00007fde340ab7a4 in WebCore::RenderBlock::layoutOnlyPositionedObjects (this=0x1d6cff8) at ../../Source/WebCore/rendering/RenderBlock.cpp:2090
#33 0x00007fde340a74b1 in WebCore::RenderBlock::layoutBlock (this=0x1d6cff8, relayoutChildren=false, pageLogicalHeight=0) at ../../Source/WebCore/rendering/RenderBlock.cpp:1137
#34 0x00007fde340a7382 in WebCore::RenderBlock::layout (this=0x1d6cff8) at ../../Source/WebCore/rendering/RenderBlock.cpp:1122
#35 0x00007fde340c001d in WebCore::RenderObject::layoutIfNeeded (this=0x1d6cff8) at ../../Source/WebCore/rendering/RenderObject.h:520
#36 0x00007fde340abacc in WebCore::RenderBlock::layoutPositionedObjects (this=0x1d61198, relayoutChildren=false) at ../../Source/WebCore/rendering/RenderBlock.cpp:2140
...

[Alejandro G. Castro]

(In reply to comment #25)
> This commit is causing a lot of crashes in the gtk debug bots:
> 

I meant, assertion failures sorry.

[WebKit Review Bot]

http://trac.webkit.org/changeset/79784 might have broken GTK Linux 32-bit Debug

[Adam Roben (:aroben)]

This patch is causing assertion failures on Windows. See bug 55321.

[Adam Roben (:aroben)]

Since the assertion failures have been happening for many days now, and a fix is not imminent, we're rolling out the change. Bug 55386 tracks the rollout.

[Adam Roben (:aroben)]

Reopening since this change is getting rolled out.

[Rik Cabanier]

Created attachment 84327 [details]
Updated patch with version that doesn't assert in debug mode

This time I verified that the layout tests also pass in debug mode :-)

[WebKit Review Bot]

Attachment 84327 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/tran..." exit_code: 1

Source/WebCore/css/CSSStyleSelector.cpp:5590:  l is incorrectly named. Don't use the single letter 'l' as an identifier name.  [readability/naming] [4]
Total errors found: 1 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Rik Cabanier]

Created attachment 84588 [details]
Updated patch with version that doesn't assert in debug mode + fixed style

[WebKit Review Bot]

Attachment 84588 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/tran..." exit_code: 1

Source/WebCore/css/CSSStyleSelector.cpp:5672:  l is incorrectly named. Don't use the single letter 'l' as an identifier name.  [readability/naming] [4]
Total errors found: 1 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Rik Cabanier]

Created attachment 84591 [details]
Updated patch with version that doesn't assert in debug mode + fixed style

[WebKit Review Bot]

Attachment 84591 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/tran..." exit_code: 1

Source/WebCore/css/CSSStyleSelector.cpp:5672:  l is incorrectly named. Don't use the single letter 'l' as an identifier name.  [readability/naming] [4]
Total errors found: 1 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Rik Cabanier]

Created attachment 84594 [details]
Updated patch with version that doesn't assert in debug mode + fixed style

[Dave Hyatt]

Comment on attachment 84594 [details]
Updated patch with version that doesn't assert in debug mode + fixed style

r=me

[WebKit Commit Bot]

Comment on attachment 84594 [details]
Updated patch with version that doesn't assert in debug mode + fixed style

Rejecting attachment 84594 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-8', 'build-..." exit_code: 2

Last 500 characters of output:
.......................................................................................................................................................................................................................
inspector ...........
inspector/audits .
inspector/audits/audits-panel-functional.html -> failed

Exiting early after 1 failures. 12760 tests run.
307.03s total testing time

12759 test cases (99%) succeeded
1 test case (<1%) had incorrect layout
8 test cases (<1%) had stderr output

Full output: http://queues.webkit.org/results/8087715

[Pavel Feldman]

Comment on attachment 84594 [details]
Updated patch with version that doesn't assert in debug mode + fixed style

> inspector/audits/audits-panel-functional.html -> failed

I am working on this flaky test, should be robust in 24 hours.

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 84594 [details]:

inspector/audits/audits-panel-functional.html bug 55776 (authors: [e-mail], [e-mail], and [e-mail])
The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

Comment on attachment 84594 [details]
Updated patch with version that doesn't assert in debug mode + fixed style

Clearing flags on attachment: 84594

Committed r80379: <http://trac.webkit.org/changeset/80379>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[WebKit Review Bot]

http://trac.webkit.org/changeset/80379 might have broken Leopard Intel Release (Build)

[Rik Cabanier]

Created attachment 85254 [details]
latest fix for leopard

[Tony Gentilcore]

I verified that attachment 85254 [details] is identical to attachment 84594 [details] which dhyatt r+'d except for a new static_cast<float> to fix the SL build. Since I rolled this back in https://bugs.webkit.org/show_bug.cgi?id=55799, I'm r+'ing to get this going again.

[WebKit Commit Bot]

Comment on attachment 85254 [details]
latest fix for leopard

Clearing flags on attachment: 85254

Committed r80806: <http://trac.webkit.org/changeset/80806>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Tony Gentilcore]

Please see bug 56275, this might have broken transitions/default-timing-function.html