Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(48)

Issue 2191233002: Add platform/geometry pretty printers for logging and testing (Closed)

Created:
4 years, 4 months ago by pdr.
Modified:
4 years, 4 months ago
CC:
ajuma+watch_chromium.org, blink-reviews, blink-reviews-platform-graphics_chromium.org, Rik, chromium-reviews, danakj+watch_chromium.org, dshwang, drott+blinkwatch_chromium.org, krit, f(malita), jbroman, Justin Novosad, pdr+graphicswatchlist_chromium.org, rwlbuis, Stephen Chennney
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Add platform/geometry pretty printers for logging and testing This patch switches from a test-only GeometryPrinters and debug-only toString() to a common pretty printer format for logging and testing. For example, to print the value of a LayoutRect: LayoutRect overflowRect(1, 2, 3, 4); LOG(INFO) << "overflow: " << overflowRect.toString(); Which prints: overflow: "1,2 3x4" In tests, gtests will automatically call the PrintTo functions. In debuggers, toString() should work as expected. Minor changes: 1) Switched floating point precision to use "%lg" which prints up to 6 significant figures, and fewer if the value is an integer. 2) Added special-cases to catch LayoutUnit's min/max/nearly{Min,Max}. BUG=632096 Committed: https://crrev.com/bdceea3ea3872c9188c48c0a6e71d9a52ff92feb Cr-Commit-Position: refs/heads/master@{#413347}

Patch Set 1 #

Patch Set 2 : Adjust tests to work around uninteresting cross-platform differences #

Total comments: 1

Patch Set 3 : Make tests simpler with testing::PrintToString #

Patch Set 4 : Switch to toString() and print shorter output #

Patch Set 5 : Rebase #

Total comments: 2

Patch Set 6 : Fastest logging this side of the mississippi #

Patch Set 7 : Fastest logging this side of the mississippi #

Total comments: 9

Patch Set 8 : Fewer redundant spaces, more toString tests #

