Improve Crypto::getRandomValues exception messages.

Improve Crypto::getRandomValues exception messages.

This CL improves the detail available in the Crypto::getRandomValues exceptions
generated when the array is either too long, nonexistent, or incorrectly typed.

The interesting bit of the CL is, however, not the new error message, but the
mechanism by which it's generated. ExceptionState may now have some additional
context passed in via the constructor, which allows us to generate the
boilerplate bits of the exception message ("Failed to execute XXX on YYY:")
inside ExceptionState rather than out in the calling code.

This isn't a huge deal for Crypto, but it will substantially reduce the
duplication we're currently forcing developers into when throwing
detailed exceptions, as we'll be able to set these values once and
only once (eventually in the generated bindings) rather than for each
exception individually.

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

[Mike West, 2013-11-26 10:21:27]

Jochen, Kentaro-san, mind taking a look at this? I think it's an improvement on
the current approach to generating boilerplate for exception messages.

The end goal would be to change all the ExceptionState constructors in generated
bindings, but I'm starting small here with one of the ExceptionState objects in
custom bindings.

Thanks!

-mike

[haraken, 2013-11-26 10:49:36]

https://codereview.chromium.org/87963002/diff/1/Source/bindings/v8/ExceptionS...
File Source/bindings/v8/ExceptionState.h (right):

https://codereview.chromium.org/87963002/diff/1/Source/bindings/v8/ExceptionS...
Source/bindings/v8/ExceptionState.h:113: const char* m_interface;

Don't you want to use String instead of char*?

[Mike West, 2013-11-26 10:53:26]

Thanks for taking a look!

https://codereview.chromium.org/87963002/diff/1/Source/bindings/v8/ExceptionS...
File Source/bindings/v8/ExceptionState.h (right):

https://codereview.chromium.org/87963002/diff/1/Source/bindings/v8/ExceptionS...
Source/bindings/v8/ExceptionState.h:113: const char* m_interface;
On 2013/11/26 10:49:36, haraken wrote:
> 
> Don't you want to use String instead of char*?

I don't want the performance hit of converting to String unless we actually
throw an exception. This sort of thing has already popped up in a number of
microbenchmarks as potentially problematic, so I'm trying to preemptively avoid
it here.

[haraken, 2013-11-26 11:14:27]

> > Don't you want to use String instead of char*?
> 
> I don't want the performance hit of converting to String unless we actually
> throw an exception. This sort of thing has already popped up in a number of
> microbenchmarks as potentially problematic, so I'm trying to preemptively
avoid
> it here.

Makes sense.

I was also thinking about performance. As far as I can tell, ExceptionState is
used by DOM methods and non-performance-sensitive getters/setters, so I hope
that adding a couple of members to ExceptionState won't affect performance.
Let's watch perf bots when we use the new ExceptionState in generated bindings.

LGTM.

[Mike West, 2013-11-26 12:22:01]

+arv, now that I think about it. :)

WDYT about this approach?

[jochen (gone - plz use gerrit), 2013-11-27 11:50:28]

lgtm

[commit-bot: I haz the power, 2013-11-27 12:01:01]

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

[commit-bot: I haz the power, 2013-11-27 12:32:48]

Retried try job too often on linux_blink for step(s) blink_platform_unittests,
webkit_lint, webkit_python_tests, webkit_tests, webkit_unit_tests, wtf_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_blin...

[commit-bot: I haz the power, 2013-11-27 12:52:13]

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

[commit-bot: I haz the power, 2013-11-27 12:59:04]

Failed to trigger a try job on win_layout
HTTP Error 400: Bad Request

[commit-bot: I haz the power, 2013-11-27 12:59:10]

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

[commit-bot: I haz the power, 2013-11-27 13:23:06]

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

[commit-bot: I haz the power, 2013-11-27 14:17:11]

Change committed as 162761

[arv (Not doing code reviews), 2013-11-27 15:22:49]

Did this have any performance implications? I intentionally kept ExceptionState
as small as possible because these gets created a lot in really hot code paths
where a single instruction is visible in benchmarks.

[Mike West, 2013-11-27 15:26:20]

On 2013/11/27 15:22:49, arv wrote:
> Did this have any performance implications? I intentionally kept
ExceptionState
> as small as possible because these gets created a lot in really hot code paths
> where a single instruction is visible in benchmarks.

I'm hoping the answer is "not enough to make a difference".

I don't expect it to make a big difference, as it boils down to two extra stores
in the general case, but given the general microbenchmark weirdness my plan is
to just watch the perf bots.

[arv (Not doing code reviews), 2013-11-27 15:26:25]

LGTM. I like this approach. Lets hope Dromaeo does not go bonkers.