Cleanup in InstantTab and InstantController

chrome/browser/ui/search/instant_tab_unittest.cc

Index: chrome/browser/ui/search/instant_tab_unittest.cc
diff --git a/chrome/browser/ui/search/instant_tab_unittest.cc b/chrome/browser/ui/search/instant_tab_unittest.cc
index 97d92e0858116c1ffd4f7418999f95113ea05992..00d1f9b792764eaba4a0d052fd282dc6fae8c441 100644
--- a/chrome/browser/ui/search/instant_tab_unittest.cc
+++ b/chrome/browser/ui/search/instant_tab_unittest.cc
@@ -35,17 +35,9 @@ class FakePageDelegate : public InstantTab::Delegate {
   MOCK_METHOD2(InstantSupportDetermined,
                void(const content::WebContents* contents,
                     bool supports_instant));
-  MOCK_METHOD1(InstantTabRenderProcessGone,
-               void(const content::WebContents* contents));
   MOCK_METHOD2(InstantTabAboutToNavigateMainFrame,
                void(const content::WebContents* contents,
                     const GURL& url));
-  MOCK_METHOD5(NavigateToURL,
-               void(const content::WebContents* contents,
-                    const GURL& url,
-                    ui::PageTransition transition,
-                    WindowOpenDisposition disposition,
-                    bool is_search_type));
 };
 
 }  // namespace
@@ -54,8 +46,8 @@ class InstantTabTest : public ChromeRenderViewHostTestHarness {
  public:
   void SetUp() override;
 
-  bool MessageWasSent(uint32_t id) {
-    return process()->sink().GetFirstMessageMatching(id) != NULL;
+  SearchTabHelper* search_tab() {
+    return SearchTabHelper::FromWebContents(web_contents());
   }
 
   std::unique_ptr<InstantTab> page;
@@ -67,36 +59,21 @@ void InstantTabTest::SetUp() {
   SearchTabHelper::CreateForWebContents(web_contents());
 }
 
-TEST_F(InstantTabTest, IsLocal) {
-  page.reset(new InstantTab(&delegate));
-  EXPECT_FALSE(page->supports_instant());
-  EXPECT_FALSE(page->IsLocal());
-  page->Init(web_contents());
-  NavigateAndCommit(GURL(chrome::kChromeSearchLocalNtpUrl));
-  EXPECT_TRUE(page->IsLocal());
-  NavigateAndCommit(GURL("http://example.com"));
-  EXPECT_FALSE(page->IsLocal());
-}
-
 TEST_F(InstantTabTest, DetermineIfPageSupportsInstant_Local) {
-  page.reset(new InstantTab(&delegate));
-  EXPECT_FALSE(page->supports_instant());
-  page->Init(web_contents());
+  page.reset(new InstantTab(&delegate, web_contents()));
+  EXPECT_FALSE(search_tab()->SupportsInstant());
   NavigateAndCommit(GURL(chrome::kChromeSearchLocalNtpUrl));
-  EXPECT_TRUE(page->IsLocal());
   EXPECT_CALL(delegate, InstantSupportDetermined(web_contents(), true))
       .Times(1);

[sfiera, 2016/07/22 12:51:00]

Not needed; gmock will infer .Times(1) when there aren't any ".Will...()"

[Marc Treib, 2016/07/22 13:11:17]

Does it? I thought that without the explicit ".Times(1)", a second call would be
considered "uninteresting" but not an error, while with ".Times(1)" it would be
an error?

[sfiera, 2016/07/25 08:20:06]

Nope, .Times(1) is the same thing.

In general, I prefer to use StrictMock<> on pretty much everything anyway, in
which case all uninteresting calls are errors. Here,
InstantTabAboutToNavigateMainFrame() could be called without failing the test.

[Marc Treib, 2016/07/25 08:56:25]

Alright, removed the "Times(1)" everywhere.
That seems like a good policy; I should adopt that too :)
Changing it here is an exercise for another day though.

   SearchTabHelper::FromWebContents(web_contents())->
       DetermineIfPageSupportsInstant();
-  EXPECT_TRUE(page->supports_instant());
+  EXPECT_TRUE(search_tab()->SupportsInstant());
 }
 
 TEST_F(InstantTabTest, DetermineIfPageSupportsInstant_NonLocal) {
-  page.reset(new InstantTab(&delegate));
-  EXPECT_FALSE(page->supports_instant());
-  page->Init(web_contents());
+  page.reset(new InstantTab(&delegate, web_contents()));
+  EXPECT_FALSE(search_tab()->SupportsInstant());
   NavigateAndCommit(GURL("chrome-search://foo/bar"));
-  EXPECT_FALSE(page->IsLocal());
   process()->sink().ClearMessages();
   SearchTabHelper::FromWebContents(web_contents())->
       DetermineIfPageSupportsInstant();
@@ -107,16 +84,14 @@ TEST_F(InstantTabTest, DetermineIfPageSupportsInstant_NonLocal) {
 }
 
 TEST_F(InstantTabTest, PageURLDoesntBelongToInstantRenderer) {
-  page.reset(new InstantTab(&delegate));
-  EXPECT_FALSE(page->supports_instant());
+  page.reset(new InstantTab(&delegate, web_contents()));
+  EXPECT_FALSE(search_tab()->SupportsInstant());
   NavigateAndCommit(GURL(chrome::kChromeSearchLocalNtpUrl));
-  page->Init(web_contents());
 
   // Navigate to a page URL that doesn't belong to Instant renderer.
   // SearchTabHelper::DeterminerIfPageSupportsInstant() should return
   // immediately without dispatching any message to the renderer.
   NavigateAndCommit(GURL("http://www.example.com"));
-  EXPECT_FALSE(page->IsLocal());
   process()->sink().ClearMessages();
   EXPECT_CALL(delegate, InstantSupportDetermined(web_contents(), false))
       .Times(1);
@@ -126,15 +101,14 @@ TEST_F(InstantTabTest, PageURLDoesntBelongToInstantRenderer) {
   const IPC::Message* message = process()->sink().GetFirstMessageMatching(
       ChromeViewMsg_DetermineIfPageSupportsInstant::ID);
   ASSERT_TRUE(message == NULL);
-  EXPECT_FALSE(page->supports_instant());
+  EXPECT_FALSE(search_tab()->SupportsInstant());
 }
 
 // Test to verify that ChromeViewMsg_DetermineIfPageSupportsInstant message
 // reply handler updates the instant support state in InstantTab.
 TEST_F(InstantTabTest, PageSupportsInstant) {
-  page.reset(new InstantTab(&delegate));
-  EXPECT_FALSE(page->supports_instant());
-  page->Init(web_contents());
+  page.reset(new InstantTab(&delegate, web_contents()));
+  EXPECT_FALSE(search_tab()->SupportsInstant());
   NavigateAndCommit(GURL("chrome-search://foo/bar"));
   process()->sink().ClearMessages();
   SearchTabHelper::FromWebContents(web_contents())->
@@ -153,5 +127,5 @@ TEST_F(InstantTabTest, PageSupportsInstant) {
       web_contents()->GetController().GetLastCommittedEntry();
   EXPECT_TRUE(entry);
   SearchTabHelper::FromWebContents(web_contents())->InstantSupportChanged(true);
-  EXPECT_TRUE(page->supports_instant());
+  EXPECT_TRUE(search_tab()->SupportsInstant());
 }