Convert OffscreenCanvas-getContext tests to testharness.

Convert OffscreenCanvas-getContext tests to testharness.

This patch uses testharness.js asserts to test
OffscreenCanvas-getContext & OffscreenCanvas-getContext-in-worker
tests. Removes the expectation files.

BUG=639732
Committed: https://crrev.com/e837ab605085aa68e5212f5f6d6c6e20c605791c
Cr-Commit-Position: refs/heads/master@{#414511}

[sivag, 2016-08-23 06:19:25]

Description was changed from

==========
Convert OffscreenCanvas-getContext tests to testharness.

This patch uses testharness.js asserts to test
OffscreenCanvas-getContext & OffscreenCanvas-getContext-in-worker
tests. Removes the expectation files.

BUG=639732
==========

to

==========
Convert OffscreenCanvas-getContext tests to testharness.

This patch uses testharness.js asserts to test
OffscreenCanvas-getContext & OffscreenCanvas-getContext-in-worker
tests. Removes the expectation files.

BUG=639732
==========

[sivag, 2016-08-23 06:19:26]

siva.gunturi@samsung.com changed reviewers:
+ srirama.m@samsung.com

[sivag, 2016-08-23 06:21:15]

siva.gunturi@samsung.com changed reviewers:
+ fs@opera.com, junov@chromium.org

[sivag, 2016-08-23 06:21:24]

ptal..

[Srirama, 2016-08-23 06:34:25]

https://codereview.chromium.org/2271593002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/fast/canvas/OffscreenCanvas-getContext-in-worker.html
(right):

https://codereview.chromium.org/2271593002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/canvas/OffscreenCanvas-getContext-in-worker.html:6:
self.onmessage = function(e) {
you can remove 'e'.

https://codereview.chromium.org/2271593002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/canvas/OffscreenCanvas-getContext-in-worker.html:7:
var aCanvas = new OffscreenCanvas(50, 50);
Use same indentation everywhere either 2 or 4.

https://codereview.chromium.org/2271593002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/canvas/OffscreenCanvas-getContext-in-worker.html:9:
try {
i think you can remove try catch. If there is an exception test will fail
anyway.

https://codereview.chromium.org/2271593002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/canvas/OffscreenCanvas-getContext-in-worker.html:12:
if (!(ctx1 instanceof OffscreenCanvasRenderingContext2D)) {
can we assert these instead of conditions ?
assert_true(ctx1 instanceof OffscreenCanvasRenderingContext2D);
assert_true(ctx2 instanceof WebGLRenderingContext);
self.postMessage("success");

https://codereview.chromium.org/2271593002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/canvas/OffscreenCanvas-getContext-in-worker.html:37:
worker.addEventListener('message', handleMessageFromWorker);
may be inline hte function here. like 
worker.onmessage = t.step-func_done(function() {
    assert_equals(msg.data, "success");
});

https://codereview.chromium.org/2271593002/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/fast/canvas/OffscreenCanvas-getContext.html
(right):

https://codereview.chromium.org/2271593002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/canvas/OffscreenCanvas-getContext.html:31:
assert_true(ctx3 == ctx);
use assert_equals

[fs, 2016-08-23 08:50:58]

https://codereview.chromium.org/2271593002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/fast/canvas/OffscreenCanvas-getContext-in-worker.html
(right):

https://codereview.chromium.org/2271593002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/canvas/OffscreenCanvas-getContext-in-worker.html:12:
if (!(ctx1 instanceof OffscreenCanvasRenderingContext2D)) {
On 2016/08/23 at 06:34:24, Srirama wrote:
> can we assert these instead of conditions ?
> assert_true(ctx1 instanceof OffscreenCanvasRenderingContext2D);
> assert_true(ctx2 instanceof WebGLRenderingContext);
> self.postMessage("success");

It needs some additional setup [1], but is probably a good idea in this case.

[1]
https://github.com/w3c/testharness.js/blob/8550cfecab28e680545486a85887334a37...

https://codereview.chromium.org/2271593002/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/fast/canvas/OffscreenCanvas-getContext.html
(right):

https://codereview.chromium.org/2271593002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/canvas/OffscreenCanvas-getContext.html:2:
<title>Tests basic functionalities of offscreenCanvas.getContext on the main
thread.</title>
OffscreenCanvas

https://codereview.chromium.org/2271593002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/canvas/OffscreenCanvas-getContext.html:7: //
Tests constructor of OffscreenCanvas and length/width change
Add full stops ('.') at end up sentences - here and all similar cases below.

Also, I guess 'length' should really be 'height'.

https://codereview.chromium.org/2271593002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/canvas/OffscreenCanvas-getContext.html:22:
ctx = aCanvas.getContext('2d');
Maybe merge this with the previous line.

https://codereview.chromium.org/2271593002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/canvas/OffscreenCanvas-getContext.html:23:
assert_true(ctx instanceof  OffscreenCanvasRenderingContext2D);
Double space after instanceof.

[Justin Novosad, 2016-08-23 15:53:00]

https://codereview.chromium.org/2271593002/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/fast/canvas/OffscreenCanvas-getContext.html
(right):

https://codereview.chromium.org/2271593002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/canvas/OffscreenCanvas-getContext.html:33:
// TODO: change the below part of the test when webgl is supported.
This comment is no longer relevant. WebGL works now. Please just remove it.

[sivag, 2016-08-25 07:39:02]

ptal..

https://codereview.chromium.org/2271593002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/fast/canvas/OffscreenCanvas-getContext-in-worker.html
(right):

https://codereview.chromium.org/2271593002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/canvas/OffscreenCanvas-getContext-in-worker.html:6:
self.onmessage = function(e) {
On 2016/08/23 06:34:24, Srirama wrote:
> you can remove 'e'.

Done.

https://codereview.chromium.org/2271593002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/canvas/OffscreenCanvas-getContext-in-worker.html:7:
var aCanvas = new OffscreenCanvas(50, 50);
On 2016/08/23 06:34:24, Srirama wrote:
> Use same indentation everywhere either 2 or 4.

Done.

https://codereview.chromium.org/2271593002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/canvas/OffscreenCanvas-getContext-in-worker.html:9:
try {
On 2016/08/23 06:34:24, Srirama wrote:
> i think you can remove try catch. If there is an exception test will fail
> anyway.

Done.

https://codereview.chromium.org/2271593002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/canvas/OffscreenCanvas-getContext-in-worker.html:12:
if (!(ctx1 instanceof OffscreenCanvasRenderingContext2D)) {
On 2016/08/23 08:50:57, fs wrote:
> On 2016/08/23 at 06:34:24, Srirama wrote:
> > can we assert these instead of conditions ?
> > assert_true(ctx1 instanceof OffscreenCanvasRenderingContext2D);
> > assert_true(ctx2 instanceof WebGLRenderingContext);
> > self.postMessage("success");
> 
> It needs some additional setup [1], but is probably a good idea in this case.
> 
> [1]
>
https://github.com/w3c/testharness.js/blob/8550cfecab28e680545486a85887334a37...

Done.

https://codereview.chromium.org/2271593002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/canvas/OffscreenCanvas-getContext-in-worker.html:12:
if (!(ctx1 instanceof OffscreenCanvasRenderingContext2D)) {
On 2016/08/23 06:34:24, Srirama wrote:
> can we assert these instead of conditions ?
> assert_true(ctx1 instanceof OffscreenCanvasRenderingContext2D);
> assert_true(ctx2 instanceof WebGLRenderingContext);
> self.postMessage("success");

Done.

https://codereview.chromium.org/2271593002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/canvas/OffscreenCanvas-getContext-in-worker.html:37:
worker.addEventListener('message', handleMessageFromWorker);
On 2016/08/23 06:34:24, Srirama wrote:
> may be inline hte function here. like 
> worker.onmessage = t.step-func_done(function() {
>     assert_equals(msg.data, "success");
> });

Done.

https://codereview.chromium.org/2271593002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/canvas/OffscreenCanvas-getContext-in-worker.html:37:
worker.addEventListener('message', handleMessageFromWorker);
On 2016/08/23 06:34:24, Srirama wrote:
> may be inline hte function here. like 
> worker.onmessage = t.step-func_done(function() {
>     assert_equals(msg.data, "success");
> });

Done. 
Modified using fetch_tests_from_worker.

https://codereview.chromium.org/2271593002/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/fast/canvas/OffscreenCanvas-getContext.html
(right):

https://codereview.chromium.org/2271593002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/canvas/OffscreenCanvas-getContext.html:2:
<title>Tests basic functionalities of offscreenCanvas.getContext on the main
thread.</title>
On 2016/08/23 08:50:58, fs wrote:
> OffscreenCanvas

Done.

https://codereview.chromium.org/2271593002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/canvas/OffscreenCanvas-getContext.html:7: //
Tests constructor of OffscreenCanvas and length/width change
On 2016/08/23 08:50:57, fs wrote:
> Add full stops ('.') at end up sentences - here and all similar cases below.
> 
> Also, I guess 'length' should really be 'height'.

Done.

https://codereview.chromium.org/2271593002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/canvas/OffscreenCanvas-getContext.html:22:
ctx = aCanvas.getContext('2d');
On 2016/08/23 08:50:57, fs wrote:
> Maybe merge this with the previous line.

Done.

https://codereview.chromium.org/2271593002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/canvas/OffscreenCanvas-getContext.html:23:
assert_true(ctx instanceof  OffscreenCanvasRenderingContext2D);
On 2016/08/23 08:50:57, fs wrote:
> Double space after instanceof.

Done.

https://codereview.chromium.org/2271593002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/canvas/OffscreenCanvas-getContext.html:31:
assert_true(ctx3 == ctx);
On 2016/08/23 06:34:24, Srirama wrote:
> use assert_equals

Done.

https://codereview.chromium.org/2271593002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/canvas/OffscreenCanvas-getContext.html:33:
// TODO: change the below part of the test when webgl is supported.
On 2016/08/23 15:53:00, Justin Novosad wrote:
> This comment is no longer relevant. WebGL works now. Please just remove it.

Done.

[fs, 2016-08-25 08:14:59]

lgtm

https://codereview.chromium.org/2271593002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/fast/canvas/OffscreenCanvas-getContext-in-worker.js
(right):

https://codereview.chromium.org/2271593002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/canvas/OffscreenCanvas-getContext-in-worker.js:1:
importScripts("../../resources/testharness.js");
Could possibly put this file in a resources/ or script-tests/ subdirectory, but
I'll leave that for others to decide (this should work as-is AFAIK.)

[Srirama, 2016-08-25 08:40:17]

LGTM with Justin having the final say.

[sivag, 2016-08-25 14:29:55]

@junov, ptal..

[Justin Novosad, 2016-08-25 14:56:50]

On 2016/08/25 14:29:55, sivag wrote:
> @junov, ptal..

lgtm

[sivag, 2016-08-25 18:14:10]

The CQ bit was checked by siva.gunturi@samsung.com

[commit-bot: I haz the power, 2016-08-25 18:14:47]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-25 19:39:38]

Description was changed from

==========
Convert OffscreenCanvas-getContext tests to testharness.

This patch uses testharness.js asserts to test
OffscreenCanvas-getContext & OffscreenCanvas-getContext-in-worker
tests. Removes the expectation files.

BUG=639732
==========

to

==========
Convert OffscreenCanvas-getContext tests to testharness.

This patch uses testharness.js asserts to test
OffscreenCanvas-getContext & OffscreenCanvas-getContext-in-worker
tests. Removes the expectation files.

BUG=639732
==========

[commit-bot: I haz the power, 2016-08-25 19:39:39]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-08-25 19:44:19]

Description was changed from

==========
Convert OffscreenCanvas-getContext tests to testharness.

This patch uses testharness.js asserts to test
OffscreenCanvas-getContext & OffscreenCanvas-getContext-in-worker
tests. Removes the expectation files.

BUG=639732
==========

to

==========
Convert OffscreenCanvas-getContext tests to testharness.

This patch uses testharness.js asserts to test
OffscreenCanvas-getContext & OffscreenCanvas-getContext-in-worker
tests. Removes the expectation files.

BUG=639732

Committed: https://crrev.com/e837ab605085aa68e5212f5f6d6c6e20c605791c
Cr-Commit-Position: refs/heads/master@{#414511}
==========

[commit-bot: I haz the power, 2016-08-25 19:44:20]

Patchset 2 (id:??) landed as
https://crrev.com/e837ab605085aa68e5212f5f6d6c6e20c605791c
Cr-Commit-Position: refs/heads/master@{#414511}