[WebCrypto] Cleanup in LoadTestKeys() in ecdh unit tests

content/child/webcrypto/test/ecdh_unittest.cc

 // Copyright 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.
 
 #include "base/stl_util.h"
 #include "content/child/webcrypto/algorithm_dispatch.h"
 #include "content/child/webcrypto/crypto_data.h"
 #include "content/child/webcrypto/jwk.h"
 #include "content/child/webcrypto/status.h"
 #include "content/child/webcrypto/test/test_helpers.h"
@@ skipping 110 matching lines @@
 
     EXPECT_EQ(CryptoData(expected_bytes), CryptoData(derived_bytes));
   }
 }
 
 // Loads up a test ECDH public and private key for P-521. The keys
 // come from different key pairs, and can be used for key derivation of up to
 // 528 bits.
 void LoadTestKeys(blink::WebCryptoKey* public_key,
                   blink::WebCryptoKey* private_key) {
-  // Assume that the 7th key in the test data is for P-521.
   scoped_ptr<base::ListValue> tests;
   ASSERT_TRUE(ReadJsonTestFileToList("ecdh.json", &tests));
 
   const base::DictionaryValue* test;

[eroman, 2014/12/22 17:56:07]

This does not handle the case where the property is not found in a predictable
way. Instead of proceeding with "test" as the last value, it should assert
explaining that the required property not found.

[Habib Virji, 2014/12/23 10:59:05]

I have added now, assert failure if it fails to find valid_p521_key. Since
LoadTestKeys is a helper function, returning early did not help as it was
failing in the calling test. So added a assert failure as a return type and
doing the assert check where LoadTestKeys is called in individual test. Since it
returns assert had to change assert check to expect check. 

-  ASSERT_TRUE(tests->GetDictionary(6, &test));
+  for (size_t test_index = 0; test_index < tests->GetSize(); ++test_index) {
+    SCOPED_TRACE(test_index);
+    ASSERT_TRUE(tests->GetDictionary(test_index, &test));
+    int result = 0;
+    test->GetInteger("load_p521_test_keys", &result);
+    if (result)
+      break;
+  }
 
   ImportKeysFromTest(test, public_key, private_key);
 
   ASSERT_EQ(blink::WebCryptoNamedCurveP521,
             public_key->algorithm().ecParams()->namedCurve());
 }
 
 // Try deriving an AES key of length 129 bits.
 TEST(WebCryptoEcdhTest, DeriveKeyBadAesLength) {
   if (!SupportsEcdh())
@@ skipping 201 matching lines @@
                       CryptoData(MakeJsonVector(*private_key_json)),
                       CreateEcdhImportAlgorithm(curve), true,
                       0, &key));
 }
 
 }  // namespace
 
 }  // namespace webcrypto
 
 }  // namespace content