Issue 8430027: Added histogram on successful check. Safe verification

Issue 8430027: Added histogram on successful check. Safe verification (Closed)

Created:
9 years, 1 month ago by whywhat

Modified:
9 years, 1 month ago

Reviewers:
Ilya Sherman, Ivan Korotkov

CC:
chromium-reviews, Paweł Hajdan Jr., dhollowa

Base URL:
svn://svn.chromium.org/chrome/trunk/src

Visibility:
Public.

Description

Added histogram on successful check. Safe verification R=ivankr@chromium.org BUG=102480 TEST=Verify that new stats appear at chrome://histograms Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=108111

Patch Set 2 : Fixed signing, added counters #

Total comments: 2

Created: 9 years, 1 month ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+39 lines, -4 lines)			Patch
M	chrome/browser/protector/histograms.h	View	1	1 chunk	+5 lines, -0 lines	0 comments	Download
M	chrome/browser/protector/histograms.cc	View	1	1 chunk	+5 lines, -0 lines	0 comments	Download
M	chrome/browser/protector/protector.h	View		1 chunk	+3 lines, -0 lines	0 comments	Download
M	chrome/browser/protector/protector.cc	View	1	1 chunk	+17 lines, -2 lines	2 comments	Download
M	chrome/browser/webdata/keyword_table.cc	View	1	3 chunks	+9 lines, -1 line	0 comments	Download
M	chrome/browser/webdata/keyword_table_unittest.cc	View		1 chunk	+0 lines, -1 line	0 comments	Download

Messages

Total messages: 5 (0 generated)

Expand Messages | Collapse Messages

Ivan Korotkov

http://codereview.chromium.org/8430027/diff/1/chrome/browser/protector/histograms.h File chrome/browser/protector/histograms.h (right): http://codereview.chromium.org/8430027/diff/1/chrome/browser/protector/histograms.h#newcode19 chrome/browser/protector/histograms.h:19: kProtectorErrorNoError, Maybe call it kProtectorNoError? Because ErrorNoError sounds really ...

9 years, 1 month ago (2011-11-01 14:18:39 UTC) #2

Ilya Sherman

http://codereview.chromium.org/8430027/diff/5001/chrome/browser/protector/protector.cc File chrome/browser/protector/protector.cc (right): http://codereview.chromium.org/8430027/diff/5001/chrome/browser/protector/protector.cc#newcode73 chrome/browser/protector/protector.cc:73: LOG(WARNING) << "Failed to initialize HMAC algorithm for signing"; ...

9 years, 1 month ago (2011-11-01 18:19:11 UTC) #4

whywhat

9 years, 1 month ago (2011-11-02 08:14:28 UTC) #5

http://codereview.chromium.org/8430027/diff/5001/chrome/browser/protector/pro...
File chrome/browser/protector/protector.cc (right):

http://codereview.chromium.org/8430027/diff/5001/chrome/browser/protector/pro...
chrome/browser/protector/protector.cc:73: LOG(WARNING) << "Failed to initialize
HMAC algorithm for signing";
On 2011/11/01 18:19:11, Ilya Sherman wrote:
> Are you sure you want LOG(WARNING) here rather than NOTREACHED()?  Are you
> expecting this to fail in builds that we ship to users; and if so, are you
> expecting the logged lines to be helpful in resolving the issue?

HMAC implementation is platform dependent so may certainly fail on user's
machines. Logging would help to identify this cases when figuring out why
Protector bubble shows up without obvious reason.

Expand Messages | Collapse Messages