163986 – JSC should support SharedArrayBuffer

Bug 163986 - JSC should support SharedArrayBuffer

Summary: JSC should support SharedArrayBuffer

Status:	RESOLVED FIXED

Alias:	None

Product:	WebKit
Classification:	Unclassified
Component:	JavaScriptCore (show other bugs)
Version:	WebKit Nightly Build
Hardware:	All All

Importance:	P2 Normal
Assignee:	Filip Pizlo

URL:
Keywords:

Depends on:
Blocks:	164108
	Show dependency tree / graph

Reported:	2016-10-25 14:28 PDT by Filip Pizlo
Modified:	2021-07-09 03:13 PDT (History)
CC List:	9 users (show)

See Also:

Attachments
it begins (8.19 KB, patch) 2016-10-25 14:29 PDT, Filip Pizlo	no flags	Details \| Formatted Diff \| Diff
getting closer (22.98 KB, patch) 2016-10-25 16:09 PDT, Filip Pizlo	no flags	Details \| Formatted Diff \| Diff
more (31.49 KB, patch) 2016-10-25 21:37 PDT, Filip Pizlo	no flags	Details \| Formatted Diff \| Diff
more (66.48 KB, patch) 2016-10-26 12:54 PDT, Filip Pizlo	no flags	Details \| Formatted Diff \| Diff
a bit more (68.01 KB, patch) 2016-10-26 22:40 PDT, Filip Pizlo	no flags	Details \| Formatted Diff \| Diff
more (78.52 KB, patch) 2016-10-27 08:57 PDT, Filip Pizlo	no flags	Details \| Formatted Diff \| Diff
rebased patch (78.75 KB, patch) 2016-10-27 09:44 PDT, Filip Pizlo	no flags	Details \| Formatted Diff \| Diff
more (96.30 KB, patch) 2016-10-27 18:34 PDT, Filip Pizlo	no flags	Details \| Formatted Diff \| Diff
it's starting to compile (100.53 KB, patch) 2016-10-27 21:32 PDT, Filip Pizlo	no flags	Details \| Formatted Diff \| Diff
closer to compiling (110.86 KB, patch) 2016-10-28 09:34 PDT, Filip Pizlo	no flags	Details \| Formatted Diff \| Diff
jsc compiles (112.41 KB, patch) 2016-10-28 10:19 PDT, Filip Pizlo	no flags	Details \| Formatted Diff \| Diff
making the DOM happy (120.12 KB, patch) 2016-10-28 11:54 PDT, Filip Pizlo	no flags	Details \| Formatted Diff \| Diff
working through more DOM uses of typed arrays (127.21 KB, patch) 2016-10-28 12:50 PDT, Filip Pizlo	no flags	Details \| Formatted Diff \| Diff
more of the DOM compiles (128.63 KB, patch) 2016-10-28 13:24 PDT, Filip Pizlo	no flags	Details \| Formatted Diff \| Diff
more of the DOM compiles (137.91 KB, patch) 2016-10-28 21:14 PDT, Filip Pizlo	no flags	Details \| Formatted Diff \| Diff
the DOM compiles (139.46 KB, patch) 2016-10-29 11:13 PDT, Filip Pizlo	no flags	Details \| Formatted Diff \| Diff
it all builds (143.36 KB, patch) 2016-10-29 12:10 PDT, Filip Pizlo	no flags	Details \| Formatted Diff \| Diff
rebased patch (140.12 KB, patch) 2016-10-29 12:24 PDT, Filip Pizlo	no flags	Details \| Formatted Diff \| Diff
more fixes (140.88 KB, patch) 2016-10-29 13:55 PDT, Filip Pizlo	no flags	Details \| Formatted Diff \| Diff
it seems to work (185.85 KB, patch) 2016-10-29 20:52 PDT, Filip Pizlo	no flags	Details \| Formatted Diff \| Diff
rebased patch (185.83 KB, patch) 2016-10-29 20:56 PDT, Filip Pizlo	no flags	Details \| Formatted Diff \| Diff
better changelog (184.79 KB, patch) 2016-10-29 21:12 PDT, Filip Pizlo	buildbot: commit-queue-	Details \| Formatted Diff \| Diff
Archive of layout-test-results from ews103 for mac-yosemite (1021.14 KB, application/zip) 2016-10-29 22:17 PDT, Build Bot	no flags	Details
Archive of layout-test-results from ews105 for mac-yosemite-wk2 (931.98 KB, application/zip) 2016-10-29 22:18 PDT, Build Bot	no flags	Details
Archive of layout-test-results from ews117 for mac-yosemite (1009.45 KB, application/zip) 2016-10-29 22:21 PDT, Build Bot	no flags	Details
Archive of layout-test-results from ews125 for ios-simulator-wk2 (829.32 KB, application/zip) 2016-10-29 22:31 PDT, Build Bot	no flags	Details
some fixes (184.77 KB, patch) 2016-10-30 10:30 PDT, Filip Pizlo	no flags	Details \| Formatted Diff \| Diff
rebased (184.71 KB, patch) 2016-10-30 10:40 PDT, Filip Pizlo	no flags	Details \| Formatted Diff \| Diff
fix gcc build (184.77 KB, patch) 2016-10-30 10:46 PDT, Filip Pizlo	no flags	Details \| Formatted Diff \| Diff
the patch (184.94 KB, patch) 2016-10-30 10:50 PDT, Filip Pizlo	no flags	Details \| Formatted Diff \| Diff
the patch (185.00 KB, patch) 2016-10-30 10:59 PDT, Filip Pizlo	no flags	Details \| Formatted Diff \| Diff
the patch (184.97 KB, patch) 2016-10-30 11:09 PDT, Filip Pizlo	buildbot: commit-queue-	Details \| Formatted Diff \| Diff
Archive of layout-test-results from ews102 for mac-yosemite (1.12 MB, application/zip) 2016-10-30 12:30 PDT, Build Bot	no flags	Details
Archive of layout-test-results from ews105 for mac-yosemite-wk2 (1.12 MB, application/zip) 2016-10-30 12:33 PDT, Build Bot	no flags	Details
Archive of layout-test-results from ews113 for mac-yosemite (1.93 MB, application/zip) 2016-10-30 12:44 PDT, Build Bot	no flags	Details
Archive of layout-test-results from ews125 for ios-simulator-wk2 (19.84 MB, application/zip) 2016-10-30 12:47 PDT, Build Bot	no flags	Details
the patch (185.71 KB, patch) 2016-10-30 22:09 PDT, Filip Pizlo	buildbot: commit-queue-	Details \| Formatted Diff \| Diff
Archive of layout-test-results from ews103 for mac-yosemite (914.18 KB, application/zip) 2016-10-30 23:28 PDT, Build Bot	no flags	Details
Archive of layout-test-results from ews104 for mac-yosemite-wk2 (1.03 MB, application/zip) 2016-10-30 23:34 PDT, Build Bot	no flags	Details
Archive of layout-test-results from ews113 for mac-yosemite (1.58 MB, application/zip) 2016-10-30 23:35 PDT, Build Bot	no flags	Details
Archive of layout-test-results from ews123 for ios-simulator-wk2 (deleted) 2016-10-30 23:48 PDT, Build Bot	no flags	Details
the patch (187.42 KB, patch) 2016-10-31 08:52 PDT, Filip Pizlo	no flags	Details \| Formatted Diff \| Diff
the patch (187.38 KB, patch) 2016-10-31 09:20 PDT, Filip Pizlo	buildbot: commit-queue-	Details \| Formatted Diff \| Diff
Archive of layout-test-results from ews107 for mac-yosemite-wk2 (1.41 MB, application/zip) 2016-10-31 10:49 PDT, Build Bot	no flags	Details
more tests WIP (196.86 KB, patch) 2016-10-31 10:54 PDT, Filip Pizlo	no flags	Details \| Formatted Diff \| Diff
the patch (202.54 KB, patch) 2016-10-31 13:21 PDT, Filip Pizlo	no flags	Details \| Formatted Diff \| Diff
the patch (203.90 KB, patch) 2016-10-31 13:24 PDT, Filip Pizlo	buildbot: commit-queue-	Details \| Formatted Diff \| Diff
Archive of layout-test-results from ews106 for mac-yosemite-wk2 (1.33 MB, application/zip) 2016-10-31 15:03 PDT, Build Bot	no flags	Details
the patch (206.70 KB, patch) 2016-10-31 15:46 PDT, Filip Pizlo	keith_miller: review+	Details \| Formatted Diff \| Diff
Show Obsolete (48) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.

