Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
57373004:
[webcrypto] Fix compiler errors in WebCryptoImplTest (Closed)
|
Created:
7 years, 1 month ago by haltonhuo Modified:
7 years, 1 month ago CC:
chromium-reviews, joi+watch-content_chromium.org, darin-cc_chromium.org, jam Base URL:
https://chromium.googlesource.com/chromium/src.git@master Visibility:
Public. |
Description[webcrypto] Fix compiler errors in WebCryptoImplTest
Also fix some changes for coding style errors reported by cpplint.py
BUG=314517
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=233233
Patch Set 1 #Patch Set 2 : Update commit log #
Total comments: 2
Patch Set 3 : Update based on padolph's comment #
Total comments: 2
Messages
Total messages: 15 (0 generated)
Thanks for the patch https://codereview.chromium.org/57373004/diff/30001/content/renderer/webcrypt... File content/renderer/webcrypto/webcrypto_impl_unittest.cc (right): https://codereview.chromium.org/57373004/diff/30001/content/renderer/webcrypt... content/renderer/webcrypto/webcrypto_impl_unittest.cc:709: EXPECT_FALSE(public_key.extractable()); Can you explain why this is necessary? both "extractable" and public_key.extractable() should be of type "bool". Is the issue perhaps due to a failure to implicitly convert between "const"? (i.e. does removing "const" from extractable fix the problem?)
https://codereview.chromium.org/57373004/diff/30001/content/renderer/webcrypt... File content/renderer/webcrypto/webcrypto_impl_unittest.cc (right): https://codereview.chromium.org/57373004/diff/30001/content/renderer/webcrypt... content/renderer/webcrypto/webcrypto_impl_unittest.cc:709: EXPECT_FALSE(public_key.extractable()); On 2013/11/04 17:35:06, eroman wrote: > Can you explain why this is necessary? both "extractable" and > public_key.extractable() should be of type "bool". > > Is the issue perhaps due to a failure to implicitly convert between "const"? > (i.e. does removing "const" from extractable fix the problem?) The reason I used EXPECT_EQ here was so we could change the value of extractable later for whatever reason, but that is not very important. If we want to hard-code EXPECT_FALSE here we might as well get rid of the extractable variable altogether and pass the value inline in the GenerateKeyPairInternal() call.
@padolph, Remove const does fix this issue also, please review the updated patch. > > Is the issue perhaps due to a failure to implicitly convert between "const"? > > (i.e. does removing "const" from extractable fix the problem?) > > The reason I used EXPECT_EQ here was so we could change the value of extractable > later for whatever reason, but that is not very important. If we want to > hard-code EXPECT_FALSE here we might as well get rid of the extractable variable > altogether and pass the value inline in the GenerateKeyPairInternal() call.
lgtm
lgtm https://codereview.chromium.org/57373004/diff/70001/content/renderer/webcrypt... File content/renderer/webcrypto/webcrypto_impl_unittest.cc (right): https://codereview.chromium.org/57373004/diff/70001/content/renderer/webcrypt... content/renderer/webcrypto/webcrypto_impl_unittest.cc:77: unsigned short key_length_bits) { // NOLINT what is the lint error that this line gives? https://codereview.chromium.org/57373004/diff/70001/content/renderer/webcrypt... content/renderer/webcrypto/webcrypto_impl_unittest.cc:721: unsigned exponent_length = sizeof(unsigned long) + 1; // NOLINT what is the lint error that this line gives?
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/halton.huo@intel.com/57373004/70001
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/halton.huo@intel.com/57373004/70001
Sorry for I got bad news for ya. Compile failed with a clobber build on linux_rel. http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&... Your code is likely broken or HEAD is junk. Please ensure your code is not broken then alert the build sheriffs. Look at the try server FAQ for more details.
On 2013/11/05 04:01:49, I haz the power (commit-bot) wrote: > Sorry for I got bad news for ya. > Compile failed with a clobber build on linux_rel. > http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&... > Your code is likely broken or HEAD is junk. Please ensure your > code is not broken then alert the build sheriffs. > Look at the try server FAQ for more details. I do not think the below error is related, instead it is the slave environment issue. Installing libcap-dev will resolve or rerun src/build/install-build-deps.sh > ../../sandbox/linux/services/credentials.cc:8:28: fatal error: sys/capability.h: No such file or directory @eroman and @padolph, I'm not sure how to deal with this kind of situation CL, any idea?
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/halton.huo@intel.com/57373004/70001
On 2013/11/05 01:06:39, eroman wrote: > lgtm > > https://codereview.chromium.org/57373004/diff/70001/content/renderer/webcrypt... > content/renderer/webcrypto/webcrypto_impl_unittest.cc:77: unsigned short > key_length_bits) { // NOLINT > what is the lint error that this line gives? > > https://codereview.chromium.org/57373004/diff/70001/content/renderer/webcrypt... > content/renderer/webcrypto/webcrypto_impl_unittest.cc:721: unsigned > exponent_length = sizeof(unsigned long) + 1; // NOLINT > what is the lint error that this line gives? @eroman, same error "Use int16/int64/etc, rather than the C type short [runtime/int] [4]"
Retried try job too often on win_rel for step(s) browser_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/halton.huo@intel.com/57373004/70001
Message was sent while issue was closed.
Change committed as 233233 |
