Issue 11347005: Change SafeBrowsingProtocolManager::GetFullHash signature.

Issue 11347005: Change SafeBrowsingProtocolManager::GetFullHash signature. (Closed)

Created:
8 years, 1 month ago by cbentzel

Modified:
8 years, 1 month ago

Reviewers:
mattm

CC:
chromium-reviews

Base URL:
http://git.chromium.org/chromium/src.git@in_progress

Visibility:
Public.

More Reviews

Description

Change SafeBrowsingProtocolManager::GetFullHash signature. This makes two changes: - Gets rid of the SafeBrowsingService::SafeBrowsingCheck argument, which was treated (mostly) opaquely by ProtocolManager. - Pass in a callback rather to be invoked rather than have SafeBrowsingProtocolManager directly call a method on SafeBrowsingService. This is one step to decoupling SafeBrowsingProtocolManager from SafeBrowsingService to make it easier to test. BUG=158246 Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=164949

Patch Set 1 #

Patch Set 2 : Use unretained #

Patch Set 3 : Fixed browser_tests #

Patch Set 4 : Header changes #

Patch Set 5 : Another comment fix #

Patch Set 6 : Move InvokeFullHashCallback to anon namespace #

Patch Set 7 : Rebase #

Patch Set 8 : Fix rebase with browser_test #

Total comments: 4

Patch Set 9 : const-ref and comment #

Patch Set 10 : rebase #

Patch Set 11 : Fix browser_tests after rebase #

Created: 8 years, 1 month ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+72 lines, -24 lines)			Patch
M	chrome/browser/safe_browsing/protocol_manager.h	View	1 2 3 4 5 6 7 8 9	4 chunks	+26 lines, -7 lines	0 comments	Download
M	chrome/browser/safe_browsing/protocol_manager.cc	View	1 2 3 4 5 6 7 8 9	5 chunks	+23 lines, -8 lines	0 comments	Download
M	chrome/browser/safe_browsing/safe_browsing_service.cc	View	1 2 3 4 5 6 7 8 9	1 chunk	+8 lines, -1 line	0 comments	Download
M	chrome/browser/safe_browsing/safe_browsing_service_browsertest.cc	View	1 2 3 4 5 6 7 8 9 10	4 chunks	+15 lines, -8 lines	0 comments	Download

Messages

Total messages: 5 (0 generated)

Expand Messages | Collapse Messages

mattm

lgtm http://codereview.chromium.org/11347005/diff/5004/chrome/browser/safe_browsing/protocol_manager.cc File chrome/browser/safe_browsing/protocol_manager.cc (right): http://codereview.chromium.org/11347005/diff/5004/chrome/browser/safe_browsing/protocol_manager.cc#newcode207 chrome/browser/safe_browsing/protocol_manager.cc:207: FullHashDetails details = it->second; Maybe use a reference? ...

8 years, 1 month ago (2012-10-29 23:57:09 UTC) #2

cbentzel

http://codereview.chromium.org/11347005/diff/5004/chrome/browser/safe_browsing/protocol_manager.cc File chrome/browser/safe_browsing/protocol_manager.cc (right): http://codereview.chromium.org/11347005/diff/5004/chrome/browser/safe_browsing/protocol_manager.cc#newcode207 chrome/browser/safe_browsing/protocol_manager.cc:207: FullHashDetails details = it->second; On 2012/10/29 23:57:09, mattm wrote: ...

8 years, 1 month ago (2012-10-30 11:34:03 UTC) #3

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/cbentzel@chromium.org/11347005/11021

8 years, 1 month ago (2012-10-30 14:44:53 UTC) #4

Change committed as 164949

Expand Messages | Collapse Messages