Unified diffs Side-by-side diffs Delta from patch set Stats (+199 lines, -228 lines) Patch
M third_party/WebKit/Source/platform/LayoutUnit.h View 1 2 3 4 5 4 chunks +6 lines, -2 lines 0 comments Download
A third_party/WebKit/Source/platform/LayoutUnit.cpp View 1 2 3 4 5 6 1 chunk +24 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/platform/blink_platform.gypi View 1 2 3 4 5 6 7 3 chunks +4 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/platform/geometry/FloatBox.h View 1 2 3 3 chunks +5 lines, -1 line 0 comments Download
A + third_party/WebKit/Source/platform/geometry/FloatBox.cpp View 1 2 3 4 5 6 7 1 chunk +3 lines, -5 lines 0 comments Download
M third_party/WebKit/Source/platform/geometry/FloatBoxTest.cpp View 1 2 3 4 5 6 7 2 chunks +7 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/platform/geometry/FloatPoint.h View 1 2 3 1 chunk +0 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/platform/geometry/FloatPoint.cpp View 1 2 3 1 chunk +1 line, -3 lines 0 comments Download
M third_party/WebKit/Source/platform/geometry/FloatPoint3D.h View 1 2 3 1 chunk +2 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/platform/geometry/FloatPoint3D.cpp View 1 2 3 2 chunks +6 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/platform/geometry/FloatQuad.h View 1 2 3 1 chunk +1 line, -4 lines 0 comments Download
M third_party/WebKit/Source/platform/geometry/FloatQuad.cpp View 1 2 3 2 chunks +7 lines, -8 lines 0 comments Download
A + third_party/WebKit/Source/platform/geometry/FloatQuadTest.cpp View 1 2 3 4 5 6 7 1 chunk +8 lines, -9 lines 0 comments Download
M third_party/WebKit/Source/platform/geometry/FloatRect.h View 1 2 3 1 chunk +0 lines, -4 lines 0 comments Download
M third_party/WebKit/Source/platform/geometry/FloatRect.cpp View 1 2 3 2 chunks +7 lines, -12 lines 0 comments Download
M third_party/WebKit/Source/platform/geometry/FloatRectTest.cpp View 1 2 3 1 chunk +12 lines, -5 lines 0 comments Download
M third_party/WebKit/Source/platform/geometry/FloatRoundedRect.h View 1 2 3 3 chunks +3 lines, -9 lines 0 comments Download
M third_party/WebKit/Source/platform/geometry/FloatRoundedRect.cpp View 1 2 3 3 chunks +14 lines, -21 lines 0 comments Download
M third_party/WebKit/Source/platform/geometry/FloatRoundedRectTest.cpp View 1 2 3 2 chunks +8 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/platform/geometry/FloatSize.h View 1 2 3 1 chunk +0 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/platform/geometry/FloatSize.cpp View 1 2 3 1 chunk +1 line, -3 lines 0 comments Download
M third_party/WebKit/Source/platform/geometry/IntPoint.h View 1 2 3 2 chunks +4 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/platform/geometry/IntPoint.cpp View 1 2 3 1 chunk +0 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/platform/geometry/IntRect.h View 1 2 3 1 chunk +0 lines, -4 lines 0 comments Download
M third_party/WebKit/Source/platform/geometry/IntRect.cpp View 1 2 3 1 chunk +3 lines, -9 lines 0 comments Download
M third_party/WebKit/Source/platform/geometry/IntRectTest.cpp View 1 2 3 2 chunks +0 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/platform/geometry/IntSize.h View 1 2 3 2 chunks +4 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/platform/geometry/IntSize.cpp View 1 2 3 1 chunk +0 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/platform/geometry/LayoutPoint.h View 1 2 3 2 chunks +4 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/platform/geometry/LayoutPoint.cpp View 1 2 3 1 chunk +3 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/platform/geometry/LayoutRect.h View 1 2 3 1 chunk +0 lines, -4 lines 0 comments Download
M third_party/WebKit/Source/platform/geometry/LayoutRect.cpp View 1 2 3 2 chunks +7 lines, -15 lines 0 comments Download
M third_party/WebKit/Source/platform/geometry/LayoutRectTest.cpp View 1 2 3 1 chunk +4 lines, -4 lines 0 comments Download
M third_party/WebKit/Source/platform/geometry/LayoutSize.h View 1 2 3 2 chunks +4 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/platform/geometry/LayoutSize.cpp View 1 2 3 1 chunk +3 lines, -3 lines 0 comments Download
D third_party/WebKit/Source/platform/testing/GeometryPrinters.h View 1 2 3 2 chunks +9 lines, -0 lines 0 comments Download
D third_party/WebKit/Source/platform/testing/GeometryPrinters.cpp View 1 2 3 1 chunk +35 lines, -82 lines 0 comments Download

Messages

