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

chrome/browser/safe_browsing/client_side_detection_host_unittest.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/file_path.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/task.h"
 #include "chrome/browser/safe_browsing/browser_feature_extractor.h"
 #include "chrome/browser/safe_browsing/client_side_detection_host.h"
@@ skipping 39 matching lines @@
 // (ClientPhishingRequest) which is passed to SendClientReportPhishingRequest
 // has the expected fields set.  Note: we can't simply compare the protocol
 // buffer strings because the BrowserFeatureExtractor might add features to the
 // verdict object before calling SendClientReportPhishingRequest.
 MATCHER_P(PartiallyEqualVerdict, other, "") {
   return (other.url() == arg.url() &&
           other.client_score() == arg.client_score() &&
           other.is_phishing() == arg.is_phishing());
 }
 
+// Test that the callback is NULL when the verdict is not phishing.
+MATCHER(CallbackIsNull, "") {
+  return arg.is_null();
+}
+
 ACTION(QuitUIMessageLoop) {
   EXPECT_TRUE(BrowserThread::CurrentlyOn(BrowserThread::UI));
   MessageLoopForUI::current()->Quit();
 }
 
 // It's kind of insane that InvokeArgument doesn't work with callbacks, but it
 // doesn't seem like it.
 ACTION_TEMPLATE(InvokeCallbackArgument,
                 HAS_1_TEMPLATE_PARAMS(int, k),
                 AND_2_VALUE_PARAMS(p0, p1)) {
-  ::std::tr1::get<k>(args)->Run(p0, p1);
-  // This is an old callback, so it needs to be deleted explicitly.
-  delete ::std::tr1::get<k>(args);
+  ::std::tr1::get<k>(args).Run(p0, p1);
 }
 
 class MockClientSideDetectionService : public ClientSideDetectionService {
  public:
   MockClientSideDetectionService() : ClientSideDetectionService(NULL) {}
   virtual ~MockClientSideDetectionService() {};
 
   MOCK_METHOD2(SendClientReportPhishingRequest,
                void(ClientPhishingRequest*,
-                    ClientReportPhishingRequestCallback*));
+                    const ClientReportPhishingRequestCallback&));
   MOCK_CONST_METHOD1(IsPrivateIPAddress, bool(const std::string&));
   MOCK_METHOD2(GetValidCachedResult, bool(const GURL&, bool*));
   MOCK_METHOD1(IsInCache, bool(const GURL&));
   MOCK_METHOD0(OverReportLimit, bool());
 
  private:
   DISALLOW_COPY_AND_ASSIGN(MockClientSideDetectionService);
 };
 
 class MockSafeBrowsingService : public SafeBrowsingService {
@@ skipping 27 matching lines @@
 };
 
 class MockBrowserFeatureExtractor : public BrowserFeatureExtractor {
  public:
   explicit MockBrowserFeatureExtractor(
       TabContents* tab,
       ClientSideDetectionService* service)
       : BrowserFeatureExtractor(tab, service) {}
   virtual ~MockBrowserFeatureExtractor() {}
 
-  MOCK_METHOD3(ExtractFeatures, void(const BrowseInfo* info,
-                                     ClientPhishingRequest*,
-                                     BrowserFeatureExtractor::DoneCallback*));
+  MOCK_METHOD3(ExtractFeatures,
+               void(const BrowseInfo* info,
+                    ClientPhishingRequest*,
+                    const BrowserFeatureExtractor::DoneCallback&));
 };
 
 // Helper function which quits the UI message loop from the IO message loop.
 void QuitUIMessageLoopFromIO() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   BrowserThread::PostTask(BrowserThread::UI,
                           FROM_HERE,
                           new MessageLoop::QuitTask());
 }
 }  // namespace
