Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2457113002:
Fixing timeout in pixel_canvas_display_linear-rgb browser pixel test (Closed)
|
Created:
4 years, 1 month ago by zakerinasab Modified:
4 years, 1 month ago CC:
chromium-reviews, jam, darin-cc_chromium.org, piman+watch_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionFixing timeout in pixel_canvas_display_linear-rgb browser pixel test
Addressing the timeout bug in browser pixel test of
codereview.chromium.org/2448583002/
BUG=657946
Committed: https://crrev.com/8009581cb30fdf146a6bd48aa77605a7a7a43096
Committed: https://crrev.com/707a5decdda5d9ee767c84f4b7f4b3eddea8cb7e
Cr-Original-Commit-Position: refs/heads/master@{#428498}
Cr-Commit-Position: refs/heads/master@{#429343}
Patch Set 1 #Patch Set 2 : Fixed. #Patch Set 3 : Adding comments to the pixel test. #
Total comments: 8
Patch Set 4 : Addressing comments. #Patch Set 5 : Addressing timeout. #Patch Set 6 : Still timeout. #
Total comments: 4
Patch Set 7 : Addressing comments. #Messages
Total messages: 26 (14 generated)
zakerinasab@chromium.org changed reviewers: + junov@chromium.org, kbr@chromium.org, xidachen@chromium.org
New patch submitted. The browser pixel test of codereview.chromium.org/2448583002/ was timing out. This one generates the expected message on *_chromium_rel_ng bots: AssertionError: Could not find image Pixel_CanvasDisplayLinearRGBAccelerated2D in cloud storage
lgtm
The CQ bit was checked by zakerinasab@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from kbr@chromium.org Link to the patchset: https://codereview.chromium.org/2457113002/#ps40001 (title: "Adding comments to the pixel test.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2457113002/diff/40001/content/test/data/gpu/p... File content/test/data/gpu/pixel_canvas_display_linear-rgb.html (right): https://codereview.chromium.org/2457113002/diff/40001/content/test/data/gpu/p... content/test/data/gpu/pixel_canvas_display_linear-rgb.html:4: <style type="text/css"> Why is this necessary? https://codereview.chromium.org/2457113002/diff/40001/content/test/data/gpu/p... content/test/data/gpu/pixel_canvas_display_linear-rgb.html:47: <body onload="main()"> Is this what fixes the timeout? If so, care to explain? https://codereview.chromium.org/2457113002/diff/40001/content/test/data/gpu/p... content/test/data/gpu/pixel_canvas_display_linear-rgb.html:48: <div style="position:relative; width:300px; height:300px; background-color:white"> Why is this necessary? https://codereview.chromium.org/2457113002/diff/40001/content/test/data/gpu/p... content/test/data/gpu/pixel_canvas_display_linear-rgb.html:50: <div id="container" style="position:absolute; top:0px; left:0px"> Why is this necessary?
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Fixing timeout in pixel_canvas_display_linear-rgb browser pixel test Addressing the timeout bug in browser pixel test of codereview.chromium.org/2448583002/ BUG=657946 ========== to ========== Fixing timeout in pixel_canvas_display_linear-rgb browser pixel test Addressing the timeout bug in browser pixel test of codereview.chromium.org/2448583002/ BUG=657946 Committed: https://crrev.com/8009581cb30fdf146a6bd48aa77605a7a7a43096 Cr-Commit-Position: refs/heads/master@{#428498} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/8009581cb30fdf146a6bd48aa77605a7a7a43096 Cr-Commit-Position: refs/heads/master@{#428498}
Message was sent while issue was closed.
Description was changed from ========== Fixing timeout in pixel_canvas_display_linear-rgb browser pixel test Addressing the timeout bug in browser pixel test of codereview.chromium.org/2448583002/ BUG=657946 Committed: https://crrev.com/8009581cb30fdf146a6bd48aa77605a7a7a43096 Cr-Commit-Position: refs/heads/master@{#428498} ========== to ========== Fixing timeout in pixel_canvas_display_linear-rgb browser pixel test Addressing the timeout bug in browser pixel test of codereview.chromium.org/2448583002/ BUG=657946 Committed: https://crrev.com/8009581cb30fdf146a6bd48aa77605a7a7a43096 Cr-Commit-Position: refs/heads/master@{#428498} ==========
Patchset #7 (id:120001) has been deleted
Patchset #6 (id:100001) has been deleted
https://codereview.chromium.org/2457113002/diff/40001/content/test/data/gpu/p... File content/test/data/gpu/pixel_canvas_display_linear-rgb.html (right): https://codereview.chromium.org/2457113002/diff/40001/content/test/data/gpu/p... content/test/data/gpu/pixel_canvas_display_linear-rgb.html:4: <style type="text/css"> On 2016/10/28 21:12:19, Justin Novosad wrote: > Why is this necessary? Not necessary, can be removed. https://codereview.chromium.org/2457113002/diff/40001/content/test/data/gpu/p... content/test/data/gpu/pixel_canvas_display_linear-rgb.html:47: <body onload="main()"> On 2016/10/28 21:12:19, Justin Novosad wrote: > Is this what fixes the timeout? If so, care to explain? It is necessary as it waits for all the components to get loaded. I suspect when it is missing the canvas element is not ready for drawing (?). https://codereview.chromium.org/2457113002/diff/40001/content/test/data/gpu/p... content/test/data/gpu/pixel_canvas_display_linear-rgb.html:48: <div style="position:relative; width:300px; height:300px; background-color:white"> On 2016/10/28 21:12:19, Justin Novosad wrote: > Why is this necessary? No, can be removed. https://codereview.chromium.org/2457113002/diff/40001/content/test/data/gpu/p... content/test/data/gpu/pixel_canvas_display_linear-rgb.html:50: <div id="container" style="position:absolute; top:0px; left:0px"> On 2016/10/28 21:12:19, Justin Novosad wrote: > Why is this necessary? It seems the canvas must be inside a div for the browser pixel tests to get the screenshot properly. Otherwise the test times out.
non-owner lgtm with nits. https://codereview.chromium.org/2457113002/diff/140001/content/test/data/gpu/... File content/test/data/gpu/pixel_canvas_display_linear-rgb.html (right): https://codereview.chromium.org/2457113002/diff/140001/content/test/data/gpu/... content/test/data/gpu/pixel_canvas_display_linear-rgb.html:41: main(); if we have a body onload="main()", do we still need to call main() here? https://codereview.chromium.org/2457113002/diff/140001/content/test/data/gpu/... content/test/data/gpu/pixel_canvas_display_linear-rgb.html:44: <body onload="main()"> why is there a small symbol here after the '>'?
The CQ bit was checked by zakerinasab@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from kbr@chromium.org, xidachen@chromium.org Link to the patchset: https://codereview.chromium.org/2457113002/#ps160001 (title: "Addressing comments.")
The CQ bit was unchecked by zakerinasab@chromium.org
https://codereview.chromium.org/2457113002/diff/140001/content/test/data/gpu/... File content/test/data/gpu/pixel_canvas_display_linear-rgb.html (right): https://codereview.chromium.org/2457113002/diff/140001/content/test/data/gpu/... content/test/data/gpu/pixel_canvas_display_linear-rgb.html:41: main(); On 2016/11/02 15:48:26, xidachen wrote: > if we have a body onload="main()", do we still need to call main() here? Fixed. https://codereview.chromium.org/2457113002/diff/140001/content/test/data/gpu/... content/test/data/gpu/pixel_canvas_display_linear-rgb.html:44: <body onload="main()"> On 2016/11/02 15:48:26, xidachen wrote: > why is there a small symbol here after the '>'? Hm. It couldn't be seen in the editor. Fixed.
The CQ bit was checked by zakerinasab@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from kbr@chromium.org, xidachen@chromium.org Link to the patchset: https://codereview.chromium.org/2457113002/#ps160001 (title: "Addressing comments.")
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 ========== Fixing timeout in pixel_canvas_display_linear-rgb browser pixel test Addressing the timeout bug in browser pixel test of codereview.chromium.org/2448583002/ BUG=657946 Committed: https://crrev.com/8009581cb30fdf146a6bd48aa77605a7a7a43096 Cr-Commit-Position: refs/heads/master@{#428498} ========== to ========== Fixing timeout in pixel_canvas_display_linear-rgb browser pixel test Addressing the timeout bug in browser pixel test of codereview.chromium.org/2448583002/ BUG=657946 Committed: https://crrev.com/8009581cb30fdf146a6bd48aa77605a7a7a43096 Cr-Commit-Position: refs/heads/master@{#428498} ==========
Message was sent while issue was closed.
Committed patchset #7 (id:160001)
Message was sent while issue was closed.
Description was changed from ========== Fixing timeout in pixel_canvas_display_linear-rgb browser pixel test Addressing the timeout bug in browser pixel test of codereview.chromium.org/2448583002/ BUG=657946 Committed: https://crrev.com/8009581cb30fdf146a6bd48aa77605a7a7a43096 Cr-Commit-Position: refs/heads/master@{#428498} ========== to ========== Fixing timeout in pixel_canvas_display_linear-rgb browser pixel test Addressing the timeout bug in browser pixel test of codereview.chromium.org/2448583002/ BUG=657946 Committed: https://crrev.com/8009581cb30fdf146a6bd48aa77605a7a7a43096 Committed: https://crrev.com/707a5decdda5d9ee767c84f4b7f4b3eddea8cb7e Cr-Original-Commit-Position: refs/heads/master@{#428498} Cr-Commit-Position: refs/heads/master@{#429343} ==========
Message was sent while issue was closed.
Patchset 7 (id:??) landed as https://crrev.com/707a5decdda5d9ee767c84f4b7f4b3eddea8cb7e Cr-Commit-Position: refs/heads/master@{#429343} |
