Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1617763003:
Fix bounds of checking if a radii are too long for a side. (Closed)
DescriptionFix bounds of checking if a radii are too long for a side.
BUG=skia:4692, skia:4413
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1617763003
Committed: https://skia.googlesource.com/skia/+/5e0883cf57176134155fb70c9bc51dbb0f0c67fb
Patch Set 1 #
Total comments: 8
Patch Set 2 : Address comment. #
Messages
Total messages: 23 (11 generated)
Description was changed from ========== Fix bounds of checking if a radii are too long for a side. BUG=skia:4692 ========== to ========== Fix bounds of checking if a radii are too long for a side. BUG=skia:4692 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
Description was changed from ========== Fix bounds of checking if a radii are too long for a side. BUG=skia:4692 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== Fix bounds of checking if a radii are too long for a side. BUG=skia:4692 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
herb@google.com changed reviewers: + reed@google.com, robertphillips@google.com
I have run the fuzzer for 2 hours, and there doesn't seem to be any cases.
mtklein@google.com changed reviewers: + mtklein@google.com
https://codereview.chromium.org/1617763003/diff/1/fuzz/FuzzScaleToSides.cpp File fuzz/FuzzScaleToSides.cpp (right): https://codereview.chromium.org/1617763003/diff/1/fuzz/FuzzScaleToSides.cpp#n... fuzz/FuzzScaleToSides.cpp:32: if (width <= 0.0f) { might want to merge this one with the previous checks
The CQ bit was checked by herb@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1617763003/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1617763003/1
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: Build-Win-MSVC-x86-Debug-Trybot on client.skia.compile (JOB_FAILED, http://build.chromium.org/p/client.skia.compile/builders/Build-Win-MSVC-x86-D...)
https://codereview.chromium.org/1617763003/diff/1/src/core/SkScaleToSides.h File src/core/SkScaleToSides.h (right): https://codereview.chromium.org/1617763003/diff/1/src/core/SkScaleToSides.h#n... src/core/SkScaleToSides.h:11: #include <cmath> Do we actually need <limits> ? https://codereview.chromium.org/1617763003/diff/1/src/core/SkScaleToSides.h#n... src/core/SkScaleToSides.h:17: public: in in -> in ? https://codereview.chromium.org/1617763003/diff/1/tests/ScaleToSidesTest.cpp File tests/ScaleToSidesTest.cpp (right): https://codereview.chromium.org/1617763003/diff/1/tests/ScaleToSidesTest.cpp#... tests/ScaleToSidesTest.cpp:10: #include <cfloat> Do we actually need all these new headers ?
Description was changed from ========== Fix bounds of checking if a radii are too long for a side. BUG=skia:4692 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== Fix bounds of checking if a radii are too long for a side. BUG=skia:4692,skia:4413 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
Address comment.
In addition, skia:4413 seems to be fixed now. https://codereview.chromium.org/1617763003/diff/1/fuzz/FuzzScaleToSides.cpp File fuzz/FuzzScaleToSides.cpp (right): https://codereview.chromium.org/1617763003/diff/1/fuzz/FuzzScaleToSides.cpp#n... fuzz/FuzzScaleToSides.cpp:32: if (width <= 0.0f) { On 2016/01/22 14:02:04, mtklein wrote: > might want to merge this one with the previous checks Done. https://codereview.chromium.org/1617763003/diff/1/src/core/SkScaleToSides.h File src/core/SkScaleToSides.h (right): https://codereview.chromium.org/1617763003/diff/1/src/core/SkScaleToSides.h#n... src/core/SkScaleToSides.h:11: #include <cmath> On 2016/01/22 14:42:51, robertphillips wrote: > Do we actually need <limits> ? Done. https://codereview.chromium.org/1617763003/diff/1/src/core/SkScaleToSides.h#n... src/core/SkScaleToSides.h:17: public: On 2016/01/22 14:42:51, robertphillips wrote: > in in -> in ? Done. https://codereview.chromium.org/1617763003/diff/1/tests/ScaleToSidesTest.cpp File tests/ScaleToSidesTest.cpp (right): https://codereview.chromium.org/1617763003/diff/1/tests/ScaleToSidesTest.cpp#... tests/ScaleToSidesTest.cpp:10: #include <cfloat> On 2016/01/22 14:42:51, robertphillips wrote: > Do we actually need all these new headers ? Done.
lgtm
The CQ bit was checked by herb@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1617763003/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1617763003/20001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by herb@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1617763003/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1617763003/20001
Message was sent while issue was closed.
Description was changed from ========== Fix bounds of checking if a radii are too long for a side. BUG=skia:4692,skia:4413 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== Fix bounds of checking if a radii are too long for a side. BUG=skia:4692,skia:4413 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... Committed: https://skia.googlesource.com/skia/+/5e0883cf57176134155fb70c9bc51dbb0f0c67fb ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://skia.googlesource.com/skia/+/5e0883cf57176134155fb70c9bc51dbb0f0c67fb |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