@@ skipping 45 matching lines @@
   void OnPhishingDetectionDone(const std::string& verdict_str) {
     csd_host_->OnPhishingDetectionDone(verdict_str);
   }
 
   void FlushIOMessageLoop() {
     // If there was a message posted on the IO thread to display the
     // interstitial page we know that it would have been posted before
     // we put the quit message there.
     BrowserThread::PostTask(BrowserThread::IO,
                             FROM_HERE,
-                            NewRunnableFunction(&QuitUIMessageLoopFromIO));
+                            base::Bind(&QuitUIMessageLoopFromIO));
     MessageLoop::current()->Run();
   }
 
   void ExpectPreClassificationChecks(const GURL& url,
                                      const bool* is_private,
                                      const bool* is_incognito,
                                      const bool* match_csd_whitelist,
                                      const bool* get_valid_cached_result,
                                      const bool* is_in_cache,
                                      const bool* over_report_limit) {
@@ skipping 99 matching lines @@
 }
 
 TEST_F(ClientSideDetectionHostTest, OnPhishingDetectionDoneNotPhishing) {
   // Case 1: client thinks the page is phishing.  The server does not agree.
   // No interstitial is shown.
   MockBrowserFeatureExtractor* mock_extractor = new MockBrowserFeatureExtractor(
       contents(),
       csd_service_.get());
   SetFeatureExtractor(mock_extractor);  // The host class takes ownership.
 
-  ClientSideDetectionService::ClientReportPhishingRequestCallback* cb;
+  ClientSideDetectionService::ClientReportPhishingRequestCallback cb;
   ClientPhishingRequest verdict;
   verdict.set_url("http://phishingurl.com/");
   verdict.set_client_score(1.0f);
   verdict.set_is_phishing(true);
 
   EXPECT_CALL(*mock_extractor, ExtractFeatures(_, _, _))
       .WillOnce(DoAll(DeleteArg<1>(),
                       InvokeCallbackArgument<2>(true, &verdict)));
   EXPECT_CALL(*csd_service_,
               SendClientReportPhishingRequest(
                   Pointee(PartiallyEqualVerdict(verdict)), _))
       .WillOnce(SaveArg<1>(&cb));
   OnPhishingDetectionDone(verdict.SerializeAsString());
   EXPECT_TRUE(Mock::VerifyAndClear(csd_service_.get()));
-  ASSERT_TRUE(cb);
+  ASSERT_FALSE(cb.is_null());
 
   // Make sure DoDisplayBlockingPage is not going to be called.
   EXPECT_CALL(*sb_service_, DoDisplayBlockingPage(_)).Times(0);
-  cb->Run(GURL(verdict.url()), false);
-  delete cb;
+  cb.Run(GURL(verdict.url()), false);
   MessageLoop::current()->RunAllPending();
   EXPECT_TRUE(Mock::VerifyAndClear(sb_service_.get()));
 }
 
 TEST_F(ClientSideDetectionHostTest, OnPhishingDetectionDoneDisabled) {
   // Case 2: client thinks the page is phishing and so does the server but
   // showing the interstitial is disabled => no interstitial is shown.
   MockBrowserFeatureExtractor* mock_extractor = new MockBrowserFeatureExtractor(
       contents(),
       csd_service_.get());
   SetFeatureExtractor(mock_extractor);  // The host class takes ownership.
 
-  ClientSideDetectionService::ClientReportPhishingRequestCallback* cb;
+  ClientSideDetectionService::ClientReportPhishingRequestCallback cb;
   ClientPhishingRequest verdict;
   verdict.set_url("http://phishingurl.com/");
   verdict.set_client_score(1.0f);
   verdict.set_is_phishing(true);
 
   EXPECT_CALL(*mock_extractor, ExtractFeatures(_, _, _))
       .WillOnce(DoAll(DeleteArg<1>(),
                       InvokeCallbackArgument<2>(true, &verdict)));
   EXPECT_CALL(*csd_service_,
               SendClientReportPhishingRequest(
                   Pointee(PartiallyEqualVerdict(verdict)), _))
       .WillOnce(SaveArg<1>(&cb));
   OnPhishingDetectionDone(verdict.SerializeAsString());
   EXPECT_TRUE(Mock::VerifyAndClear(csd_service_.get()));
-  ASSERT_TRUE(cb);
+  ASSERT_FALSE(cb.is_null());
 
   // Make sure DoDisplayBlockingPage is not going to be called.
   EXPECT_CALL(*sb_service_, DoDisplayBlockingPage(_)).Times(0);
-  cb->Run(GURL(verdict.url()), false);
-  delete cb;
+  cb.Run(GURL(verdict.url()), false);
   MessageLoop::current()->RunAllPending();
   EXPECT_TRUE(Mock::VerifyAndClear(sb_service_.get()));
 }
 
 TEST_F(ClientSideDetectionHostTest, OnPhishingDetectionDoneShowInterstitial) {
   // Case 3: client thinks the page is phishing and so does the server.
   // We show an interstitial.
   MockBrowserFeatureExtractor* mock_extractor = new MockBrowserFeatureExtractor(
       contents(),
       csd_service_.get());
   SetFeatureExtractor(mock_extractor);  // The host class takes ownership.
 
-  ClientSideDetectionService::ClientReportPhishingRequestCallback* cb;
+  ClientSideDetectionService::ClientReportPhishingRequestCallback cb;
   GURL phishing_url("http://phishingurl.com/");
   ClientPhishingRequest verdict;
   verdict.set_url(phishing_url.spec());
   verdict.set_client_score(1.0f);
   verdict.set_is_phishing(true);
 
   EXPECT_CALL(*mock_extractor, ExtractFeatures(_, _, _))
       .WillOnce(DoAll(DeleteArg<1>(),
                       InvokeCallbackArgument<2>(true, &verdict)));
   EXPECT_CALL(*csd_service_,
               SendClientReportPhishingRequest(
                   Pointee(PartiallyEqualVerdict(verdict)), _))
       .WillOnce(SaveArg<1>(&cb));
   OnPhishingDetectionDone(verdict.SerializeAsString());
   EXPECT_TRUE(Mock::VerifyAndClear(csd_service_.get()));
-  ASSERT_TRUE(cb);
+  ASSERT_FALSE(cb.is_null());
 
   SafeBrowsingService::UnsafeResource resource;
   EXPECT_CALL(*sb_service_, DoDisplayBlockingPage(_))
       .WillOnce(SaveArg<0>(&resource));
-  cb->Run(phishing_url, true);
-  delete cb;
+  cb.Run(phishing_url, true);
 
   MessageLoop::current()->RunAllPending();
   EXPECT_TRUE(Mock::VerifyAndClear(sb_service_.get()));
   EXPECT_EQ(phishing_url, resource.url);
   EXPECT_EQ(phishing_url, resource.original_url);
   EXPECT_FALSE(resource.is_subresource);
   EXPECT_EQ(SafeBrowsingService::CLIENT_SIDE_PHISHING_URL,
             resource.threat_type);
   EXPECT_EQ(contents()->GetRenderProcessHost()->id(),
             resource.render_process_host_id);
   EXPECT_EQ(contents()->render_view_host()->routing_id(),
             resource.render_view_id);
 
   // Make sure the client object will be deleted.
   BrowserThread::PostTask(
       BrowserThread::IO,
       FROM_HERE,
-      NewRunnableMethod(
-          sb_service_.get(),
-          &MockSafeBrowsingService::InvokeOnBlockingPageComplete,
-          resource.client));
+      base::Bind(&MockSafeBrowsingService::InvokeOnBlockingPageComplete,
+                 sb_service_.get(), resource.client));
   // Since the CsdClient object will be deleted on the UI thread I need
   // to run the UI message loop.  Post a task to stop the UI message loop
   // after the client object destructor is called.
   FlushIOMessageLoop();
 }
 
 TEST_F(ClientSideDetectionHostTest, OnPhishingDetectionDoneMultiplePings) {
   // Case 4 & 5: client thinks a page is phishing then navigates to
   // another page which is also considered phishing by the client
   // before the server responds with a verdict.  After a while the
   // server responds for both requests with a phishing verdict.  Only
   // a single interstitial is shown for the second URL.
   MockBrowserFeatureExtractor* mock_extractor = new MockBrowserFeatureExtractor(
       contents(),
       csd_service_.get());
   SetFeatureExtractor(mock_extractor);  // The host class takes ownership.
 
-  ClientSideDetectionService::ClientReportPhishingRequestCallback* cb;
+  ClientSideDetectionService::ClientReportPhishingRequestCallback cb;
   GURL phishing_url("http://phishingurl.com/");
   ClientPhishingRequest verdict;
   verdict.set_url(phishing_url.spec());
   verdict.set_client_score(1.0f);
   verdict.set_is_phishing(true);
 
   EXPECT_CALL(*mock_extractor, ExtractFeatures(_, _, _))
       .WillOnce(DoAll(DeleteArg<1>(),
                       InvokeCallbackArgument<2>(true, &verdict)));
   EXPECT_CALL(*csd_service_,
               SendClientReportPhishingRequest(
                   Pointee(PartiallyEqualVerdict(verdict)), _))
       .WillOnce(SaveArg<1>(&cb));
   OnPhishingDetectionDone(verdict.SerializeAsString());
   EXPECT_TRUE(Mock::VerifyAndClear(csd_service_.get()));
-  ASSERT_TRUE(cb);
+  ASSERT_FALSE(cb.is_null());
 
   // Set this back to a normal browser feature extractor since we're using
   // NavigateAndCommit() and it's easier to use the real thing than setting up
   // mock expectations.
   SetFeatureExtractor(new BrowserFeatureExtractor(contents(),
                                                   csd_service_.get()));
   GURL other_phishing_url("http://other_phishing_url.com/bla");
   ExpectPreClassificationChecks(other_phishing_url, &kFalse, &kFalse, &kFalse,
                                 &kFalse, &kFalse, &kFalse);
   // We navigate away.  The callback cb should be revoked.
   NavigateAndCommit(other_phishing_url);
   // Wait for the pre-classification checks to finish for other_phishing_url.
   WaitAndCheckPreClassificationChecks();
 
-  ClientSideDetectionService::ClientReportPhishingRequestCallback* cb_other;
+  ClientSideDetectionService::ClientReportPhishingRequestCallback cb_other;
   verdict.set_url(other_phishing_url.spec());
   verdict.set_client_score(0.8f);
   EXPECT_CALL(*csd_service_,
               SendClientReportPhishingRequest(
                   Pointee(PartiallyEqualVerdict(verdict)), _))
       .WillOnce(DoAll(DeleteArg<0>(),
                       SaveArg<1>(&cb_other),
                       QuitUIMessageLoop()));
   std::vector<GURL> redirect_chain;
   redirect_chain.push_back(other_phishing_url);
   SetRedirectChain(redirect_chain);
   OnPhishingDetectionDone(verdict.SerializeAsString());
   MessageLoop::current()->Run();
   EXPECT_TRUE(Mock::VerifyAndClear(csd_service_.get()));
-  ASSERT_TRUE(cb_other);
+  ASSERT_FALSE(cb_other.is_null());
 
   // We expect that the interstitial is shown for the second phishing URL and
   // not for the first phishing URL.
   SafeBrowsingService::UnsafeResource resource;
   EXPECT_CALL(*sb_service_, DoDisplayBlockingPage(_))
       .WillOnce(SaveArg<0>(&resource));
 
-  cb->Run(phishing_url, true);  // Should have no effect.
-  delete cb;
-  cb_other->Run(other_phishing_url, true);  // Should show interstitial.
-  delete cb_other;
+  cb.Run(phishing_url, true);  // Should have no effect.
+  cb_other.Run(other_phishing_url, true);  // Should show interstitial.
 
   MessageLoop::current()->RunAllPending();
   EXPECT_TRUE(Mock::VerifyAndClear(sb_service_.get()));
   EXPECT_EQ(other_phishing_url, resource.url);
   EXPECT_EQ(other_phishing_url, resource.original_url);
   EXPECT_FALSE(resource.is_subresource);
   EXPECT_EQ(SafeBrowsingService::CLIENT_SIDE_PHISHING_URL,
             resource.threat_type);
   EXPECT_EQ(contents()->GetRenderProcessHost()->id(),
             resource.render_process_host_id);
   EXPECT_EQ(contents()->render_view_host()->routing_id(),
             resource.render_view_id);
 
   // Make sure the client object will be deleted.
   BrowserThread::PostTask(
       BrowserThread::IO,
       FROM_HERE,
-      NewRunnableMethod(
-          sb_service_.get(),
-          &MockSafeBrowsingService::InvokeOnBlockingPageComplete,
-          resource.client));
+      base::Bind(&MockSafeBrowsingService::InvokeOnBlockingPageComplete,
+                 sb_service_.get(), resource.client));
   // Since the CsdClient object will be deleted on the UI thread I need
   // to run the UI message loop.  Post a task to stop the UI message loop
   // after the client object destructor is called.
   FlushIOMessageLoop();
 }
 
 TEST_F(ClientSideDetectionHostTest, OnPhishingDetectionDoneVerdictNotPhishing) {
   // Case 6: renderer sends a verdict string that isn't phishing.
   MockBrowserFeatureExtractor* mock_extractor = new MockBrowserFeatureExtractor(
       contents(),
@@ skipping 22 matching lines @@
 
   // First we have to navigate to the URL to set the unique page ID.
   ExpectPreClassificationChecks(url, &kFalse, &kFalse, &kFalse, &kFalse,
                                 &kFalse, &kFalse);
   NavigateAndCommit(url);
   WaitAndCheckPreClassificationChecks();
   SetUnsafeResourceToCurrent();
 
   EXPECT_CALL(*csd_service_,
               SendClientReportPhishingRequest(
-                  Pointee(PartiallyEqualVerdict(verdict)), IsNull()))
+                  Pointee(PartiallyEqualVerdict(verdict)), CallbackIsNull()))
       .WillOnce(DoAll(DeleteArg<0>(), QuitUIMessageLoop()));
   std::vector<GURL> redirect_chain;
   redirect_chain.push_back(url);
   SetRedirectChain(redirect_chain);
   OnPhishingDetectionDone(verdict.SerializeAsString());
   MessageLoop::current()->Run();
   EXPECT_TRUE(Mock::VerifyAndClear(csd_service_.get()));
 }
 
 TEST_F(ClientSideDetectionHostTest, NavigationCancelsShouldClassifyUrl) {
@@ skipping 164 matching lines @@
   EXPECT_TRUE(Mock::VerifyAndClear(sb_service_.get()));
   EXPECT_EQ(url, resource.url);
   EXPECT_EQ(url, resource.original_url);
   delete resource.client;
   msg = process()->sink().GetFirstMessageMatching(
       SafeBrowsingMsg_StartPhishingDetection::ID);
   ASSERT_FALSE(msg);
 }
 
 }  // namespace safe_browsing