Convert to base::Callback in safe_browsing client-side-detection code.

chrome/browser/safe_browsing/client_side_detection_service.cc

Index: chrome/browser/safe_browsing/client_side_detection_service.cc
diff --git a/chrome/browser/safe_browsing/client_side_detection_service.cc b/chrome/browser/safe_browsing/client_side_detection_service.cc
index f92cca51226160298611c399bb4532c196677143..ddb0d24adebee11172c03c2c45ff5dd154d9f4a7 100644
--- a/chrome/browser/safe_browsing/client_side_detection_service.cc
+++ b/chrome/browser/safe_browsing/client_side_detection_service.cc
@@ -60,7 +60,7 @@ const char ClientSideDetectionService::kClientModelUrl[] =
     "https://ssl.gstatic.com/safebrowsing/csd/client_model_v4.pb";
 
 struct ClientSideDetectionService::ClientReportInfo {
-  scoped_ptr<ClientReportPhishingRequestCallback> callback;
+  ClientReportPhishingRequestCallback callback;
   GURL phishing_url;
 };
 
@@ -118,8 +118,7 @@ void ClientSideDetectionService::SetEnabledAndRefreshState(bool enabled) {
              client_phishing_reports_.begin();
          it != client_phishing_reports_.end(); ++it) {
       ClientReportInfo* info = it->second;
-      if (info->callback.get())
-        info->callback->Run(info->phishing_url, false);
+      info->callback.Run(info->phishing_url, false);
     }
     STLDeleteContainerPairPointers(client_phishing_reports_.begin(),
                                    client_phishing_reports_.end());
@@ -130,7 +129,7 @@ void ClientSideDetectionService::SetEnabledAndRefreshState(bool enabled) {
 
 void ClientSideDetectionService::SendClientReportPhishingRequest(
     ClientPhishingRequest* verdict,
-    ClientReportPhishingRequestCallback* callback) {
+    const ClientReportPhishingRequestCallback& callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   MessageLoop::current()->PostTask(
       FROM_HERE,
@@ -287,14 +286,12 @@ void ClientSideDetectionService::EndFetchModel(ClientModelStatus status) {
 
 void ClientSideDetectionService::StartClientReportPhishingRequest(
     ClientPhishingRequest* verdict,
-    ClientReportPhishingRequestCallback* callback) {
+    const ClientReportPhishingRequestCallback& callback) {

[noelutz, 2011/11/21 21:46:05]

note: this callback can be null.

[Scott Hess - ex-Googler, 2011/11/21 22:15:20]

That's odd.  I'm sure I had reviewed the code and verified that a callback used
like this:

  base::Callback<void(void)> cb;
  if (SomethingIsTrue()) {
    cb = base::Bind(&myfun);
  }
  cb.Run();

would be safe to call Run() in either case.  But it doesn't look that way to me
when I look at the code, now.  I might have to sprinkle some is_null() calls in
here.

[noelutz, 2011/11/21 22:24:00]

Oh, if this code is safe then we're fine.  In other words: is it ok to call
Run() if the callback is_null()?

   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   scoped_ptr<ClientPhishingRequest> request(verdict);
-  scoped_ptr<ClientReportPhishingRequestCallback> cb(callback);
 
   if (!enabled_) {
-    if (cb.get())
-      cb->Run(GURL(request->url()), false);
+    callback.Run(GURL(request->url()), false);
     return;
   }
 
@@ -302,9 +299,7 @@ void ClientSideDetectionService::StartClientReportPhishingRequest(
   if (!request->SerializeToString(&request_data)) {
     UMA_HISTOGRAM_COUNTS("SBClientPhishing.RequestNotSerialized", 1);
     VLOG(1) << "Unable to serialize the CSD request. Proto file changed?";
-    if (cb.get()) {
-      cb->Run(GURL(request->url()), false);
-    }
+    callback.Run(GURL(request->url()), false);
     return;
   }
 
@@ -314,7 +309,7 @@ void ClientSideDetectionService::StartClientReportPhishingRequest(
 
   // Remember which callback and URL correspond to the current fetcher object.
   ClientReportInfo* info = new ClientReportInfo;
-  info->callback.swap(cb);  // takes ownership of the callback.
+  info->callback = callback;
   info->phishing_url = GURL(request->url());
   client_phishing_reports_[fetcher] = info;
 
@@ -390,9 +385,7 @@ void ClientSideDetectionService::HandlePhishingVerdict(
                 << info->phishing_url << " status: " << status.status() << " "
                 << "response_code:" << response_code;
   }
-  if (info->callback.get()) {
-    info->callback->Run(info->phishing_url, is_phishing);
-  }
+  info->callback.Run(info->phishing_url, is_phishing);
   client_phishing_reports_.erase(source);
   delete source;
 }