Description Filip Pizlo 2016-10-25 14:28:46 PDT

Patch forthcoming.

Comment 1 Filip Pizlo 2016-10-25 14:29:33 PDT

Created attachment 292820 [details]
it begins

Comment 2 Filip Pizlo 2016-10-25 14:29:50 PDT

Comment on attachment 292820 [details]
it begins

It's nowhere near ready for review.

Comment 3 WebKit Commit Bot 2016-10-25 14:31:57 PDT

Attachment 292820 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:36:  Missing space inside { }.  [whitespace/braces] [5]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

Comment 4 Filip Pizlo 2016-10-25 16:09:09 PDT

Created attachment 292841 [details]
getting closer

Comment 5 Filip Pizlo 2016-10-25 16:18:33 PDT

Relevant: https://github.com/tc39/ecmascript_sharedmem/issues/38#issuecomment-163048837

I think that we want to make it harder to unwrap an ArrayBuffer if you haven't made a statement about whether you're happy sharing.

Comment 6 Filip Pizlo 2016-10-25 21:37:27 PDT

Created attachment 292874 [details]
more

Comment 7 Filip Pizlo 2016-10-26 12:54:00 PDT

Created attachment 292955 [details]
more

Comment 8 Filip Pizlo 2016-10-26 22:40:51 PDT

