Transfer WebCrypto databuffers across the Blink API using blink::WebVector.

Transfer WebCrypto databuffers across the Blink API using blink::WebVector.

This addresses a TODO added by https://codereview.chromium.org/2141843002/, to transfer ownership rather than copy them.

Committed: https://crrev.com/eeb3a0e242614c79ce7114c8f2e7273ee9fd3318
Cr-Commit-Position: refs/heads/master@{#406664}

[eroman, 2016-07-19 04:22:46]

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

[commit-bot: I haz the power, 2016-07-19 04:23:01]

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

[eroman, 2016-07-19 04:35:14]

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

[commit-bot: I haz the power, 2016-07-19 04:35:26]

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

[eroman, 2016-07-19 04:37:48]

Description was changed from

==========
Transfer WebCrypto databuffers across the Blink API using blink::WebVector.

This addresses a TODO added by https://codereview.chromium.org/2141843002/.
==========

to

==========
Transfer WebCrypto databuffers across the Blink API using blink::WebVector.

This addresses a TODO added by https://codereview.chromium.org/2141843002/, to
transfer ownership rather than copy them.
==========

[eroman, 2016-07-19 04:49:01]

eroman@chromium.org changed reviewers:
+ jochen@chromium.org

[commit-bot: I haz the power, 2016-07-19 07:15:18]

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

[commit-bot: I haz the power, 2016-07-19 07:15:19]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[jochen (gone - plz use gerrit), 2016-07-19 12:27:41]

https://codereview.chromium.org/2160943003/diff/20001/third_party/WebKit/publ...
File third_party/WebKit/public/platform/WebCrypto.h (right):

https://codereview.chromium.org/2160943003/diff/20001/third_party/WebKit/publ...
third_party/WebKit/public/platform/WebCrypto.h:201: virtual void encrypt(const
WebCryptoAlgorithm&, const WebCryptoKey&, WebVector<unsigned char> data,
WebCryptoResult result) {
result.completeWithError(WebCryptoErrorTypeNotSupported, ""); }
why not WebVector<unsigned char>&

[eroman, 2016-07-19 16:17:41]

https://codereview.chromium.org/2160943003/diff/20001/third_party/WebKit/publ...
File third_party/WebKit/public/platform/WebCrypto.h (right):

https://codereview.chromium.org/2160943003/diff/20001/third_party/WebKit/publ...
third_party/WebKit/public/platform/WebCrypto.h:201: virtual void encrypt(const
WebCryptoAlgorithm&, const WebCryptoKey&, WebVector<unsigned char> data,
WebCryptoResult result) {
result.completeWithError(WebCryptoErrorTypeNotSupported, ""); }
On 2016/07/19 12:27:40, jochen wrote:
> why not WebVector<unsigned char>&

My preference for pass-by-value is:
  * Makes it clear that it is an input parameter (and not an out or in/out
parameter)
  * Self-describes the "pass ownership" semantics
  * [weaker argument] Harder to mis-use, as the implementation cannot
accidentally save a reference to variable owned by caller.

For the other direction, the main advantage I can think of for pass-by-reference
approach, is it makes it harder to accidentally create a copy of vectors when
Blink calls into this interface. This is a consequence of WebVector having
implicit single-parameter constructors that make *copies* of things :(

For instance, in an earlier patchset I was passing in std::move(x) as expected,
and it looked correct. However, "x" was of type "Vector" rather than WeVector.
What this meant is that in reality it was calling into the WebVector(const
Vector& x) constructor, and hence making a copy instead of transferring.

I think the best fix for the problem above is to mark the constructors as
explicit as per the Chromium style guide. In the meantime though, making the
databuffer parameters be of type WebVector& would also catch such bugs since you
cannot take references of temporaries so that would have been a compile failure
I believe.

WDYT?

[jochen (gone - plz use gerrit), 2016-07-20 10:51:58]

lgtm

[eroman, 2016-07-20 19:09:15]

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

[commit-bot: I haz the power, 2016-07-20 19:10:06]

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

[eroman, 2016-07-20 19:50:06]

The CQ bit was unchecked by eroman@chromium.org

[eroman, 2016-07-20 19:50:13]

The CQ bit was checked by eroman@chromium.org

[eroman, 2016-07-20 19:50:13]

The patchset sent to the CQ was uploaded after l-g-t-m from jochen@chromium.org
Link to the patchset: https://codereview.chromium.org/2160943003/#ps40001
(title: "rebase")

[commit-bot: I haz the power, 2016-07-20 19:50:44]

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-07-20 20:56:18]

Description was changed from

==========
Transfer WebCrypto databuffers across the Blink API using blink::WebVector.

This addresses a TODO added by https://codereview.chromium.org/2141843002/, to
transfer ownership rather than copy them.
==========

to

==========
Transfer WebCrypto databuffers across the Blink API using blink::WebVector.

This addresses a TODO added by https://codereview.chromium.org/2141843002/, to
transfer ownership rather than copy them.
==========

[commit-bot: I haz the power, 2016-07-20 20:56:20]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-07-20 20:59:18]

Description was changed from

==========
Transfer WebCrypto databuffers across the Blink API using blink::WebVector.

This addresses a TODO added by https://codereview.chromium.org/2141843002/, to
transfer ownership rather than copy them.
==========

to

==========
Transfer WebCrypto databuffers across the Blink API using blink::WebVector.

This addresses a TODO added by https://codereview.chromium.org/2141843002/, to
transfer ownership rather than copy them.

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

[commit-bot: I haz the power, 2016-07-20 20:59:19]

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