Cleanup in InstantTab and InstantController

chrome/browser/ui/search/instant_tab_unittest.cc

 // Copyright 2013 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 "chrome/browser/ui/search/instant_tab.h"
 
 #include <stdint.h>
 
 #include <memory>
 
@@ skipping 17 matching lines @@
 namespace {
 
 class FakePageDelegate : public InstantTab::Delegate {
  public:
   virtual ~FakePageDelegate() {
   }
 
   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
 
 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;
   FakePageDelegate delegate;
 };
 
 void InstantTabTest::SetUp() {
   ChromeRenderViewHostTestHarness::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());
+TEST_F(InstantTabTest, DetermineIfPageSupportsInstant_Local) {
+  page.reset(new InstantTab(&delegate, web_contents()));
+  EXPECT_FALSE(search_tab()->SupportsInstant());
   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());
-  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();
   const IPC::Message* message = process()->sink().GetFirstMessageMatching(
       ChromeViewMsg_DetermineIfPageSupportsInstant::ID);
   ASSERT_TRUE(message != NULL);
   EXPECT_EQ(web_contents()->GetRoutingID(), message->routing_id());
 }
 
 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);
 
   SearchTabHelper::FromWebContents(web_contents())->
       DetermineIfPageSupportsInstant();
   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())->
       DetermineIfPageSupportsInstant();
   const IPC::Message* message = process()->sink().GetFirstMessageMatching(
       ChromeViewMsg_DetermineIfPageSupportsInstant::ID);
   ASSERT_TRUE(message != NULL);
   EXPECT_EQ(web_contents()->GetRoutingID(), message->routing_id());
 
   EXPECT_CALL(delegate, InstantSupportDetermined(web_contents(), true))
       .Times(1);
 
   // Assume the page supports instant. Invoke the message reply handler to make
   // sure the InstantTab is notified about the instant support state.
   const content::NavigationEntry* entry =
       web_contents()->GetController().GetLastCommittedEntry();
   EXPECT_TRUE(entry);
   SearchTabHelper::FromWebContents(web_contents())->InstantSupportChanged(true);
-  EXPECT_TRUE(page->supports_instant());
+  EXPECT_TRUE(search_tab()->SupportsInstant());
 }