|
Description
Simon Fraser (smfr)
2011-01-18 21:48:26 PST
*** Bug 53836 has been marked as a duplicate of this bug. *** 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.
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). > > 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? (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. 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? My new implementation of length can store an int or float. It should behave the same for integer content. 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. Created attachment 82556 [details]
Working patch with fixes for 2 layout tests
I have also have a test file that shows the fix but I'm unsure where it should go. Created attachment 82558 [details]
Working patch with fixes for 2 layout tests
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. (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. You can just ditch rawValue also instead of keeping it. It has no meaning if the value isn't being hacked. 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. Created attachment 83031 [details]
new patch that fixes the style + removes setRawValue calls
(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. Created attachment 83572 [details]
split Length calculation in 2 seperate functions as not to disturb non-transform parameters
Created attachment 83725 [details]
removed bit specifiers in Length structure
Created attachment 83831 [details]
moved 'unsigned char' to 'bool' type
Comment on attachment 83831 [details]
moved 'unsigned char' to 'bool' type
r=me
Comment on attachment 83831 [details] moved 'unsigned char' to 'bool' type Clearing flags on attachment: 83831 Committed r79784: <http://trac.webkit.org/changeset/79784> All reviewed patches have been landed. Closing bug. 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 ?? 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 ... (In reply to comment #25) > This commit is causing a lot of crashes in the gtk debug bots: > I meant, assertion failures sorry. http://trac.webkit.org/changeset/79784 might have broken GTK Linux 32-bit Debug This patch is causing assertion failures on Windows. See bug 55321. 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. Reopening since this change is getting rolled out. 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 :-)
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.
Created attachment 84588 [details]
Updated patch with version that doesn't assert in debug mode + fixed style
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.
Created attachment 84591 [details]
Updated patch with version that doesn't assert in debug mode + fixed style
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.
Created attachment 84594 [details]
Updated patch with version that doesn't assert in debug mode + fixed style
Comment on attachment 84594 [details]
Updated patch with version that doesn't assert in debug mode + fixed style
r=me
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 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. The commit-queue encountered the following flaky tests while processing attachment 84594 [details]: inspector/audits/audits-panel-functional.html bug 55776 (authors: apavlov@chromium.org, pfeldman@chromium.org, and rniwa@webkit.org) The commit-queue is continuing to process your patch. 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> All reviewed patches have been landed. Closing bug. http://trac.webkit.org/changeset/80379 might have broken Leopard Intel Release (Build) Created attachment 85254 [details]
latest fix for leopard
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. Comment on attachment 85254 [details] latest fix for leopard Clearing flags on attachment: 85254 Committed r80806: <http://trac.webkit.org/changeset/80806> All reviewed patches have been landed. Closing bug. Please see bug 56275, this might have broken transitions/default-timing-function.html |