Add sanity check to generateKey to make sure usages aren't empty

content/child/webcrypto/algorithm_dispatch.cc

Index: content/child/webcrypto/algorithm_dispatch.cc
diff --git a/content/child/webcrypto/algorithm_dispatch.cc b/content/child/webcrypto/algorithm_dispatch.cc
index 1c9b409ace58e4cf87aa1dfa7014a4e46bb6aec4..e622e925319b2c0985338c90829095aa68132acd 100644
--- a/content/child/webcrypto/algorithm_dispatch.cc
+++ b/content/child/webcrypto/algorithm_dispatch.cc
@@ -111,7 +111,23 @@ Status GenerateKey(const blink::WebCryptoAlgorithm& algorithm,
   if (status.IsError())
     return status;
 
-  return impl->GenerateKey(algorithm, extractable, usages, result);
+  status = impl->GenerateKey(algorithm, extractable, usages, result);
+  if (status.IsSuccess()) {

[eroman, 2014/12/05 19:41:55]

style: Can you write this in the opposite direction:

if (status.IsError())
  return status;

That is the style used through-out the rest of the code, and I find it is easier
to follow the code flow when you use early-returns (fire and forget so to
speak!)

[nharper, 2014/12/05 23:15:47]

Done.

+    const blink::WebCryptoKey* key = nullptr;

[eroman, 2014/12/05 19:41:55]

Does this compile on all platforms? Not sure that nullptr can be used in
Chromium yet, but I may be wrong (using NULL for the rest of webcrypto code).

[nharper, 2014/12/05 23:15:47]

Codesearch shows nullptr used in thousands of places (some are comments) in
chromium, including use in the quic code. If the style in the webcrypto code is
to use NULL instead, I'm fine with that.

+    if (result->type() == GenerateKeyResult::TYPE_SECRET_KEY) {
+      key = &result->secret_key();
+    }
+    if (result->type() == GenerateKeyResult::TYPE_PUBLIC_PRIVATE_KEY_PAIR) {
+      key = &result->private_key();
+    }
+    if (key != nullptr) {
+      // This should only fail if an algorithm is implemented incorrectly and
+      // does not do its own check of the usages.
+      DCHECK(key->usages() != 0)

[eroman, 2014/12/05 20:52:45]

also: DCHECK_NE(0, key->usages());

[nharper, 2014/12/05 23:15:47]

Since I need to do other things when the DCHECK condition is met, I ended up
changing it to
if (key->usages() == 0) {
  DCHECK(false)...
  do other things
}

+          << "Key usages for generateKey() must not be empty";

[eroman, 2014/12/05 19:41:55]

Great. Can you also return an ErrorUnexpected here? (DCHECK only triggers in
developpers build, so the return would catch badness from propagating in the
release builds).

[nharper, 2014/12/05 23:15:47]

Done.

+    }
+  }
+  return status;
 }
 
 Status ImportKey(blink::WebCryptoKeyFormat format,