Refactor to share more code between OpenSSL and NSS implementations.

content/renderer/webcrypto/crypto_data.h

+// Copyright (c) 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CONTENT_RENDERER_WEBCRYPTO_CRYPTO_DATA_
+#define CONTENT_RENDERER_WEBCRYPTO_CRYPTO_DATA_
+
+#include <string>
+#include <vector>
+
+#include "base/basictypes.h"
+#include "content/common/content_export.h"
+
+namespace blink {
+class WebArrayBuffer;
+}
+
+namespace content {
+
+namespace webcrypto {
+
+// Helper to pass around a range of immutable bytes. This is conceptually
+// similar to StringPiece, but for crypto byte data.

[Ryan Sleevi, 2014/02/07 01:19:21]

base::StringPiece

[eroman, 2014/02/07 21:15:57]

Done.

+//
+// The data used at construction is NOT owned, and should remain valid as long
+// as the CryptoData is being used.
+class CONTENT_EXPORT CryptoData {
+ public:
+  // Constructs empty data.
+  CryptoData();
+
+  CryptoData(const unsigned char* bytes, unsigned int byte_length);
+
+  // Allow implicit conversions for convenience.
+  CryptoData(const std::vector<unsigned char>& bytes);
+  CryptoData(const std::string& bytes);
+  CryptoData(const blink::WebArrayBuffer& buffer);

[Ryan Sleevi, 2014/02/07 01:19:21]

style guide says: boo

http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Explic...

[eroman, 2014/02/07 21:15:57]

I am aware of the style guide, however I was modelling after base::StringPiece
which uses implicit constructors. I will switch it to explicit and upload as
separate patchset, since it has a large impact on the unittests (which largely
work with std::vectors).

+
+  const unsigned char* bytes() const { return bytes_; }
+  unsigned int byte_length() const { return byte_length_; }
+
+ private:
+  const unsigned char* bytes_;
+  const unsigned int byte_length_;
+
+  // Copying would not be safe, since the data is not owned.
+  DISALLOW_COPY_AND_ASSIGN(CryptoData);

[Ryan Sleevi, 2014/02/07 01:19:21]

This comment doesn't make any sense. Just because the data isn't owned doesn't
mean it can't be copied.

StringPiece can be copied. It can also slice data.

[eroman, 2014/02/07 21:15:57]

Copies are dangerous, since they encourage developpers to pass around copies,
perhaps not realizing that the underlying memory is now owned.

I'll remove the comment, but I stand by it as being a meaningful restriction.

[Ryan Sleevi, 2014/02/07 21:18:49]

I still don't believe this does anything to reduce the danger. It's still
incredibly easy to misuse this class

CryptoData foo(SomethingReturnsAString());
foo.bytes();  // INVALID

If you're going to have a class not take ownership (which is fine, I think),
then I don't think it makes sense to suggest that some things are more dangerous
than others, given how much you're allowing for potential misuse.

+};
+
+}  // namespace webcrypto
+
+}  // namespace content
+
+#endif  // CONTENT_RENDERER_WEBCRYPTO_CRYPTO_DATA_