WIP: Migrate generated bindings to new ExceptionState constructor.

Migrate generated bindings to new ExceptionState constructor.

This patch touches the first few ExceptionState constructors in the generated
bindings. Subsequent patches will update the remainder.

BUG=270033
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=163178

[Mike West, 2013-12-03 15:09:24]

Jochen, would you mind taking a quick look at this tiny and trivial patch?
*cough*

Sigbjørn also sounded interested; please do feel ever so free to add comments.
:)

-mike

[sof, 2013-12-03 16:04:27]

https://codereview.chromium.org/99083002/diff/110001/LayoutTests/fast/events/...
File LayoutTests/fast/events/message-port-clone.html (right):

https://codereview.chromium.org/99083002/diff/110001/LayoutTests/fast/events/...
LayoutTests/fast/events/message-port-clone.html:22:
shouldThrow("channel2.port1.postMessage('msg', [channel.port2])",
'"DataCloneError: Failed to execute \'postMessage\' on \'MessagePort\': Item #0
in the array of ports is already neutered."');
Can't we just do away with the duplication of exception message here & keep it
in -expected.txt only? If so, deprecating/remvoing shouldThrow()'s 2nd argument
could be handled in a later CL.

https://codereview.chromium.org/99083002/diff/110001/Source/bindings/tests/re...
File Source/bindings/tests/results/V8Float64Array.cpp (left):

https://codereview.chromium.org/99083002/diff/110001/Source/bindings/tests/re...
Source/bindings/tests/results/V8Float64Array.cpp:88:
throwTypeError(ExceptionMessages::failedToExecute("foo", "Float64Array",
ExceptionMessages::notEnoughArguments(1, info.Length())), info.GetIsolate());
Not for here, but reusing the exceptionState above seems worth doing.

https://codereview.chromium.org/99083002/diff/110001/Source/bindings/tests/re...
File Source/bindings/tests/results/V8SupportTestInterface.cpp (right):

https://codereview.chromium.org/99083002/diff/110001/Source/bindings/tests/re...
Source/bindings/tests/results/V8SupportTestInterface.cpp:116: ExceptionState
exceptionState(ExceptionState::SetterContext, "supplementalStaticAttr",
"SupportTestInterface", info.Holder(), info.GetIsolate());
Unused? Seems a pity to stack allocate these if not passed along anywhere.

https://codereview.chromium.org/99083002/diff/110001/Source/core/css/CSSGroup...
File Source/core/css/CSSGroupingRule.cpp (right):

https://codereview.chromium.org/99083002/diff/110001/Source/core/css/CSSGroup...
Source/core/css/CSSGroupingRule.cpp:64:
exceptionState.throwDOMException(IndexSizeError, "the index " +
String::number(index) + " must be less than or equal to the length of the rule
list.");
"the" => "The" is perhaps consistent with capitalization used by other APIs; not
sure if such consistency matters.

[Mike West, 2013-12-04 08:42:18]

Thanks for your comments, Sigbjørn! Responses inline.

https://codereview.chromium.org/99083002/diff/110001/LayoutTests/fast/events/...
File LayoutTests/fast/events/message-port-clone.html (right):

https://codereview.chromium.org/99083002/diff/110001/LayoutTests/fast/events/...
LayoutTests/fast/events/message-port-clone.html:22:
shouldThrow("channel2.port1.postMessage('msg', [channel.port2])",
'"DataCloneError: Failed to execute \'postMessage\' on \'MessagePort\': Item #0
in the array of ports is already neutered."');
On 2013/12/03 16:04:29, sof wrote:
> Can't we just do away with the duplication of exception message here & keep it
> in -expected.txt only? If so, deprecating/remvoing shouldThrow()'s 2nd
argument
> could be handled in a later CL.

It's valuable to have a test fail if the exception's text unexpectedly changes,
and it's very easy to overlook changes if the line of the expectation file still
reads PASS. That said, testing the text on every thrown exception is massive
overkill, and a pain in the ass for cleanup CLs like this one.

I'd like to get to a place where we have one and only one of these two-argument
'shouldThrow()' assertions for each exception we throw. We're not there yet.
Until we are, I'd prefer to keep the two-argument 'shouldThrow()' calls (but I'd
gladly review CLs that set up some sort of "canonical thrown exception" check
and removed the text expectation from other instances.

+arv, who has opinions.

https://codereview.chromium.org/99083002/diff/110001/Source/bindings/tests/re...
File Source/bindings/tests/results/V8Float64Array.cpp (left):

https://codereview.chromium.org/99083002/diff/110001/Source/bindings/tests/re...
Source/bindings/tests/results/V8Float64Array.cpp:88:
throwTypeError(ExceptionMessages::failedToExecute("foo", "Float64Array",
ExceptionMessages::notEnoughArguments(1, info.Length())), info.GetIsolate());
On 2013/12/03 16:04:29, sof wrote:
> Not for here, but reusing the exceptionState above seems worth doing.

Yes. I'll fix these in a subsequent CL (or you can do it, if you like! :) ).

https://codereview.chromium.org/99083002/diff/110001/Source/bindings/tests/re...
File Source/bindings/tests/results/V8SupportTestInterface.cpp (right):

https://codereview.chromium.org/99083002/diff/110001/Source/bindings/tests/re...
Source/bindings/tests/results/V8SupportTestInterface.cpp:116: ExceptionState
exceptionState(ExceptionState::SetterContext, "supplementalStaticAttr",
"SupportTestInterface", info.Holder(), info.GetIsolate());
On 2013/12/03 16:04:29, sof wrote:
> Unused? Seems a pity to stack allocate these if not passed along anywhere.

Hrm. Yes, we used to have checks to create the object only when we used it. I
think I inadvertently dropped those when moving things around.

https://codereview.chromium.org/99083002/diff/110001/Source/core/css/CSSGroup...
File Source/core/css/CSSGroupingRule.cpp (right):

https://codereview.chromium.org/99083002/diff/110001/Source/core/css/CSSGroup...
Source/core/css/CSSGroupingRule.cpp:64:
exceptionState.throwDOMException(IndexSizeError, "the index " +
String::number(index) + " must be less than or equal to the length of the rule
list.");
On 2013/12/03 16:04:29, sof wrote:
> "the" => "The" is perhaps consistent with capitalization used by other APIs;
not
> sure if such consistency matters.

It does matter. I stopped caring about changing it about halfway through this
CL, as it would generate more changes in expectation files, and I was trying to
keep things small. We can go through and do a capitalization cleanup in a
subsequent CL (it's not terribly important :) ).

[Mike West, 2013-12-04 09:31:04]

New patch! ExceptionState should now only be generated when it's relevant, and
hopefully Linux will now access less random memory. :)

[jochen (gone - plz use gerrit), 2013-12-04 10:44:30]

lgtm

[sof, 2013-12-04 10:48:36]

https://codereview.chromium.org/99083002/diff/140001/Source/bindings/tests/re...
File Source/bindings/tests/results/V8TestObjectPython.cpp (right):

https://codereview.chromium.org/99083002/diff/140001/Source/bindings/tests/re...
Source/bindings/tests/results/V8TestObjectPython.cpp:3143: ExceptionState
exceptionState(ExceptionState::ExecutionContext, "voidMethod",
"TestObjectPython", info.Holder(), info.GetIsolate());
Is this still expected? (File not regenerated since the code generator was
updated, perhaps?)

[Mike West, 2013-12-04 13:17:19]

On 2013/12/04 10:48:36, sof wrote:
>
https://codereview.chromium.org/99083002/diff/140001/Source/bindings/tests/re...
> File Source/bindings/tests/results/V8TestObjectPython.cpp (right):
> 
>
https://codereview.chromium.org/99083002/diff/140001/Source/bindings/tests/re...
> Source/bindings/tests/results/V8TestObjectPython.cpp:3143: ExceptionState
> exceptionState(ExceptionState::ExecutionContext, "voidMethod",
> "TestObjectPython", info.Holder(), info.GetIsolate());
> Is this still expected? (File not regenerated since the code generator was
> updated, perhaps?)

Hrm. No, I missed that one too. Fixing now.

Thanks both of you for the review comments.

[commit-bot: I haz the power, 2013-12-04 13:21:48]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mkwst@chromium.org/99083002/150001

[commit-bot: I haz the power, 2013-12-04 13:23:28]

Failed to apply patch for
LayoutTests/http/tests/websocket/tests/hybi/bad-sub-protocol-control-chars-expected.txt:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  can't find file to patch at input line 6
  Perhaps you used the wrong -p or --strip option?
  The text leading up to this was:
  --------------------------
  |Index:
LayoutTests/http/tests/websocket/tests/hybi/bad-sub-protocol-control-chars-expected.txt
  |diff --git
a/LayoutTests/http/tests/websocket/tests/hybi/bad-sub-protocol-control-chars-expected.txt
b/LayoutTests/http/tests/websocket/tests/hybi/bad-sub-protocol-control-chars-expected.txt
  |index
c24c6c88d7fe0f7bf142d97e1d8c778a5bfe2332..aa3d3500b750e4417ebee6750a58e50bc81608d0
100644
  |---
a/LayoutTests/http/tests/websocket/tests/hybi/bad-sub-protocol-control-chars-expected.txt
  |+++
b/LayoutTests/http/tests/websocket/tests/hybi/bad-sub-protocol-control-chars-expected.txt
  --------------------------
  No file to patch.  Skipping patch.
  1 out of 1 hunk ignored

Patch:      
LayoutTests/http/tests/websocket/tests/hybi/bad-sub-protocol-control-chars-expected.txt
Index:
LayoutTests/http/tests/websocket/tests/hybi/bad-sub-protocol-control-chars-expected.txt
diff --git
a/LayoutTests/http/tests/websocket/tests/hybi/bad-sub-protocol-control-chars-expected.txt
b/LayoutTests/http/tests/websocket/tests/hybi/bad-sub-protocol-control-chars-expected.txt
index
c24c6c88d7fe0f7bf142d97e1d8c778a5bfe2332..aa3d3500b750e4417ebee6750a58e50bc81608d0
100644
---
a/LayoutTests/http/tests/websocket/tests/hybi/bad-sub-protocol-control-chars-expected.txt
+++
b/LayoutTests/http/tests/websocket/tests/hybi/bad-sub-protocol-control-chars-expected.txt
@@ -2,13 +2,13 @@ Test WebSocket bad sub-protocol names by control characters.
 
 On success, you will see a series of "PASS" messages, followed by "TEST
COMPLETE".
 
-PASS new WebSocket("ws://127.0.0.1:8880/simple", "\0") threw exception
SyntaxError: Failed to execute 'connect' on 'WebSocket': The subprotocol
'\u0000' is invalid..
-PASS new WebSocket("ws://127.0.0.1:8880/simple", "	") threw exception
SyntaxError: Failed to execute 'connect' on 'WebSocket': The subprotocol
'\u0009' is invalid..
+PASS new WebSocket("ws://127.0.0.1:8880/simple", "\0") threw exception
SyntaxError: The subprotocol '\u0000' is invalid..
+PASS new WebSocket("ws://127.0.0.1:8880/simple", "	") threw exception
SyntaxError: The subprotocol '\u0009' is invalid..
 PASS new WebSocket("ws://127.0.0.1:8880/simple", "
 ") threw exception SyntaxError: Unexpected token ILLEGAL.
 PASS new WebSocket("ws://127.0.0.1:8880/simple", "
 ") threw exception SyntaxError: Unexpected token ILLEGAL.
-PASS new WebSocket("ws://127.0.0.1:8880/simple", "") threw exception
SyntaxError: Failed to execute 'connect' on 'WebSocket': The subprotocol
'\u001B' is invalid..
+PASS new WebSocket("ws://127.0.0.1:8880/simple", "") threw exception
SyntaxError: The subprotocol '\u001B' is invalid..
 PASS successfullyParsed is true
 
 TEST COMPLETE

[commit-bot: I haz the power, 2013-12-04 13:51:37]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mkwst@chromium.org/99083002/170001

[commit-bot: I haz the power, 2013-12-04 15:47:19]

Change committed as 163178