Total messages: 59 (31 generated)
pdr.
4 years, 4 months ago (2016-07-29 03:40:48 UTC) #2
jbroman
Possibly a little on the verbose side, but I don't care to bikeshed so lgtm. ...
4 years, 4 months ago (2016-07-29 14:37:29 UTC) #11
eae
Make logging great again! LGTM
4 years, 4 months ago (2016-07-29 17:08:34 UTC) #12
chrishtr
lgtm
4 years, 4 months ago (2016-07-29 17:30:39 UTC) #13
esprehn
I dont want to have to use ostringstream in all of these tests, let's make ...
4 years, 4 months ago (2016-07-29 18:07:38 UTC) #14
esprehn
Hmm I guess the tests are just for the formatting code itself. Okay let's do ...
4 years, 4 months ago (2016-07-29 18:09:35 UTC) #15
pdr.
On 2016/07/29 at 14:37:29, jbroman wrote: > Possibly a little on the verbose side, but ...
4 years, 4 months ago (2016-07-29 22:50:48 UTC) #16
pdr.
Dana pointed out that the ToString approach used in https://codereview.chromium.org/368903003 may be better in terms ...
4 years, 4 months ago (2016-07-29 22:56:19 UTC) #17
wkorman
On 2016/07/29 at 22:56:19, pdr wrote: > Dana pointed out that the ToString approach used ...
4 years, 4 months ago (2016-08-02 17:47:17 UTC) #18
pdr.
On 2016/08/02 at 17:47:17, wkorman wrote: > On 2016/07/29 at 22:56:19, pdr wrote: > > ...
4 years, 4 months ago (2016-08-02 18:07:11 UTC) #19
esprehn
I'm inclined to keep toString() and make the stream operators call it so printf debugging ...
4 years, 4 months ago (2016-08-02 18:51:48 UTC) #20
blink-reviews
Thats how gfx works also, and it is nice. On Tue, Aug 2, 2016 at ...
4 years, 4 months ago (2016-08-02 20:16:15 UTC) #21
chromium-reviews
Thats how gfx works also, and it is nice. On Tue, Aug 2, 2016 at ...
4 years, 4 months ago (2016-08-02 20:16:15 UTC) #22
esprehn
Is this ready to land?
4 years, 4 months ago (2016-08-15 16:13:48 UTC) #23
pdr.
On 2016/08/15 at 16:13:48, esprehn wrote: > Is this ready to land? Not yet, I ...
4 years, 4 months ago (2016-08-15 17:01:54 UTC) #24
pdr.
In the latest patch I've switched to toString() instead of operator<< which will be more ...
4 years, 4 months ago (2016-08-19 06:25:05 UTC) #33
eae
LGTM
4 years, 4 months ago (2016-08-19 17:53:31 UTC) #36
esprehn
lgtm https://codereview.chromium.org/2191233002/diff/80001/third_party/WebKit/Source/platform/LayoutUnit.h File third_party/WebKit/Source/platform/LayoutUnit.h (right): https://codereview.chromium.org/2191233002/diff/80001/third_party/WebKit/Source/platform/LayoutUnit.h#newcode218 third_party/WebKit/Source/platform/LayoutUnit.h:218: if (m_value == LayoutUnit::max().rawValue()) can we make this ...
4 years, 4 months ago (2016-08-19 17:58:53 UTC) #37
pdr.
On 2016/08/19 at 17:58:53, esprehn wrote: > lgtm > > https://codereview.chromium.org/2191233002/diff/80001/third_party/WebKit/Source/platform/LayoutUnit.h > File third_party/WebKit/Source/platform/LayoutUnit.h (right): ...
4 years, 4 months ago (2016-08-19 18:47:38 UTC) #38
wkorman
lgtm So exciting! Thanks for doing this. https://codereview.chromium.org/2191233002/diff/120001/third_party/WebKit/Source/platform/geometry/FloatBox.cpp File third_party/WebKit/Source/platform/geometry/FloatBox.cpp (right): https://codereview.chromium.org/2191233002/diff/120001/third_party/WebKit/Source/platform/geometry/FloatBox.cpp#newcode13 third_party/WebKit/Source/platform/geometry/FloatBox.cpp:13: return String::format("%lg,%lg,%lg ...
4 years, 4 months ago (2016-08-19 22:18:44 UTC) #40
pdr.
https://codereview.chromium.org/2191233002/diff/120001/third_party/WebKit/Source/platform/geometry/FloatBox.cpp File third_party/WebKit/Source/platform/geometry/FloatBox.cpp (right): https://codereview.chromium.org/2191233002/diff/120001/third_party/WebKit/Source/platform/geometry/FloatBox.cpp#newcode13 third_party/WebKit/Source/platform/geometry/FloatBox.cpp:13: return String::format("%lg,%lg,%lg %lgx%lgx%lg", x(), y(), z(), width(), height(), depth()); ...
4 years, 4 months ago (2016-08-19 23:21:15 UTC) #41
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2191233002/140001
4 years, 4 months ago (2016-08-20 05:05:07 UTC) #48
commit-bot: I haz the power
Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_android_rel_ng/builds/126453)
4 years, 4 months ago (2016-08-20 05:57:21 UTC) #50
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2191233002/140001
4 years, 4 months ago (2016-08-20 16:17:28 UTC) #52
commit-bot: I haz the power
Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_android_rel_ng/builds/126564)
4 years, 4 months ago (2016-08-20 17:04:49 UTC) #54
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2191233002/140001
4 years, 4 months ago (2016-08-20 17:59:17 UTC) #56
commit-bot: I haz the power
Committed patchset #8 (id:140001)
4 years, 4 months ago (2016-08-20 19:19:56 UTC) #57
commit-bot: I haz the power
4 years, 4 months ago (2016-08-20 19:21:48 UTC) #59
Message was sent while issue was closed.
Patchset 8 (id:??) landed as
https://crrev.com/bdceea3ea3872c9188c48c0a6e71d9a52ff92feb
Cr-Commit-Position: refs/heads/master@{#413347}

Powered by Google App Engine
This is Rietveld 408576698