Summary: | Remove LayoutTypes abstraction | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Emil A Eklund <eae> | ||||||||||||||
Component: | Platform | Assignee: | Emil A Eklund <eae> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | andersca, apinheiro, cfleizach, cmarcelo, dglazkov, dino, dmazzoni, d-r, eric.carlson, eric, feature-media-reviews, fmalita, gyuyoung.kim, jamesr, japhet, jdiggs, leviw, macpherson, menard, mifenton, noam, pdr, rakuco, schenney, simon.fraser, tkent, tonikitoo, webkit.review.bot | ||||||||||||||
Priority: | P2 | ||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||
OS: | Unspecified | ||||||||||||||||
Bug Depends on: | 93037 | ||||||||||||||||
Bug Blocks: | |||||||||||||||||
Attachments: |
|
Description
Emil A Eklund
2012-08-02 17:02:38 PDT
Created attachment 172394 [details]
Patch
Attachment 172394 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/CMakeLists.txt', u'Source/W..." exit_code: 1
Source/WebCore/platform/graphics/LayoutRect.h:215: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3]
Source/WebCore/platform/LayoutUnit.h:191: wtf_ceil is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
Source/WebCore/platform/graphics/LayoutRect.cpp:38: Use 'using namespace std;' instead of 'using std::max;'. [build/using_std] [4]
Source/WebCore/platform/graphics/LayoutRect.cpp:39: Use 'using namespace std;' instead of 'using std::min;'. [build/using_std] [4]
Source/WebCore/platform/graphics/LayoutBoxExtent.cpp:187: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3]
Source/WebCore/platform/graphics/LayoutBoxExtent.cpp:193: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3]
Total errors found: 6 in 125 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 172394 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=172394&action=review > Source/WebCore/rendering/RenderBox.cpp:366 > + return clientWidth() - min(LayoutUnit(0), layoutOverflowRect().x() - borderLeft()); Why not just LayoutUnit()? Comment on attachment 172394 [details] Patch Attachment 172394 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/14726747 Comment on attachment 172394 [details] Patch Attachment 172394 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/14675328 Comment on attachment 172394 [details] Patch Attachment 172394 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/14745206 Created attachment 172408 [details]
Patch
Attachment 172408 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/CMakeLists.txt', u'Source/W..." exit_code: 1
Source/WebCore/platform/graphics/LayoutRect.h:215: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3]
Source/WebCore/platform/LayoutUnit.h:191: wtf_ceil is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
Source/WebCore/platform/graphics/LayoutRect.cpp:38: Use 'using namespace std;' instead of 'using std::max;'. [build/using_std] [4]
Source/WebCore/platform/graphics/LayoutRect.cpp:39: Use 'using namespace std;' instead of 'using std::min;'. [build/using_std] [4]
Source/WebCore/platform/graphics/LayoutBoxExtent.cpp:187: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3]
Source/WebCore/platform/graphics/LayoutBoxExtent.cpp:193: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3]
Total errors found: 6 in 125 files
If any of these errors are false positives, please file a bug against check-webkit-style.
(In reply to comment #3) > (From update of attachment 172394 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=172394&action=review > > > Source/WebCore/rendering/RenderBox.cpp:366 > > + return clientWidth() - min(LayoutUnit(0), layoutOverflowRect().x() - borderLeft()); > > Why not just LayoutUnit()? No reason, expanded the ZERO_LAYOUT_UNIT define which was LayoutUnit(0). Changed to LayoutUnit(). Comment on attachment 172408 [details] Patch Attachment 172408 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/14682292 Comment on attachment 172408 [details] Patch Attachment 172408 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/14690297 Comment on attachment 172408 [details] Patch Attachment 172408 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/14728695 r133536 added a new dependency on LayoutTypes.h (which is why the mac and cr-linux bots fails), will upload a new patch momentarily. Created attachment 172430 [details]
Patch
Attachment 172430 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/CMakeLists.txt', u'Source/W..." exit_code: 1
Source/WebCore/platform/graphics/LayoutRect.h:215: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3]
Source/WebCore/platform/LayoutUnit.h:191: wtf_ceil is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
Source/WebCore/platform/graphics/LayoutRect.cpp:38: Use 'using namespace std;' instead of 'using std::max;'. [build/using_std] [4]
Source/WebCore/platform/graphics/LayoutRect.cpp:39: Use 'using namespace std;' instead of 'using std::min;'. [build/using_std] [4]
Source/WebCore/platform/graphics/LayoutBoxExtent.cpp:187: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3]
Source/WebCore/platform/graphics/LayoutBoxExtent.cpp:193: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3]
Total errors found: 6 in 126 files
If any of these errors are false positives, please file a bug against check-webkit-style.
The style failure is to be expected, the other bots should all pass assuming they run before anyone else adds a new dependency... Comment on attachment 172430 [details] Patch Attachment 172430 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/14755036 Comment on attachment 172430 [details] Patch Attachment 172430 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/14665446 Comment on attachment 172430 [details] Patch Attachment 172430 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/14717942 New failing tests: fast/css/text-overflow-ellipsis-bidi.html fast/multicol/span/span-as-immediate-child-property-removal.html fast/forms/basic-textareas-quirks.html fast/forms/input-readonly-dimmed.html fast/replaced/width100percent-textarea.html fast/forms/input-text-scroll-left-on-blur.html fast/css/text-overflow-input.html svg/text/text-overflow-ellipsis-svgfont.html fast/css/vertical-text-overflow-ellipsis-text-align-center.html fast/css/text-overflow-ellipsis-strict.html fast/multicol/span/clone-anonymous-block-non-inline-child-crash.html fast/forms/input-readonly-autoscroll.html fast/css/text-overflow-ellipsis.html fast/forms/input-disabled-color.html fast/multicol/span/anonymous-split-block-crash.html fast/forms/search-rtl.html fast/forms/basic-textareas.html Comment on attachment 172430 [details] Patch Attachment 172430 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/14738186 New failing tests: fast/css/vertical-text-overflow-ellipsis-text-align-center.html fast/multicol/span/anonymous-split-block-crash.html fast/css/text-overflow-ellipsis-text-align-center.html svg/text/text-overflow-ellipsis-svgfont.html fast/multicol/span/clone-anonymous-block-non-inline-child-crash.html Comment on attachment 172430 [details] Patch Attachment 172430 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/14691331 New failing tests: fast/css/vertical-text-overflow-ellipsis-text-align-center.html fast/multicol/span/anonymous-split-block-crash.html fast/css/text-overflow-ellipsis-text-align-center.html svg/text/text-overflow-ellipsis-svgfont.html fast/multicol/span/clone-anonymous-block-non-inline-child-crash.html (In reply to comment #21) > (From update of attachment 172430 [details]) > Attachment 172430 [details] did not pass chromium-ews (chromium-xvfb): > Output: http://queues.webkit.org/results/14691331 > > New failing tests: > fast/css/vertical-text-overflow-ellipsis-text-align-center.html > fast/multicol/span/anonymous-split-block-crash.html > fast/css/text-overflow-ellipsis-text-align-center.html > svg/text/text-overflow-ellipsis-svgfont.html > fast/multicol/span/clone-anonymous-block-non-inline-child-crash.html Failed tests? That seems very curious. (In reply to comment #22) > Failed tests? That seems very curious. Seems unlikely, rerunning all tests now just to make sure. (In reply to comment #23) > (In reply to comment #22) > > Failed tests? That seems very curious. > > Seems unlikely, rerunning all tests now just to make sure. I was thinking the same thing, though the multicol tests are notoriously finicky with changes in LayoutUnits. (In reply to comment #24) > (In reply to comment #23) > > (In reply to comment #22) > > > Failed tests? That seems very curious. > > > > Seems unlikely, rerunning all tests now just to make sure. > > I was thinking the same thing, though the multicol tests are notoriously finicky with changes in LayoutUnits. The failures are all due to the inline enclosingLayoutRect function in LayoutTypesInlineMethods.h masking a function of the same name LayoutRect.cpp. The LayoutTypesInlineMethods versions is an alias for enclosingIntRect, this is likely incorrect but I'll make that change in a separate patch. Created attachment 172641 [details]
Patch
Attachment 172641 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/CMakeLists.txt', u'Source/W..." exit_code: 1
Source/WebCore/platform/graphics/LayoutRect.h:215: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3]
Source/WebCore/platform/LayoutUnit.h:191: wtf_ceil is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
Source/WebCore/platform/graphics/LayoutRect.cpp:38: Use 'using namespace std;' instead of 'using std::max;'. [build/using_std] [4]
Source/WebCore/platform/graphics/LayoutRect.cpp:39: Use 'using namespace std;' instead of 'using std::min;'. [build/using_std] [4]
Source/WebCore/platform/graphics/LayoutBoxExtent.cpp:187: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3]
Source/WebCore/platform/graphics/LayoutBoxExtent.cpp:193: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3]
Total errors found: 6 in 123 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 172641 [details] Patch Attachment 172641 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/14744515 The cr-linux bot failed to remove the old FractionalLayoutUnit.h file even though it is marked as deleted in the patch :( It is almost like the bots don't like me. Created attachment 172658 [details]
Patch
Attachment 172658 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/CMakeLists.txt', u'Source/W..." exit_code: 1
Source/WebCore/platform/graphics/LayoutRect.h:215: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3]
Source/WebCore/platform/LayoutUnit.h:191: wtf_ceil is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
Source/WebCore/platform/graphics/LayoutRect.cpp:38: Use 'using namespace std;' instead of 'using std::max;'. [build/using_std] [4]
Source/WebCore/platform/graphics/LayoutRect.cpp:39: Use 'using namespace std;' instead of 'using std::min;'. [build/using_std] [4]
Source/WebCore/platform/graphics/LayoutBoxExtent.cpp:187: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3]
Source/WebCore/platform/graphics/LayoutBoxExtent.cpp:193: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3]
Total errors found: 6 in 123 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 172658 [details] Patch Attachment 172658 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/14755305 Comment on attachment 172658 [details] Patch Attachment 172658 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/14745534 Comment on attachment 172658 [details] Patch Attachment 172658 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/14748467 Comment on attachment 172658 [details] Patch Attachment 172658 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/14762159 New failing tests: fast/transforms/rotated-transform-affects-scrolling-2.html fast/clip/overflow-border-radius-composited.html compositing/tiling/tile-cache-zoomed.html fast/transforms/rotated-transform-affects-scrolling-1.html transforms/2d/hindi-rotated.html fast/clip/overflow-border-radius-transformed.html fast/forms/validation-message-appearance.html Comment on attachment 172658 [details] Patch Attachment 172658 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/14762172 New failing tests: fast/transforms/rotated-transform-affects-scrolling-2.html fast/clip/overflow-border-radius-composited.html compositing/tiling/tile-cache-zoomed.html fast/transforms/rotated-transform-affects-scrolling-1.html transforms/2d/hindi-rotated.html fast/clip/overflow-border-radius-transformed.html fast/forms/validation-message-appearance.html Created attachment 172815 [details]
Patch
Attachment 172815 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/CMakeLists.txt', u'Source/W..." exit_code: 1
Source/WebCore/platform/graphics/LayoutRect.h:215: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3]
Source/WebCore/platform/LayoutUnit.h:191: wtf_ceil is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
Source/WebCore/platform/graphics/LayoutRect.cpp:38: Use 'using namespace std;' instead of 'using std::max;'. [build/using_std] [4]
Source/WebCore/platform/graphics/LayoutRect.cpp:39: Use 'using namespace std;' instead of 'using std::min;'. [build/using_std] [4]
Source/WebCore/platform/graphics/LayoutBoxExtent.cpp:187: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3]
Source/WebCore/platform/graphics/LayoutBoxExtent.cpp:193: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3]
Total errors found: 6 in 124 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 172815 [details] Patch Attachment 172815 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/14761371 Comment on attachment 172815 [details] Patch Attachment 172815 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/14759414 Passed on mac now and the cr-linux bot is broken again (didn't remove old files). Please review. Comment on attachment 172815 [details]
Patch
You may want to warn the troopers before trying to land this. I suspect there'll need to be some clobbers.
Committed r133779: <http://trac.webkit.org/changeset/133779> |