Created attachment 292994 [details]
a bit more

Comment 9 Filip Pizlo 2016-10-27 08:57:35 PDT

Created attachment 293023 [details]
more

I'm renaming all of the functions that give native code access to a typed array's guts.  For each toBlahTypedArray()/toArrayBuffer() function, we will now have two:

toUnsharedBlah() -> same as the old toBlah() but asserts that the underlying buffer is unshared, or in the case of versions that dynamic cast, returns zero if the underlying buffer is unshared.
toPossiblySharedBlah() -> same as the old toBlah() but may return something that has memory that is aliased with another thread.

Comment 10 Filip Pizlo 2016-10-27 09:44:48 PDT

Created attachment 293028 [details]
rebased patch

Comment 11 Filip Pizlo 2016-10-27 18:34:54 PDT

Created attachment 293094 [details]
more

Comment 12 Filip Pizlo 2016-10-27 21:32:13 PDT

Created attachment 293113 [details]
it's starting to compile

Comment 13 Filip Pizlo 2016-10-28 09:34:52 PDT

Created attachment 293165 [details]
closer to compiling

Comment 14 Filip Pizlo 2016-10-28 10:19:07 PDT

Created attachment 293172 [details]
jsc compiles

Comment 15 Filip Pizlo 2016-10-28 11:54:00 PDT

Created attachment 293181 [details]
making the DOM happy

Comment 16 Filip Pizlo 2016-10-28 12:50:26 PDT

Created attachment 293188 [details]
working through more DOM uses of typed arrays

Comment 17 Filip Pizlo 2016-10-28 13:24:21 PDT

Created attachment 293195 [details]
more of the DOM compiles

Comment 18 Filip Pizlo 2016-10-28 21:14:20 PDT

Created attachment 293273 [details]
more of the DOM compiles

Comment 19 Filip Pizlo 2016-10-29 11:13:43 PDT

Created attachment 293299 [details]
the DOM compiles

Comment 20 Filip Pizlo 2016-10-29 12:10:40 PDT

Created attachment 293307 [details]
it all builds

It builds and SharedArrayBuffer is behind a runtime flag.  I haven't tested it yet.

Comment 21 Filip Pizlo 2016-10-29 12:24:38 PDT

Created attachment 293309 [details]
rebased patch

Comment 22 WebKit Commit Bot 2016-10-29 12:27:00 PDT

Attachment 293309 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:115:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:184:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:36:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/JSArrayBufferView.h:163:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/JSArrayBufferView.h:164:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 5 in 69 files


If any of these errors are false positives, please file a bug against check-webkit-style.

Comment 23 Filip Pizlo 2016-10-29 13:55:22 PDT

Created attachment 293311 [details]
more fixes

Comment 24 WebKit Commit Bot 2016-10-29 13:58:05 PDT

Attachment 293311 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:115:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:184:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:36:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/JSArrayBufferView.h:163:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/JSArrayBufferView.h:164:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 5 in 70 files


If any of these errors are false positives, please file a bug against check-webkit-style.

Comment 25 Filip Pizlo 2016-10-29 20:52:33 PDT

Created attachment 293324 [details]
it seems to work

Comment 26 Filip Pizlo 2016-10-29 20:56:47 PDT

Created attachment 293325 [details]
rebased patch

Comment 27 WebKit Commit Bot 2016-10-29 20:58:53 PDT

Attachment 293325 [details] did not pass style-queue:


ERROR: Source/WebCore/bindings/js/JSWorkerGlobalScopeBase.cpp:100:  Declaration has space between type name and * in const JSWorkerGlobalScopeBase *thisObject  [whitespace/declaration] [3]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:115:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:184:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:36:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/JSArrayBufferView.h:163:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/JSArrayBufferView.h:164:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 6 in 93 files


If any of these errors are false positives, please file a bug against check-webkit-style.

Comment 28 Filip Pizlo 2016-10-29 21:12:45 PDT

Created attachment 293326 [details]
better changelog

Comment 29 WebKit Commit Bot 2016-10-29 21:14:11 PDT

Attachment 293326 [details] did not pass style-queue:


ERROR: Source/WebCore/bindings/js/JSWorkerGlobalScopeBase.cpp:100:  Declaration has space between type name and * in const JSWorkerGlobalScopeBase *thisObject  [whitespace/declaration] [3]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:115:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:184:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:36:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/JSArrayBufferView.h:163:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/JSArrayBufferView.h:164:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 6 in 93 files


If any of these errors are false positives, please file a bug against check-webkit-style.

Comment 30 Build Bot 2016-10-29 22:17:06 PDT

Comment on attachment 293326 [details]
better changelog

Attachment 293326 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/2403208

Number of test failures exceeded the failure limit.

Comment 31 Build Bot 2016-10-29 22:17:10 PDT

Created attachment 293329 [details]
Archive of layout-test-results from ews103 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-yosemite  Platform: Mac OS X 10.10.5

Comment 32 Build Bot 2016-10-29 22:18:54 PDT

Comment on attachment 293326 [details]
better changelog

Attachment 293326 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/2403207

Number of test failures exceeded the failure limit.

Comment 33 Build Bot 2016-10-29 22:18:58 PDT

Created attachment 293330 [details]
Archive of layout-test-results from ews105 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5

Comment 34 Build Bot 2016-10-29 22:21:07 PDT

Comment on attachment 293326 [details]
better changelog

Attachment 293326 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/2403205

Number of test failures exceeded the failure limit.

Comment 35 Build Bot 2016-10-29 22:21:11 PDT

Created attachment 293331 [details]
Archive of layout-test-results from ews117 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117  Port: mac-yosemite  Platform: Mac OS X 10.10.5

Comment 36 Build Bot 2016-10-29 22:31:33 PDT

Comment on attachment 293326 [details]
better changelog

Attachment 293326 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/2403217

Number of test failures exceeded the failure limit.

Comment 37 Build Bot 2016-10-29 22:31:37 PDT

Created attachment 293333 [details]
Archive of layout-test-results from ews125 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews125  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.6

Comment 38 Filip Pizlo 2016-10-30 09:18:31 PDT

Ooops the crashes look like a missing null check.

Comment 39 Filip Pizlo 2016-10-30 10:30:41 PDT

Created attachment 293352 [details]
some fixes

toUnsharedArrayBufferView was missing a null check.

Comment 40 Filip Pizlo 2016-10-30 10:40:48 PDT

Created attachment 293353 [details]
rebased

Comment 41 WebKit Commit Bot 2016-10-30 10:42:52 PDT

Attachment 293353 [details] did not pass style-queue:


ERROR: Source/WebCore/bindings/js/JSWorkerGlobalScopeBase.cpp:101:  Declaration has space between type name and * in const JSWorkerGlobalScopeBase *thisObject  [whitespace/declaration] [3]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:115:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:184:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:36:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/JSArrayBufferView.h:163:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/JSArrayBufferView.h:164:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 6 in 93 files


If any of these errors are false positives, please file a bug against check-webkit-style.

Comment 42 Filip Pizlo 2016-10-30 10:46:04 PDT

Created attachment 293354 [details]
fix gcc build

Comment 43 WebKit Commit Bot 2016-10-30 10:48:07 PDT

Attachment 293354 [details] did not pass style-queue:


ERROR: Source/WebCore/bindings/js/JSWorkerGlobalScopeBase.cpp:101:  Declaration has space between type name and * in const JSWorkerGlobalScopeBase *thisObject  [whitespace/declaration] [3]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:115:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:184:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:36:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/JSArrayBufferView.h:163:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/JSArrayBufferView.h:164:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 6 in 93 files


If any of these errors are false positives, please file a bug against check-webkit-style.

Comment 44 Filip Pizlo 2016-10-30 10:50:01 PDT

Created attachment 293355 [details]
the patch

Comment 45 WebKit Commit Bot 2016-10-30 10:51:32 PDT

Attachment 293355 [details] did not pass style-queue:


ERROR: Source/WebCore/bindings/js/JSWorkerGlobalScopeBase.cpp:101:  Declaration has space between type name and * in const JSWorkerGlobalScopeBase *thisObject  [whitespace/declaration] [3]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:115:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:184:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:36:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/JSArrayBufferView.h:163:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/JSArrayBufferView.h:164:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 6 in 93 files


If any of these errors are false positives, please file a bug against check-webkit-style.

Comment 46 Filip Pizlo 2016-10-30 10:59:32 PDT

Created attachment 293356 [details]
the patch

Comment 47 WebKit Commit Bot 2016-10-30 11:02:45 PDT

Attachment 293356 [details] did not pass style-queue:


ERROR: Source/WebCore/bindings/js/JSWorkerGlobalScopeBase.cpp:101:  Declaration has space between type name and * in const JSWorkerGlobalScopeBase *thisObject  [whitespace/declaration] [3]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:115:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:184:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:36:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/JSArrayBufferView.h:163:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/JSArrayBufferView.h:164:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 6 in 93 files


If any of these errors are false positives, please file a bug against check-webkit-style.

Comment 48 Filip Pizlo 2016-10-30 11:09:19 PDT

Created attachment 293358 [details]
the patch

Comment 49 WebKit Commit Bot 2016-10-30 11:11:28 PDT

Attachment 293358 [details] did not pass style-queue:


ERROR: Source/WebCore/bindings/js/JSWorkerGlobalScopeBase.cpp:101:  Declaration has space between type name and * in const JSWorkerGlobalScopeBase *thisObject  [whitespace/declaration] [3]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:115:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:184:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:36:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/JSArrayBufferView.h:163:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/JSArrayBufferView.h:164:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 6 in 93 files


If any of these errors are false positives, please file a bug against check-webkit-style.

Comment 50 Filip Pizlo 2016-10-30 11:48:10 PDT

Looks like I messed up ArrayBuffer transfer for the non-shared case.

Comment 51 Build Bot 2016-10-30 12:30:03 PDT

Comment on attachment 293358 [details]
the patch

Attachment 293358 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/2406561

New failing tests:
js/typed-array-mutated-during-set.html
workers/sab/simple.html
fast/dom/Window/window-postmessage-args.html
fast/canvas/webgl/arraybuffer-transfer-of-control.html
js/dom/global-constructors-attributes-dedicated-worker.html
js/dom/dfg-byteOffset-neuter.html

Comment 52 Build Bot 2016-10-30 12:30:07 PDT

Created attachment 293362 [details]
Archive of layout-test-results from ews102 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-yosemite  Platform: Mac OS X 10.10.5

Comment 53 Build Bot 2016-10-30 12:33:08 PDT

Comment on attachment 293358 [details]
the patch

Attachment 293358 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/2406562

New failing tests:
js/dom/global-constructors-attributes.html
workers/sab/simple.html
js/dom/dfg-byteOffset-neuter.html
fast/canvas/webgl/arraybuffer-transfer-of-control.html
js/dom/global-constructors-attributes-dedicated-worker.html
fast/dom/Window/window-postmessage-args.html
js/typed-array-mutated-during-set.html

Comment 54 Build Bot 2016-10-30 12:33:13 PDT

Created attachment 293364 [details]
Archive of layout-test-results from ews105 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5

Comment 55 Build Bot 2016-10-30 12:44:51 PDT

Comment on attachment 293358 [details]
the patch

Attachment 293358 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/2406565

New failing tests:
js/typed-array-mutated-during-set.html
workers/sab/simple.html
fast/dom/Window/window-postmessage-args.html
fast/canvas/webgl/arraybuffer-transfer-of-control.html
js/dom/global-constructors-attributes-dedicated-worker.html
js/dom/dfg-byteOffset-neuter.html

Comment 56 Build Bot 2016-10-30 12:44:56 PDT

Created attachment 293366 [details]
Archive of layout-test-results from ews113 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews113  Port: mac-yosemite  Platform: Mac OS X 10.10.5

Comment 57 Build Bot 2016-10-30 12:47:36 PDT

Comment on attachment 293358 [details]
the patch

Attachment 293358 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/2406568

New failing tests:
js/dom/dfg-byteOffset-neuter.html
fast/dom/Window/window-postmessage-args.html
js/dom/global-constructors-attributes-dedicated-worker.html
js/typed-array-mutated-during-set.html
workers/sab/simple.html

Comment 58 Build Bot 2016-10-30 12:47:41 PDT

Created attachment 293367 [details]
Archive of layout-test-results from ews125 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews125  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.6

Comment 59 Filip Pizlo 2016-10-30 18:29:48 PDT

I'm debugging the crashes.  They are subtle.  An array buffer view's vector goes null at some point, don't know why yet.

Comment 60 Filip Pizlo 2016-10-30 18:49:07 PDT

