Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2311223002:
Convert fast/canvas Layouttests to testharness (Closed)
|
Created:
4 years, 3 months ago by Abhishek Kanike Modified:
4 years, 3 months ago Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionConvert fast/canvas Layouttests to testharness
This patch uses testharness.js asserts to test canvas
winding enumeration and also checks canvas borkedness
of rects with zero size.
BUG=639732
Committed: https://crrev.com/e3a7b314c7a3a12c1df78938b89b52b99413b8d7
Cr-Commit-Position: refs/heads/master@{#418536}
Patch Set 1 #
Total comments: 10
Patch Set 2 : Convert fast/canvas Layouttests to testharness #
Total comments: 8
Patch Set 3 : Refactor #
Total comments: 2
Patch Set 4 : Refactor2 #
Messages
Total messages: 25 (10 generated)
Description was changed from ========== Convert fast/canvas Layouttests to testharness This patch uses testharness.js asserts to test canvas winding enumeration and also checks canvas borkedness of rects with zero size. BUG=639732 ========== to ========== Convert fast/canvas Layouttests to testharness This patch uses testharness.js asserts to test canvas winding enumeration and also checks canvas borkedness of rects with zero size. BUG=639732 ==========
abhishek.ka@samsung.com changed reviewers: + siva.gunturi@samsung.com, srirama.m@samsung.com
https://codereview.chromium.org/2311223002/diff/1/third_party/WebKit/LayoutTe... File third_party/WebKit/LayoutTests/fast/canvas/winding-enumeration.html (right): https://codereview.chromium.org/2311223002/diff/1/third_party/WebKit/LayoutTe... third_party/WebKit/LayoutTests/fast/canvas/winding-enumeration.html:9: var canvas = document.createElement("canvas"); move this to html <canvas></canvas> and access using document.queryselector('canvas'); https://codereview.chromium.org/2311223002/diff/1/third_party/WebKit/LayoutTe... File third_party/WebKit/LayoutTests/fast/canvas/zero-size-fill-rect.html (right): https://codereview.chromium.org/2311223002/diff/1/third_party/WebKit/LayoutTe... third_party/WebKit/LayoutTests/fast/canvas/zero-size-fill-rect.html:9: <html> remove html, head not needed. https://codereview.chromium.org/2311223002/diff/1/third_party/WebKit/LayoutTe... third_party/WebKit/LayoutTests/fast/canvas/zero-size-fill-rect.html:18: var canvas = document.getElementById("test"); document.queryselector('canvas') https://codereview.chromium.org/2311223002/diff/1/third_party/WebKit/LayoutTe... third_party/WebKit/LayoutTests/fast/canvas/zero-size-fill-rect.html:30: </head> remove head, body, br, html tags. move canvas tag to top. remove id from canvas tag.
https://codereview.chromium.org/2311223002/diff/1/third_party/WebKit/LayoutTe... File third_party/WebKit/LayoutTests/fast/canvas/zero-size-fill-rect.html (right): https://codereview.chromium.org/2311223002/diff/1/third_party/WebKit/LayoutTe... third_party/WebKit/LayoutTests/fast/canvas/zero-size-fill-rect.html:15: <script type="text/javascript"> remove type attribute.
https://codereview.chromium.org/2311223002/diff/1/third_party/WebKit/LayoutTe... File third_party/WebKit/LayoutTests/fast/canvas/winding-enumeration.html (right): https://codereview.chromium.org/2311223002/diff/1/third_party/WebKit/LayoutTe... third_party/WebKit/LayoutTests/fast/canvas/winding-enumeration.html:9: var canvas = document.createElement("canvas"); On 2016/09/06 12:58:44, sivag wrote: > move this to html <canvas></canvas> > and access using document.queryselector('canvas'); Done. https://codereview.chromium.org/2311223002/diff/1/third_party/WebKit/LayoutTe... File third_party/WebKit/LayoutTests/fast/canvas/zero-size-fill-rect.html (right): https://codereview.chromium.org/2311223002/diff/1/third_party/WebKit/LayoutTe... third_party/WebKit/LayoutTests/fast/canvas/zero-size-fill-rect.html:9: <html> On 2016/09/06 12:58:44, sivag wrote: > remove html, head not needed. Done. https://codereview.chromium.org/2311223002/diff/1/third_party/WebKit/LayoutTe... third_party/WebKit/LayoutTests/fast/canvas/zero-size-fill-rect.html:15: <script type="text/javascript"> On 2016/09/06 13:27:20, Srirama wrote: > remove type attribute. Done. https://codereview.chromium.org/2311223002/diff/1/third_party/WebKit/LayoutTe... third_party/WebKit/LayoutTests/fast/canvas/zero-size-fill-rect.html:18: var canvas = document.getElementById("test"); On 2016/09/06 12:58:44, sivag wrote: > document.queryselector('canvas') Done. https://codereview.chromium.org/2311223002/diff/1/third_party/WebKit/LayoutTe... third_party/WebKit/LayoutTests/fast/canvas/zero-size-fill-rect.html:30: </head> On 2016/09/06 12:58:44, sivag wrote: > remove head, body, br, html tags. > move canvas tag to top. > remove id from canvas tag. Done.
abhishek.ka@samsung.com changed reviewers: + fs@opera.com, junov@chromium.org
Please take a look
Please take a look
lgtm
https://codereview.chromium.org/2311223002/diff/20001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/fast/canvas/winding-enumeration.html (right): https://codereview.chromium.org/2311223002/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/canvas/winding-enumeration.html:7: <canvas></canvas> Probably good the way it was (w/ createElement.) https://codereview.chromium.org/2311223002/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/canvas/winding-enumeration.html:10: var canvas = document.querySelector('canvas'); Indent 2 or 4 spaces within the test-function body. https://codereview.chromium.org/2311223002/diff/20001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/fast/canvas/zero-size-fill-rect.html (right): https://codereview.chromium.org/2311223002/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/canvas/zero-size-fill-rect.html:8: <!DOCTYPE html> Move this first in the file. And maybe move the comment into the script. https://codereview.chromium.org/2311223002/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/canvas/zero-size-fill-rect.html:15: async_test(function(t) { Doesn't look like this test needs to be async (needs to wait for onload), but I guess being conservative doesn't hurt.
https://codereview.chromium.org/2311223002/diff/20001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/fast/canvas/winding-enumeration.html (right): https://codereview.chromium.org/2311223002/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/canvas/winding-enumeration.html:7: <canvas></canvas> On 2016/09/08 10:13:10, fs wrote: > Probably good the way it was (w/ createElement.) Done. https://codereview.chromium.org/2311223002/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/canvas/winding-enumeration.html:10: var canvas = document.querySelector('canvas'); On 2016/09/08 10:13:10, fs wrote: > Indent 2 or 4 spaces within the test-function body. Done. https://codereview.chromium.org/2311223002/diff/20001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/fast/canvas/zero-size-fill-rect.html (right): https://codereview.chromium.org/2311223002/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/canvas/zero-size-fill-rect.html:8: <!DOCTYPE html> On 2016/09/08 10:13:10, fs wrote: > Move this first in the file. And maybe move the comment into the script. Done. https://codereview.chromium.org/2311223002/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/canvas/zero-size-fill-rect.html:15: async_test(function(t) { On 2016/09/08 10:13:11, fs wrote: > Doesn't look like this test needs to be async (needs to wait for onload), but I > guess being conservative doesn't hurt. Done.
lgtm
The CQ bit was checked by abhishek.ka@samsung.com
The patchset sent to the CQ was uploaded after l-g-t-m from srirama.m@samsung.com Link to the patchset: https://codereview.chromium.org/2311223002/#ps40001 (title: "Refactor")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2311223002/diff/40001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/fast/canvas/winding-enumeration.html (right): https://codereview.chromium.org/2311223002/diff/40001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/canvas/winding-enumeration.html:9: var canvas = document.createElement("canvas"); Looks like tabs were used, please use spaces instead of tabs.
The CQ bit was unchecked by abhishek.ka@samsung.com
https://codereview.chromium.org/2311223002/diff/40001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/fast/canvas/winding-enumeration.html (right): https://codereview.chromium.org/2311223002/diff/40001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/canvas/winding-enumeration.html:9: var canvas = document.createElement("canvas"); On 2016/09/14 10:22:47, Srirama_OOO wrote: > Looks like tabs were used, please use spaces instead of tabs. Done.
The CQ bit was checked by abhishek.ka@samsung.com
The patchset sent to the CQ was uploaded after l-g-t-m from srirama.m@samsung.com, fs@opera.com Link to the patchset: https://codereview.chromium.org/2311223002/#ps60001 (title: "Refactor2")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Convert fast/canvas Layouttests to testharness This patch uses testharness.js asserts to test canvas winding enumeration and also checks canvas borkedness of rects with zero size. BUG=639732 ========== to ========== Convert fast/canvas Layouttests to testharness This patch uses testharness.js asserts to test canvas winding enumeration and also checks canvas borkedness of rects with zero size. BUG=639732 ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== Convert fast/canvas Layouttests to testharness This patch uses testharness.js asserts to test canvas winding enumeration and also checks canvas borkedness of rects with zero size. BUG=639732 ========== to ========== Convert fast/canvas Layouttests to testharness This patch uses testharness.js asserts to test canvas winding enumeration and also checks canvas borkedness of rects with zero size. BUG=639732 Committed: https://crrev.com/e3a7b314c7a3a12c1df78938b89b52b99413b8d7 Cr-Commit-Position: refs/heads/master@{#418536} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/e3a7b314c7a3a12c1df78938b89b52b99413b8d7 Cr-Commit-Position: refs/heads/master@{#418536} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
