Run pre-classification checks in the browser before starting client-side phishing detection.

chrome/browser/safe_browsing/client_side_detection_service_unittest.cc

 // Copyright (c) 2010 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 <map>
 #include <queue>
 #include <string>
 
 #include "base/callback.h"
 #include "base/file_path.h"
 #include "base/file_util.h"
 #include "base/file_util_proxy.h"
 #include "base/logging.h"
 #include "base/message_loop.h"
 #include "base/platform_file.h"
 #include "base/scoped_ptr.h"
 #include "base/scoped_temp_dir.h"
 #include "base/task.h"
 #include "base/time.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "chrome/browser/browser_thread.h"
+#include "chrome/browser/renderer_host/test/test_render_view_host.h"
 #include "chrome/browser/safe_browsing/client_side_detection_service.h"
 #include "chrome/browser/safe_browsing/csd.pb.h"
+#include "chrome/common/render_messages.h"
 #include "chrome/common/net/test_url_fetcher_factory.h"
 #include "chrome/common/net/url_fetcher.h"
 #include "googleurl/src/gurl.h"
+#include "ipc/ipc_channel.h"
+#include "ipc/ipc_test_sink.h"
 #include "net/url_request/url_request_status.h"
 
 namespace safe_browsing {
 
 class ClientSideDetectionServiceTest : public testing::Test {
  protected:
   virtual void SetUp() {
     file_thread_.reset(new BrowserThread(BrowserThread::FILE, &msg_loop_));
 
     factory_.reset(new FakeURLFetcherFactory());
@@ skipping 180 matching lines @@
   base::Time now = base::Time::Now();
   base::TimeDelta twenty_five_hours = base::TimeDelta::FromHours(25);
   report_times.push(now - twenty_five_hours);
   report_times.push(now - twenty_five_hours);
   report_times.push(now);
   report_times.push(now);
 
   EXPECT_EQ(2, GetNumReportsPerDay());
 }
 
+// We use a separate test fixture for testing the ClientSideDetectionService's
+// handling of load notifications from TabContents.  This uses
+// RenderViewHostTestHarness to set up a fake TabContents and related objects.
+class ClientSideDetectionServiceHooksTest : public RenderViewHostTestHarness,
+                                            public IPC::Channel::Listener {
+ public:
+  // IPC::Channel::Listener
+  virtual bool OnMessageReceived(const IPC::Message& msg) {
+    if (msg.type() == ViewMsg_StartPhishingDetection::ID) {
+      received_msg_ = msg;
+      MessageLoop::current()->Quit();
+      return true;
+    }
+    return false;
+  }
+
+ protected:
+  virtual void SetUp() {
+    RenderViewHostTestHarness::SetUp();
+    file_thread_.reset(new BrowserThread(BrowserThread::FILE, &message_loop_));
+    ui_thread_.reset(new BrowserThread(BrowserThread::UI, &message_loop_));
+
+    // We're not exercising model fetching here, so just set up a canned
+    // success response.
+    factory_.reset(new FakeURLFetcherFactory());
+    factory_->SetFakeResponse(ClientSideDetectionService::kClientModelUrl,
+                              "dummy model data", true);
+    URLFetcher::set_factory(factory_.get());
+
+    process()->sink().AddFilter(this);
+  }
+
+  virtual void TearDown() {
+    process()->sink().RemoveFilter(this);
+    URLFetcher::set_factory(NULL);
+    file_thread_.reset();
+    ui_thread_.reset();
+    RenderViewHostTestHarness::TearDown();
+  }
+
+  scoped_ptr<FakeURLFetcherFactory> factory_;
+  scoped_ptr<BrowserThread> ui_thread_;
+  scoped_ptr<BrowserThread> file_thread_;
+  IPC::Message received_msg_;
+};
+
+TEST_F(ClientSideDetectionServiceHooksTest, ShouldClassifyUrl) {
+  ScopedTempDir tmp_dir;
+  ASSERT_TRUE(tmp_dir.CreateUniqueTempDir());
+  FilePath model_path = tmp_dir.path().AppendASCII("model");
+
+  scoped_ptr<ClientSideDetectionService> csd_service(
+      ClientSideDetectionService::Create(model_path, NULL));

[noelutz, 2011/02/10 01:36:07]

I created a separate empty constructor for this in my CL.  Feel free to do the
same and I will merge.

[Brian Ryner, 2011/02/10 21:15:20]

I'm not so sure I'm a fan of the second constructor.  It duplicates all of the
initialization, which makes it more likely that we'll forget at some point to
add a new initializer to both constructers.  Do you actually need to have the
empty constructor for your test, or could you just have your
MockClientSideDetectionService initialize the base class with an empty FilePath
and a NULL URLRequestContextGetter?

+
+  // Navigate the tab to a page.  We should see a StartPhishingDetection IPC.
+  NavigateAndCommit(GURL("http://host.com/"));
+  // Wait for the StartPhishingDetection message to arrive.
+  MessageLoop::current()->Run();
+
+  Tuple1<GURL> url;
+  ViewMsg_StartPhishingDetection::Read(&received_msg_, &url);
+  EXPECT_EQ(GURL("http://host.com/"), url.a);
+  EXPECT_EQ(rvh()->routing_id(), received_msg_.routing_id());

[noelutz, 2011/02/10 01:36:07]

Maybe do a reload and check that you don't send an IPC in this case?

[Brian Ryner, 2011/02/10 21:15:20]

Since I'm no longer doing anything different on back/forward navigation, there's
probably no need to test that case explicitly.

I wouldn't mind doing a test for the in-page navigation case, but it would be a
little difficult -- since I wouldn't expect an IPC, I wouldn't be able to quit
the message loop the same way as I'm doing above.  I could possibly call
RunAllPending() (more than once if needed)... thoughts?

+}
+
 }  // namespace safe_browsing