Summary: | PCM: Add error logging for CryptoKit operations | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | John Wilander <wilander> | ||||||||
Component: | WebKit Misc. | Assignee: | John Wilander <wilander> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | bfulgham, cdumez, ews-watchlist, japhet, katherine_cheney, webkit-bug-importer | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=227777 | ||||||||||
Attachments: |
|
Description
John Wilander
2021-07-06 18:04:30 PDT
Created attachment 432994 [details]
Patch
Comment on attachment 432994 [details]
Patch
Marking as obsolete pending updates to API tests.
A failing API test was addressed in a separate patch (Bugzilla related to this one). Created attachment 433106 [details]
Patch
Comment on attachment 433106 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=433106&action=review r=me, but consider my suggestion about the test case. > Tools/TestWebKitAPI/Tests/WebCore/PrivateClickMeasurement.cpp:323 > + EXPECT_FALSE(pcm.calculateAndUpdateSourceUnlinkableToken(serverPublicKeyBase64URL)); I think these tests would look better if you asserted EXPECT_FALSE(pcm.calculateAndUpdateSourceUnlinkableToken(serverPublicKeyBase64URL).has_value()); The fact that the return value is an error message (or no-value if successful) isn't obvious from the name, so it looks like we are expecting the calculateAndUpdate to fail. Created attachment 433115 [details]
Patch for landing
(In reply to Brent Fulgham from comment #6) > Comment on attachment 433106 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=433106&action=review > > r=me, but consider my suggestion about the test case. > > > Tools/TestWebKitAPI/Tests/WebCore/PrivateClickMeasurement.cpp:323 > > + EXPECT_FALSE(pcm.calculateAndUpdateSourceUnlinkableToken(serverPublicKeyBase64URL)); > > I think these tests would look better if you asserted > EXPECT_FALSE(pcm. > calculateAndUpdateSourceUnlinkableToken(serverPublicKeyBase64URL). > has_value()); > > The fact that the return value is an error message (or no-value if > successful) isn't obvious from the name, so it looks like we are expecting > the calculateAndUpdate to fail. You're absolutely right. I made the test look like the actually call sites with a local variable errorMessage. Should make it clear. Thanks for the review! Committed r279710 (239502@main): <https://commits.webkit.org/239502@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 433115 [details]. |