Querying the history service to get the redirect information for urls....

chrome/browser/safe_browsing/malware_details_unittest.cc

Index: chrome/browser/safe_browsing/malware_details_unittest.cc
===================================================================
--- chrome/browser/safe_browsing/malware_details_unittest.cc	(revision 84048)
+++ chrome/browser/safe_browsing/malware_details_unittest.cc	(working copy)
@@ -8,12 +8,15 @@
 #include "base/time.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/safe_browsing/malware_details.h"
+#include "chrome/browser/safe_browsing/malware_details_history.h"
 #include "chrome/browser/safe_browsing/report.pb.h"
 #include "chrome/common/render_messages.h"
 #include "chrome/common/safe_browsing/safebrowsing_messages.h"
 #include "chrome/test/test_url_request_context_getter.h"
 #include "chrome/test/testing_profile.h"
 #include "content/browser/browser_thread.h"
+#include "chrome/browser/history/history_backend.h"
+#include "chrome/browser/history/history.h"
 #include "content/browser/renderer_host/test_render_view_host.h"
 #include "content/browser/tab_contents/navigation_entry.h"
 #include "content/browser/tab_contents/test_tab_contents.h"
@@ -175,9 +178,13 @@
     // The URLFetcher checks that the messageloop type is IO.
     ASSERT_TRUE(io_thread_.StartWithOptions(
         base::Thread::Options(MessageLoop::TYPE_IO, 0)));
+
+    profile_.reset(new TestingProfile);

[mattm, 2011/05/17 03:17:01]

It looks like RenderViewHostTestHarness already creates a TestingProfile?

[kewang, 2011/05/18 05:47:40]

Yes, and it got reset in SetUp() too. Remove the definition and reset here.

+    profile_->CreateHistoryService(false, false);

[mattm, 2011/05/17 03:17:01]

include arg name comments (like false /* delete_file */, false /* no_db */)

[kewang, 2011/05/18 05:47:40]

Done.

   }
 
   virtual void TearDown() {
+    profile_.reset();

[mattm, 2011/05/17 03:17:01]

Unnecessary, unless there is some reason you need this to be destructed during
TearDown instead of when the TestCase is destructed.

[kewang, 2011/05/18 05:47:40]

Done.

     io_thread_.Stop();
     RenderViewHostTestHarness::TearDown();
   }
@@ -200,6 +207,10 @@
     return sb_service_->GetSerialized();
   }
 
+  HistoryService* history_service() {
+    return profile_->GetHistoryService(Profile::EXPLICIT_ACCESS);
+  }
+
  protected:
   void InitResource(SafeBrowsingService::UnsafeResource* resource,
                     ResourceType::Type resource_type,
@@ -278,9 +289,18 @@
     }
   }
 
+  // Adds a page to history.

[mattm, 2011/05/17 03:17:01]

Add a comment about the expected order of redirects.

[kewang, 2011/05/18 05:47:40]

Done.

+  void AddPageToHistory(const GURL& url,
+                        const history::RedirectList& redirects) {
+    history_service()->AddPage(
+        url, static_cast<void*>(this), 0, GURL(), PageTransition::TYPED,
+        redirects, history::SOURCE_BROWSED, false);
+  }
+
   BrowserThread ui_thread_;
   BrowserThread io_thread_;
   scoped_refptr<MockSafeBrowsingService> sb_service_;
+  scoped_ptr<TestingProfile> profile_;

[mattm, 2011/05/17 03:17:01]

RenderViewHostTestHarness already has a profile_ member

[kewang, 2011/05/18 05:47:40]

removed.

 };
 
 // Tests creating a simple malware report.
@@ -592,3 +612,62 @@
 
   VerifyResults(actual, expected);
 }
+
+// Test getting redirects from history service

[Paweł Hajdan Jr., 2011/05/17 15:13:54]

nit: Dot at the end please.

[kewang, 2011/05/18 05:47:40]

Done.

+TEST_F(MalwareDetailsTest, HistoryServiceUrls) {
+  // Add content to history service

[Paweł Hajdan Jr., 2011/05/17 15:13:54]

nit: Dot at the end.

[kewang, 2011/05/18 05:47:40]

Done.

+  GURL baseurl(kMalwareURL);
+  history::RedirectList redirects;
+  redirects.push_back(GURL(kFirstRedirectURL));
+  redirects.push_back(GURL(kSecondRedirectURL));
+  // the last item of the redirect chain has to be the final URL

[mattm, 2011/05/17 03:17:01]

I was a bit confused since that's opposite the order the history service returns
them in.  Maybe add a bit to the comment like "when adding".

[Paweł Hajdan Jr., 2011/05/17 15:13:54]

nit: Start with a capital letter and end with a dot.

[kewang, 2011/05/18 05:47:40]

yes, history backend has a check: request->redirects.back() == request->url. The
order is actually the url redirection order.

I modified the AddPageToHistory() to add the ending url there.

[kewang, 2011/05/18 05:47:40]

Done.

+  redirects.push_back(baseurl);
+  AddPageToHistory(baseurl, redirects);
+  // Wait for history service operation finished

[Paweł Hajdan Jr., 2011/05/17 15:13:54]

nit: Dot at the end.

[kewang, 2011/05/18 05:47:40]

Done.

+  profile_->BlockUntilHistoryProcessesPendingRequests();
+
+  controller().LoadURL(GURL(kLandingURL), GURL(), PageTransition::TYPED);
+
+  SafeBrowsingService::UnsafeResource resource;
+  InitResource(&resource, ResourceType::SUB_RESOURCE, GURL(kMalwareURL));
+  scoped_refptr<MalwareDetailsWrap> report = new MalwareDetailsWrap(
+      sb_service_.get(), contents(), resource, NULL);
+
+  // Reset the history service pointer
+  report->redirects_collector_->SetHistory(history_service());
+
+  // The redirects collection starts after the IPC from the DOM is fired.
+  std::vector<SafeBrowsingHostMsg_MalwareDOMDetails_Node> params;
+  report->OnReceivedMalwareDOMDetails(params);
+
+  // Let the redirects callbacks complete

[Paweł Hajdan Jr., 2011/05/17 15:13:54]

nit: Dot at the end.

[kewang, 2011/05/18 05:47:40]

Done.

+  MessageLoop::current()->RunAllPending();
+
+  std::string serialized = WaitForSerializedReport(report);
+  ClientMalwareReportRequest actual;
+  actual.ParseFromString(serialized);
+
+  ClientMalwareReportRequest expected;
+  expected.set_malware_url(kMalwareURL);
+  expected.set_page_url(kLandingURL);
+  expected.set_referrer_url("");
+
+  ClientMalwareReportRequest::Resource* pb_resource = expected.add_resources();
+  pb_resource->set_id(0);
+  pb_resource->set_url(kLandingURL);
+  pb_resource = expected.add_resources();
+  pb_resource->set_id(1);
+  pb_resource->set_parent_id(2);
+  pb_resource->set_url(kMalwareURL);
+  pb_resource = expected.add_resources();
+  pb_resource->set_id(2);
+  pb_resource->set_parent_id(3);
+  pb_resource->set_tag_name("redir");
+  pb_resource->set_url(kSecondRedirectURL);
+  pb_resource = expected.add_resources();
+  pb_resource->set_id(3);
+  pb_resource->set_tag_name("redir");
+  pb_resource->set_url(kFirstRedirectURL);
+
+  VerifyResults(actual, expected);
+}