(In reply to comment #59)
> I'm debugging the crashes.  They are subtle.  An array buffer view's vector
> goes null at some point, don't know why yet.

Found it.  ArrayBuffer::transferTo() needs to clear() this.  Failing to do so creates all manner of problems.

Basically, the vector was supposed to be null because of neutering, but our logic for handling neutered array buffers wasn't working right because the ArrayBuffer itself had not been neutered (because the of the missing clear call).

Comment 61 Filip Pizlo 2016-10-30 22:09:57 PDT

Created attachment 293391 [details]
the patch

I fixed the crash in neutering.

Comment 62 WebKit Commit Bot 2016-10-30 22:12:27 PDT

Attachment 293391 [details] did not pass style-queue:


ERROR: Source/WebCore/bindings/js/JSWorkerGlobalScopeBase.cpp:101:  Declaration has space between type name and * in const JSWorkerGlobalScopeBase *thisObject  [whitespace/declaration] [3]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:115:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:185:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:36:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/JSArrayBufferView.h:163:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/JSArrayBufferView.h:164:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 6 in 93 files


If any of these errors are false positives, please file a bug against check-webkit-style.

Comment 63 Build Bot 2016-10-30 23:28:25 PDT

Comment on attachment 293391 [details]
the patch

Attachment 293391 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/2409241

New failing tests:
js/dom/global-constructors-attributes-dedicated-worker.html
workers/sab/simple.html

Comment 64 Build Bot 2016-10-30 23:28:31 PDT

Created attachment 293392 [details]
Archive of layout-test-results from ews103 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-yosemite  Platform: Mac OS X 10.10.5

Comment 65 Build Bot 2016-10-30 23:34:52 PDT

Comment on attachment 293391 [details]
the patch

Attachment 293391 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/2409260

New failing tests:
js/dom/global-constructors-attributes.html
js/dom/global-constructors-attributes-dedicated-worker.html
media/modern-media-controls/volume-support/volume-support-click.html
workers/sab/simple.html

Comment 66 Build Bot 2016-10-30 23:34:57 PDT

Created attachment 293393 [details]
Archive of layout-test-results from ews104 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5

Comment 67 Build Bot 2016-10-30 23:35:09 PDT

Comment on attachment 293391 [details]
the patch

Attachment 293391 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/2409254

New failing tests:
js/dom/global-constructors-attributes-dedicated-worker.html
workers/sab/simple.html

Comment 68 Build Bot 2016-10-30 23:35:15 PDT

Created attachment 293394 [details]
Archive of layout-test-results from ews113 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews113  Port: mac-yosemite  Platform: Mac OS X 10.10.5

Comment 69 Build Bot 2016-10-30 23:48:01 PDT

Comment on attachment 293391 [details]
the patch

Attachment 293391 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/2409276

New failing tests:
js/dom/global-constructors-attributes-dedicated-worker.html
workers/sab/simple.html

Comment 70 Build Bot 2016-10-30 23:48:06 PDT

Created attachment 293395 [details]
Archive of layout-test-results from ews123 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.6

Comment 71 Filip Pizlo 2016-10-31 08:52:45 PDT

Created attachment 293423 [details]
the patch

Comment 72 WebKit Commit Bot 2016-10-31 08:56:05 PDT

Attachment 293423 [details] did not pass style-queue:


ERROR: Source/WebCore/bindings/js/JSWorkerGlobalScopeBase.cpp:101:  Declaration has space between type name and * in const JSWorkerGlobalScopeBase *thisObject  [whitespace/declaration] [3]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:115:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:185:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:36:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/JSArrayBufferView.h:163:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/JSArrayBufferView.h:164:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 6 in 95 files


If any of these errors are false positives, please file a bug against check-webkit-style.

Comment 73 Filip Pizlo 2016-10-31 09:20:40 PDT

Created attachment 293426 [details]
the patch

Comment 74 WebKit Commit Bot 2016-10-31 09:23:39 PDT

Attachment 293426 [details] did not pass style-queue:


ERROR: Source/WebCore/bindings/js/JSWorkerGlobalScopeBase.cpp:101:  Declaration has space between type name and * in const JSWorkerGlobalScopeBase *thisObject  [whitespace/declaration] [3]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:115:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:185:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:36:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/JSArrayBufferView.h:163:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/JSArrayBufferView.h:164:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 6 in 95 files


If any of these errors are false positives, please file a bug against check-webkit-style.

Comment 75 Build Bot 2016-10-31 10:49:19 PDT

Comment on attachment 293426 [details]
the patch

Attachment 293426 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/2411984

New failing tests:
js/dom/global-constructors-attributes.html

Comment 76 Build Bot 2016-10-31 10:49:25 PDT

Created attachment 293432 [details]
Archive of layout-test-results from ews107 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5

Comment 77 Filip Pizlo 2016-10-31 10:54:42 PDT

Created attachment 293434 [details]
more tests WIP

Comment 78 Keith Miller 2016-10-31 10:56:25 PDT

Comment on attachment 293426 [details]
the patch

View in context: https://bugs.webkit.org/attachment.cgi?id=293426&action=review

Some comments. Mostly looks good. I'll hold off on the final review until the tests are done though.

> Source/JavaScriptCore/ChangeLog:20
> +        There is also a Atomics object in global scope, which exposes sequentiall consistent atomic

typo: a Atomcis => an Atomics. sequentiall => sequentially.

> Source/JavaScriptCore/runtime/ArrayBuffer.cpp:109
> +        ASSERT(policy == DontInitialize);

Is this assert really necessary?

> Source/JavaScriptCore/runtime/ArrayBuffer.h:188
> +    // FIXME: We probably want to scale this by the shared ref count or something.

yikes, this seems unfortunate.

> Source/JavaScriptCore/runtime/AtomicsObject.cpp:322
> +    // FIXME: Stop using std::chrono.

File a bug! :)

