Upstream IndexedDB binary key tests to WPT.

Upstream IndexedDB binary key tests to WPT.

BUG=683463
Review-Url: https://codereview.chromium.org/2673603003
Cr-Commit-Position: refs/heads/master@{#449206}
Committed: https://chromium.googlesource.com/chromium/src/+/c06d49ab041c8d203d0b451f9eb7251a5832db97

[pwnall, 2017-02-03 02:38:39]

pwnall@chromium.org changed reviewers:
+ jsbell@chromium.org

[pwnall, 2017-02-03 02:38:40]

PTAL?

Asides from breaking up and reformatting, this upstreaming fixes a few bugs that
I've highlighted below.

https://codereview.chromium.org/2673603003/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/storage/indexeddb/key-type-binary.html
(left):

https://codereview.chromium.org/2673603003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/storage/indexeddb/key-type-binary.html:46:
found_key, buffer, 'Buffers should be equal to put buffer');
This fails for views that start at a non-zero offset into the buffer.

https://codereview.chromium.org/2673603003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/storage/indexeddb/key-type-binary.html:97:
value_test('Offset DataView', new DataView(buffer), 3, 3);
IIUC, the "3, 3" arguments were meant to be passed to the DataView constructor.
value_test ignores them.

https://codereview.chromium.org/2673603003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/storage/indexeddb/key-type-binary.html:98:
value_test('Offset Uint8Array', new Uint8Array(buffer), 3, 3);
Same issue as above.

[pwnall, 2017-02-03 02:38:47]

The CQ bit was checked by pwnall@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-03 02:39:23]

Dry run: 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, 2017-02-03 03:44:58]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-03 03:44:58]

Dry run: This issue passed the CQ dry run.

[jeffcarp, 2017-02-05 19:38:28]

On 2017/02/03 at 02:38:40, pwnall wrote:
> PTAL?
> 
> Asides from breaking up and reformatting, this upstreaming fixes a few bugs
that I've highlighted below.
> 
>
https://codereview.chromium.org/2673603003/diff/1/third_party/WebKit/LayoutTe...
> File third_party/WebKit/LayoutTests/storage/indexeddb/key-type-binary.html
(left):
> 
>
https://codereview.chromium.org/2673603003/diff/1/third_party/WebKit/LayoutTe...
> third_party/WebKit/LayoutTests/storage/indexeddb/key-type-binary.html:46:
found_key, buffer, 'Buffers should be equal to put buffer');
> This fails for views that start at a non-zero offset into the buffer.
> 
>
https://codereview.chromium.org/2673603003/diff/1/third_party/WebKit/LayoutTe...
> third_party/WebKit/LayoutTests/storage/indexeddb/key-type-binary.html:97:
value_test('Offset DataView', new DataView(buffer), 3, 3);
> IIUC, the "3, 3" arguments were meant to be passed to the DataView
constructor. value_test ignores them.
> 
>
https://codereview.chromium.org/2673603003/diff/1/third_party/WebKit/LayoutTe...
> third_party/WebKit/LayoutTests/storage/indexeddb/key-type-binary.html:98:
value_test('Offset Uint8Array', new Uint8Array(buffer), 3, 3);
> Same issue as above.

It looks like the WPT TravisCI stability checker runs into problems when the
file /IndexedDB/support.js is changed (see
https://github.com/w3c/web-platform-tests/pull/4661#issuecomment-276418878). I'm
currently working with jgraham to diagnose the issue further.

Don't let this discourage you from changing that file - just please be advised
that this CL might take longer to get exported (caused by needing to reach out
and have a WPT maintainer force merge the PR manually).

[jsbell, 2017-02-07 21:50:17]

https://codereview.chromium.org/2673603003/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/external/wpt/IndexedDB/idb-binary-key-neutered.htm
(right):

https://codereview.chromium.org/2673603003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/IndexedDB/idb-binary-key-neutered.htm:3:
<title>IndexedDB: Neutered buffers supplied as binary keys</title>
Can you reword this from "neutered" to "detached"?

That's the preferred WebIDL/ECMAScript term now.

Also, rename the test file.

https://codereview.chromium.org/2673603003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/IndexedDB/idb-binary-key-neutered.htm:20:
// Neuter the ArrayBuffer by transferring it to a worker.
-> Detach

https://codereview.chromium.org/2673603003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/IndexedDB/idb-binary-key-neutered.htm:28:
'Neutered ArrayBuffer'
-> Detached

https://codereview.chromium.org/2673603003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/IndexedDB/idb-binary-key-neutered.htm:40:
// Neuter the ArrayBuffer by transferring it to a worker.
-> Detach

https://codereview.chromium.org/2673603003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/IndexedDB/idb-binary-key-neutered.htm:48:
'Neutered ArrayBufferView'
-> Detached

https://codereview.chromium.org/2673603003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/IndexedDB/idb-binary-key-neutered.htm:48:
'Neutered ArrayBufferView'
ArrayBufferView -> TypedArray

https://codereview.chromium.org/2673603003/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/external/wpt/IndexedDB/idb-binary-key-roundtrip.htm
(right):

https://codereview.chromium.org/2673603003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/IndexedDB/idb-binary-key-roundtrip.htm:18:
[].slice.call(new Uint8Array(a)),
Change this to Array.from()

https://codereview.chromium.org/2673603003/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/storage/indexeddb/key-type-binary.html
(left):

https://codereview.chromium.org/2673603003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/storage/indexeddb/key-type-binary.html:46:
found_key, buffer, 'Buffers should be equal to put buffer');
On 2017/02/03 02:38:40, pwnall wrote:
> This fails for views that start at a non-zero offset into the buffer.

