Separate memory purger from safe browsing.

Separate memory purger from safe browsing.

The memory purger already sends a notification when it's purging memory;
safe browsing can listen for this notification instead of the memory
purger closing the safe browsing database itself.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=91688

[Scott Hess - ex-Googler, 2011-07-06 17:01:28]

LGTM.

But it might be safest to wait for Peter - I don't know for sure whether there
are any special considerations with the memory purger.

http://codereview.chromium.org/7313003/diff/1/chrome/browser/safe_browsing/sa...
File chrome/browser/safe_browsing/safe_browsing_service.cc (right):

http://codereview.chromium.org/7313003/diff/1/chrome/browser/safe_browsing/sa...
chrome/browser/safe_browsing/safe_browsing_service.cc:1215: DCHECK(type ==
NotificationType::PURGE_MEMORY);
Prefer DCHECK_EQ() for the improved diagnostic output.

[Peter Kasting, 2011-07-06 18:18:37]

LGTM

http://codereview.chromium.org/7313003/diff/1/chrome/browser/safe_browsing/sa...
File chrome/browser/safe_browsing/safe_browsing_service.cc (right):

http://codereview.chromium.org/7313003/diff/1/chrome/browser/safe_browsing/sa...
chrome/browser/safe_browsing/safe_browsing_service.cc:527: registrar_.Add(
Nit: Put as many args on this line as will fit

http://codereview.chromium.org/7313003/diff/1/chrome/browser/safe_browsing/sa...
chrome/browser/safe_browsing/safe_browsing_service.cc:1215: DCHECK(type ==
NotificationType::PURGE_MEMORY);
On 2011/07/06 17:01:28, shess wrote:
> Prefer DCHECK_EQ() for the improved diagnostic output.

I don't think DCHECK_EQ() works for this argument type.

http://codereview.chromium.org/7313003/diff/1/chrome/browser/safe_browsing/sa...
chrome/browser/safe_browsing/safe_browsing_service.cc:1216: CloseDatabase();
Nit: Is this now only called from within the class?  If so, it can become
private.

[Torne, 2011-07-07 09:29:34]

http://codereview.chromium.org/7313003/diff/1/chrome/browser/safe_browsing/sa...
File chrome/browser/safe_browsing/safe_browsing_service.cc (right):

http://codereview.chromium.org/7313003/diff/1/chrome/browser/safe_browsing/sa...
chrome/browser/safe_browsing/safe_browsing_service.cc:527: registrar_.Add(
On 2011/07/06 18:18:37, Peter Kasting wrote:
> Nit: Put as many args on this line as will fit

Done.

http://codereview.chromium.org/7313003/diff/1/chrome/browser/safe_browsing/sa...
chrome/browser/safe_browsing/safe_browsing_service.cc:1215: DCHECK(type ==
NotificationType::PURGE_MEMORY);
On 2011/07/06 18:18:37, Peter Kasting wrote:
> On 2011/07/06 17:01:28, shess wrote:
> > Prefer DCHECK_EQ() for the improved diagnostic output.
> 
> I don't think DCHECK_EQ() works for this argument type.

It doesn't, no; there's no ostream::operator<< defined for it, so the logger
can't print it.

http://codereview.chromium.org/7313003/diff/1/chrome/browser/safe_browsing/sa...
chrome/browser/safe_browsing/safe_browsing_service.cc:1216: CloseDatabase();
On 2011/07/06 18:18:37, Peter Kasting wrote:
> Nit: Is this now only called from within the class?  If so, it can become
> private.

Done.

[commit-bot: I haz the power, 2011-07-07 10:51:49]

Change committed as 91688