Comment 79 Filip Pizlo 2016-10-31 11:00:08 PDT

(In reply to comment #78)
> Comment on attachment 293426 [details]
> the patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=293426&action=review
> 
> Some comments. Mostly looks good. I'll hold off on the final review until
> the tests are done though.
> 
> > Source/JavaScriptCore/ChangeLog:20
> > +        There is also a Atomics object in global scope, which exposes sequentiall consistent atomic
> 
> typo: a Atomcis => an Atomics. sequentiall => sequentially.

Fixed.

> 
> > Source/JavaScriptCore/runtime/ArrayBuffer.cpp:109
> > +        ASSERT(policy == DontInitialize);
> 
> Is this assert really necessary?

I think it was there before I moved that code from ArrayBuffer.h

> 
> > Source/JavaScriptCore/runtime/ArrayBuffer.h:188
> > +    // FIXME: We probably want to scale this by the shared ref count or something.
> 
> yikes, this seems unfortunate.
> 
> > Source/JavaScriptCore/runtime/AtomicsObject.cpp:322
> > +    // FIXME: Stop using std::chrono.
> 
> File a bug! :)

I think we have one.  I'll find it.

Comment 80 JF Bastien 2016-10-31 11:08:38 PDT

Comment on attachment 293426 [details]
the patch

View in context: https://bugs.webkit.org/attachment.cgi?id=293426&action=review

> Source/JavaScriptCore/ChangeLog:20
> +        There is also a Atomics object in global scope, which exposes sequentiall consistent atomic

Typo "sequential"