Sounds like a bug! 

The conversion should be taking into account the offset/length of the view:

https://heycam.github.io/webidl/#es-buffer-source-types

Implementation is at:

https://cs.chromium.org/chromium/src/third_party/WebKit/Source/bindings/modul...

https://codereview.chromium.org/2673603003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/storage/indexeddb/key-type-binary.html:97:
value_test('Offset DataView', new DataView(buffer), 3, 3);
On 2017/02/03 02:38:40, pwnall wrote:
> IIUC, the "3, 3" arguments were meant to be passed to the DataView
constructor.
> value_test ignores them.

Yeah, I agree.

https://codereview.chromium.org/2673603003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/storage/indexeddb/key-type-binary.html:98:
value_test('Offset Uint8Array', new Uint8Array(buffer), 3, 3);
On 2017/02/03 02:38:40, pwnall wrote:
> Same issue as above.

Ditto.

[pwnall, 2017-02-08 00:23:25]

The CQ bit was checked by pwnall@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-08 00:24:15]

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

[pwnall, 2017-02-08 00:33:34]

Thank you very much for the thorough feedback!

https://codereview.chromium.org/2673603003/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/external/wpt/IndexedDB/idb-binary-key-neutered.htm
(right):

https://codereview.chromium.org/2673603003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/IndexedDB/idb-binary-key-neutered.htm:3:
<title>IndexedDB: Neutered buffers supplied as binary keys</title>
On 2017/02/07 21:50:17, jsbell wrote:
> Can you reword this from "neutered" to "detached"?
> 
> That's the preferred WebIDL/ECMAScript term now.
> 
> Also, rename the test file.

Done.
Cool, it sounds less creepy!

https://codereview.chromium.org/2673603003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/IndexedDB/idb-binary-key-neutered.htm:20:
// Neuter the ArrayBuffer by transferring it to a worker.
On 2017/02/07 21:50:17, jsbell wrote:
> -> Detach

Done.

https://codereview.chromium.org/2673603003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/IndexedDB/idb-binary-key-neutered.htm:28:
'Neutered ArrayBuffer'
On 2017/02/07 21:50:17, jsbell wrote:
> -> Detached

Done.

https://codereview.chromium.org/2673603003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/IndexedDB/idb-binary-key-neutered.htm:40:
// Neuter the ArrayBuffer by transferring it to a worker.
On 2017/02/07 21:50:17, jsbell wrote:
> -> Detach

Done.

https://codereview.chromium.org/2673603003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/IndexedDB/idb-binary-key-neutered.htm:48:
'Neutered ArrayBufferView'
On 2017/02/07 21:50:17, jsbell wrote:
> ArrayBufferView -> TypedArray

Done.

https://codereview.chromium.org/2673603003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/IndexedDB/idb-binary-key-neutered.htm:48:
'Neutered ArrayBufferView'
On 2017/02/07 21:50:17, jsbell wrote:
> -> Detached

Done.

https://codereview.chromium.org/2673603003/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/external/wpt/IndexedDB/idb-binary-key-roundtrip.htm
(right):

https://codereview.chromium.org/2673603003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/IndexedDB/idb-binary-key-roundtrip.htm:18:
[].slice.call(new Uint8Array(a)),
On 2017/02/07 21:50:17, jsbell wrote:
> Change this to Array.from()

Done.

https://codereview.chromium.org/2673603003/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/storage/indexeddb/key-type-binary.html
(left):

https://codereview.chromium.org/2673603003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/storage/indexeddb/key-type-binary.html:46:
found_key, buffer, 'Buffers should be equal to put buffer');
On 2017/02/07 21:50:17, jsbell wrote:
> On 2017/02/03 02:38:40, pwnall wrote:
> > This fails for views that start at a non-zero offset into the buffer.
> 
> Sounds like a bug! 
> 
> The conversion should be taking into account the offset/length of the view:
> 
> https://heycam.github.io/webidl/#es-buffer-source-types
> 
> Implementation is at:
> 
>
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/bindings/modul...
> 

Sorry for being unclear!

I meant that if the bug below is fixed, this check will start failing. I've
updated the code around the check so that we compare against the correct value.
Our implementation is correct here.

[pwnall, 2017-02-08 00:34:04]

Also, PTAL?

[commit-bot: I haz the power, 2017-02-08 01:45:27]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-08 01:45:28]

Dry run: This issue passed the CQ dry run.

[jsbell, 2017-02-09 01:02:34]

lgtm

[pwnall, 2017-02-09 01:06:15]

The CQ bit was checked by pwnall@chromium.org

[commit-bot: I haz the power, 2017-02-09 01:06:57]

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, 2017-02-09 05:27:03]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1486602375016700,
"parent_rev": "a0e5e40a1f939430bf21f60045bdf5bc28100f97", "commit_rev":
"c06d49ab041c8d203d0b451f9eb7251a5832db97"}

[commit-bot: I haz the power, 2017-02-09 05:28:26]

Description was changed from

==========
Upstream IndexedDB binary key tests to WPT.

BUG=683463
==========

to

==========
Upstream IndexedDB binary key tests to WPT.

BUG=683463

Review-Url: https://codereview.chromium.org/2673603003
Cr-Commit-Position: refs/heads/master@{#449206}
Committed:
https://chromium.googlesource.com/chromium/src/+/c06d49ab041c8d203d0b451f9eb7...
==========

[commit-bot: I haz the power, 2017-02-09 05:28:30]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/c06d49ab041c8d203d0b451f9eb7...