Reload Instant NTP and Instant-process tabs on search url change

chrome/browser/ui/browser_instant_controller_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 <string>
+
+#include "base/basictypes.h"
+#include "chrome/browser/chrome_notification_types.h"
+#include "chrome/browser/search/instant_service.h"
+#include "chrome/browser/search/instant_unittest_base.h"
+#include "chrome/browser/search/search.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/common/pref_names.h"
+#include "chrome/common/url_constants.h"
+#include "content/public/browser/navigation_controller.h"
+#include "content/public/browser/render_process_host.h"
+#include "content/public/browser/web_contents.h"
+#include "content/public/browser/web_contents_observer.h"
+
+namespace chrome {
+
+namespace {
+
+class BrowserInstantControllerTest : public InstantUnitTestBase {
+ protected:
+  friend class FakeWebContentsObserver;
+};
+
+class FakeWebContentsObserver : public content::WebContentsObserver {
+ public:
+  FakeWebContentsObserver(BrowserInstantControllerTest* base_test,
+                          const GURL& url,
+                          content::NavigationController::ReloadType reload_type,
+                          const char* description)
+      : base_test_(base_test),
+        url_(url),
+        reload_type_(reload_type),
+        description_(description),
+        validated_(0) {}
+
+  void NavigateToPendingEntry(

[samarth, 2013/08/12 19:07:28]

This is an OVERRIDE, right? (and should be virtual?)

[Anuj, 2013/08/13 00:18:09]

Done.

+      const GURL& url,
+      content::NavigationController::ReloadType reload_type) {
+    EXPECT_EQ(url_, url) << description_;
+    EXPECT_EQ(reload_type_, reload_type) << description_;
+    content::NavigationController* controller =
+        &web_contents()->GetController();
+    Observe(NULL);
+    base_test_->NavigateAndCommit(controller, url_);
+    validated_++;
+  }
+
+  bool Validated() const {
+    return validated_ == 1;

[samarth, 2013/08/12 19:07:28]

What is being validated here? Please use a more descriptive name.

[Anuj, 2013/08/13 00:18:09]

Done.

+  }
+
+ protected:
+  friend class BrowserInstantControllerTabReloadTest;
+  FRIEND_TEST_ALL_PREFIXES(BrowserInstantControllerTabReloadTest,
+                           DefaultSearchProviderChanged);
+
+ private:
+  BrowserInstantControllerTest* base_test_;
+  const GURL& url_;
+  content::NavigationController::ReloadType reload_type_;
+  const char* description_;
+  int validated_;
+};
+
+const struct TabReloadTestCase {
+  const char* description;
+  const char* start_url;
+  bool start_in_instant_process;
+  bool should_reload;
+  bool end_in_instant_process;
+} kTabReloadTestCases[] = {
+    {"Local Embedded SEHP", chrome::kChromeSearchLocalNtpUrl,

[samarth, 2013/08/12 19:07:28]

s/Embedded SEHP/NTP

[Anuj, 2013/08/13 00:18:09]

Done.

+     true, true, true},
+    {"Remote Embedded SEHP", "https://www.google.com/instant?strk",
+     true, true, false},
+    {"Remote Embedded SERP", "https://www.google.com/url?strk",
+     true, true, false},
+    {"Other SEHP", "https://bar.com/instant?strk",
+     false, false, false}
+};
+
+struct TabInfo {
+  content::WebContents* contents;
+  FakeWebContentsObserver* observer;
+  const GURL* url;
+  int pid;
+};
+
+class BrowserInstantControllerTabReloadTest
+    : public BrowserInstantControllerTest {
+ protected:
+  TabInfo* SetUpTabReloadTestCases() {

[samarth, 2013/08/12 19:07:28]

This is way too complex to follow. I shouldn't feel the need to have tests for
your test code :)

Let's get rid of TabInfo and just inline setup and validation in the actual
tests. Yes, you'll repeat it but it'll be more readable overall I think.

Also, just make a FakeWebContentsObserver for each tab and then add a DidReload
helper function to test whether a reload actually happened.

Please also pepper comments throughout about what the setup / verification steps
are.

[Anuj, 2013/08/13 00:18:09]

Done.

+    size_t numTestCases = arraysize(kTabReloadTestCases);
+    TabInfo* tabInfos = new TabInfo[numTestCases];
+    for (size_t i = 0; i < numTestCases; ++i) {
+      const TabReloadTestCase& test = kTabReloadTestCases[i];
+      TabInfo& tabInfo = tabInfos[i];
+      AddTab(browser(), GURL(test.start_url));
+      tabInfo.contents = browser()->tab_strip_model()->GetActiveWebContents();
+      tabInfo.url = &tabInfo.contents->GetURL();
+      tabInfo.pid = tabInfo.contents->GetRenderProcessHost()->GetID();
+      EXPECT_EQ(test.start_in_instant_process,
+                chrome::IsInstantNTP(tabInfo.contents))
+          << test.description;
+      EXPECT_EQ(test.start_in_instant_process,
+                instant_service_->IsInstantProcess(tabInfos[i].pid))
+          << test.description;
+      if (test.should_reload) {
+        tabInfo.observer = new FakeWebContentsObserver(
+            this,
+            tabInfo.contents->GetURL(),
+            (test.start_in_instant_process == test.end_in_instant_process)
+                ? content::NavigationController::RELOAD
+                : content::NavigationController::NO_RELOAD,
+            test.description);
+        tabInfo.observer->Observe(tabInfo.contents);
+      }
+    }
+    return tabInfos;
+  }
+
+  void ValidateTabReloadTestCases(TabInfo* tabInfos) {
+    size_t numTestCases = arraysize(kTabReloadTestCases);
+    for (size_t i = 0; i < numTestCases; ++i) {
+      const TabReloadTestCase& test = kTabReloadTestCases[i];
+      TabInfo& tabInfo = tabInfos[i];
+      if (test.should_reload) {
+        EXPECT_TRUE(tabInfo.observer->Validated()) << test.description;
+      }
+      int pid = tabInfo.contents->GetRenderProcessHost()->GetID();
+      EXPECT_EQ(*tabInfo.url, tabInfo.contents->GetURL()) << test.description;
+      if (test.should_reload) {
+        EXPECT_EQ(test.end_in_instant_process,
+                  ShouldAssignURLToInstantRenderer(tabInfo.contents->GetURL(),
+                                                   profile()))
+            << test.description;
+      }
+      EXPECT_EQ(test.end_in_instant_process,
+                chrome::IsInstantNTP(tabInfo.contents))
+          << test.description;
+      EXPECT_EQ(test.end_in_instant_process,
+                instant_service_->IsInstantProcess(pid))
+          << test.description;
+      EXPECT_EQ(test.start_in_instant_process == test.end_in_instant_process,
+                pid == tabInfo.pid)
+          << test.description;
+    }
+  }
+
+  friend class FakeWebContentsObserver;
+};
+
+TEST_F(BrowserInstantControllerTabReloadTest, DefaultSearchProviderChanged) {
+  EXPECT_EQ(1, instant_service_->GetInstantProcessCount());
+  TabInfo* tabInfos = SetUpTabReloadTestCases();
+
+  SetSearchProvider("https://bar.com/");
+
+  ValidateTabReloadTestCases(tabInfos);
+}
+
+TEST_F(BrowserInstantControllerTabReloadTest, GoogleBaseURLUpdated) {
+  EXPECT_EQ(1, instant_service_->GetInstantProcessCount());
+  TabInfo* tabInfos = SetUpTabReloadTestCases();
+
+  NotifyGoogleBaseURLUpdate("https://www.google.es/");
+
+  ValidateTabReloadTestCases(tabInfos);
+}
+
+}  // namespace
+}  // namespace chrome