> Source/JavaScriptCore/runtime/ArrayBuffer.cpp:95
> +{

Should this set `m_shared = nullptr;` at the beginning? Or assert on it?

> Source/JavaScriptCore/runtime/ArrayBuffer.cpp:101
> +            m_data = nullptr;

It seems like setting `m_sizeInBytes = 0;` on both failure paths would be conservatively good.

> Source/JavaScriptCore/runtime/ArrayBufferSharingMode.h:33
> +    Default,

Why not "Unshared"? That seems more explicit.

> Source/JavaScriptCore/runtime/AtomicsObject.cpp:229
> +    case 3:

case 3 should return false, and case 4 should return true (along with 1 and 2).

> Source/JavaScriptCore/runtime/AtomicsObject.cpp:305
> +    double timeoutInNanoseconds = timeoutInMilliseconds * 1000;

Missing 1000?

> Source/JavaScriptCore/runtime/AtomicsObject.cpp:315
> +    if (timeoutInNanoseconds == timeoutInNanoseconds)

std::isnan

> Source/JavaScriptCore/runtime/AtomicsObject.cpp:316
> +        timeoutInNanoseconds = std::max(0., timeoutInNanoseconds);

This seems to be a spec thing: is -0. a valid thing here?

> Source/WTF/wtf/Atomics.h:-96
> -    ASSERT(bitwise_cast<std::atomic<T>*>(location)->is_lock_free() && "expected lock-free type");

I think you want to keep these asserts.

Comment 81 Filip Pizlo 2016-10-31 11:14:48 PDT

(In reply to comment #80)
> Comment on attachment 293426 [details]
> the patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=293426&action=review
> 
> > Source/JavaScriptCore/ChangeLog:20
> > +        There is also a Atomics object in global scope, which exposes sequentiall consistent atomic
> 
> Typo "sequential"

Fixed.

> 
> > Source/JavaScriptCore/runtime/ArrayBuffer.cpp:95
> > +{
> 
> Should this set `m_shared = nullptr;` at the beginning? Or assert on it?

I think it's already certain that this function is only called right after ArrayBufferContents is instantiated.

> 
> > Source/JavaScriptCore/runtime/ArrayBuffer.cpp:101
> > +            m_data = nullptr;
> 
> It seems like setting `m_sizeInBytes = 0;` on both failure paths would be
> conservatively good.

I made it call reset().

> 
> > Source/JavaScriptCore/runtime/ArrayBufferSharingMode.h:33
> > +    Default,
> 
> Why not "Unshared"? That seems more explicit.

To reflect the naming of the types.  It's ArrayBuffer, not UnsharedArrayBuffer, and "Default" seems to be closer to meaning "the ArrayBuffer that isn't special".

> 
> > Source/JavaScriptCore/runtime/AtomicsObject.cpp:229
> > +    case 3:
> 
> case 3 should return false, and case 4 should return true (along with 1 and
> 2).

Oops.

> 
> > Source/JavaScriptCore/runtime/AtomicsObject.cpp:305
> > +    double timeoutInNanoseconds = timeoutInMilliseconds * 1000;
> 
> Missing 1000?

Oops.

> 
> > Source/JavaScriptCore/runtime/AtomicsObject.cpp:315
> > +    if (timeoutInNanoseconds == timeoutInNanoseconds)
> 
> std::isnan

== is a fundamental operator in C++ that already answers this question.  I don't think we should mandate using helpers for operations that are already primitive.

> 
> > Source/JavaScriptCore/runtime/AtomicsObject.cpp:316
> > +        timeoutInNanoseconds = std::max(0., timeoutInNanoseconds);
> 
> This seems to be a spec thing: is -0. a valid thing here?

The spec wants +0 here, but it doesn't matter.

> 
> > Source/WTF/wtf/Atomics.h:-96
> > -    ASSERT(bitwise_cast<std::atomic<T>*>(location)->is_lock_free() && "expected lock-free type");
> 
> I think you want to keep these asserts.

I removed them because I didn't want this noise everywhere.  If we did want an assertion about the lock freedom of primitive types, we should probably put them somewhere else.

Comment 82 Filip Pizlo 2016-10-31 13:21:36 PDT

Created attachment 293459 [details]
the patch

Comment 83 WebKit Commit Bot 2016-10-31 13:24:28 PDT

Attachment 293459 [details] did not pass style-queue:


ERROR: Source/WebCore/bindings/js/JSWorkerGlobalScopeBase.cpp:101:  Declaration has space between type name and * in const JSWorkerGlobalScopeBase *thisObject  [whitespace/declaration] [3]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:115:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:185:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:36:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/JSArrayBufferView.h:163:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/JSArrayBufferView.h:164:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 6 in 105 files


If any of these errors are false positives, please file a bug against check-webkit-style.

Comment 84 Filip Pizlo 2016-10-31 13:24:41 PDT

Created attachment 293460 [details]
the patch

Comment 85 WebKit Commit Bot 2016-10-31 13:27:58 PDT

Attachment 293460 [details] did not pass style-queue:


ERROR: Source/WebCore/bindings/js/JSWorkerGlobalScopeBase.cpp:101:  Declaration has space between type name and * in const JSWorkerGlobalScopeBase *thisObject  [whitespace/declaration] [3]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:115:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:185:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:36:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/JSArrayBufferView.h:163:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/JSArrayBufferView.h:164:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 6 in 106 files


If any of these errors are false positives, please file a bug against check-webkit-style.

Comment 86 Build Bot 2016-10-31 15:02:55 PDT

Comment on attachment 293460 [details]
the patch

Attachment 293460 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/2414059

New failing tests:
js/dom/global-constructors-attributes.html

Comment 87 Build Bot 2016-10-31 15:03:01 PDT

Created attachment 293473 [details]
Archive of layout-test-results from ews106 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5

Comment 88 Filip Pizlo 2016-10-31 15:46:09 PDT

Created attachment 293478 [details]
the patch

Comment 89 WebKit Commit Bot 2016-10-31 15:48:51 PDT

Attachment 293478 [details] did not pass style-queue:


ERROR: Source/WebCore/bindings/js/JSWorkerGlobalScopeBase.cpp:101:  Declaration has space between type name and * in const JSWorkerGlobalScopeBase *thisObject  [whitespace/declaration] [3]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:115:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:185:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:36:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/JSArrayBufferView.h:163:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/JSArrayBufferView.h:164:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 6 in 108 files


If any of these errors are false positives, please file a bug against check-webkit-style.

Comment 90 Keith Miller 2016-10-31 16:18:51 PDT

Comment on attachment 293478 [details]
the patch

r=me. Assuming the bots are happy and you intend on importing more tests since this current testing seems a little light.

Comment 91 Filip Pizlo 2016-10-31 17:38:03 PDT

This patch passes everything for me locally.  I think that EWS is seeing unrelated crashes.  But I'll wait a bit before landing.

Comment 92 Filip Pizlo 2016-10-31 20:13:32 PDT

Landed in https://trac.webkit.org/changeset/208209

Comment 93 Simon Fraser (smfr) 2016-10-31 22:24:15 PDT

This patch broke bindings tests:
https://build.webkit.org/builders/Apple%20Sierra%20Release%20WK1%20%28Tests%29/builds/1208

Fixing.