Instant/LocalNTP tests cleanup

chrome/browser/ui/search/instant_extended_interactive_uitest.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 <stdint.h>
 
 #include <sstream>
 
 #include "base/base_switches.h"
 #include "base/command_line.h"
@@ skipping 14 matching lines @@
 #include "chrome/browser/history/history_service_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/search/instant_service.h"
 #include "chrome/browser/search/instant_service_factory.h"
 #include "chrome/browser/search/search.h"
 #include "chrome/browser/search_engines/template_url_service_factory.h"
 #include "chrome/browser/themes/theme_service.h"
 #include "chrome/browser/themes/theme_service_factory.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/browser_tabstrip.h"
+#include "chrome/browser/ui/browser_window.h"
+#include "chrome/browser/ui/location_bar/location_bar.h"
 #include "chrome/browser/ui/search/instant_tab.h"
 #include "chrome/browser/ui/search/instant_test_utils.h"
+#include "chrome/browser/ui/search/instant_uitest_base.h"
 #include "chrome/browser/ui/search/search_tab_helper.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/webui/theme_source.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/search/instant_types.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/interactive_test_utils.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "components/bookmarks/browser/bookmark_utils.h"
@@ skipping 76 matching lines @@
 
   ~FakeNetworkChangeNotifier() override {}
 
  private:
   ConnectionType connection_type_;
   DISALLOW_COPY_AND_ASSIGN(FakeNetworkChangeNotifier);
 };
 }  // namespace
 
 class InstantExtendedTest : public InProcessBrowserTest,
-                            public InstantTestBase {
+                            public InstantUITestBase {

[Bernhard Bauer, 2017/05/03 16:48:10]

I can't say I'm super happy about using multiple inheritance like this here (and
I think neither would your manager 😉), but then again it's already been this
way... Maybe for a followup, could we think about using composition to get this
additional functionality?

[Marc Treib, 2017/05/04 08:25:40]

Agreed re multiple inheritance, but one step at a time :)

Assuming Instant stays unlaunched, much of this class can probably just be
deleted. And yes, the rest can then be used via composition, or just global
helper functions, like the Get*FromJS ones.

  public:
   InstantExtendedTest()
       : on_most_visited_change_calls_(0),
         most_visited_items_count_(0),
         first_most_visited_item_id_(0),
         submit_count_(0),
         on_focus_changed_calls_(0),
         is_focused_(false) {}
 
  protected:
@@ skipping 11 matching lines @@
         base::StatisticsRecorder::FindHistogram(name);
     if (!histogram) {
       // If no histogram is found, it's possible that no values have been
       // recorded yet. Assume that the value is zero.
       return 0;
     }
     return histogram->SnapshotSamples()->TotalCount();
   }
 
   bool UpdateSearchState(content::WebContents* contents) WARN_UNUSED_RESULT {
-    return GetIntFromJS(contents, "onMostVisitedChangedCalls",
-                        &on_most_visited_change_calls_) &&
-           GetIntFromJS(contents, "mostVisitedItemsCount",
-                        &most_visited_items_count_) &&
-           GetIntFromJS(contents, "firstMostVisitedItemId",
-                        &first_most_visited_item_id_) &&
-           GetIntFromJS(contents, "submitCount",
-                        &submit_count_) &&
-           GetStringFromJS(contents, "apiHandle.value",
-                           &query_value_) &&
-           GetIntFromJS(contents, "onFocusChangedCalls",
-                       &on_focus_changed_calls_) &&
-           GetBoolFromJS(contents, "isFocused",
-                         &is_focused_) &&
-           GetStringFromJS(contents, "prefetchQuery", &prefetch_query_value_);
+    return instant_test_utils::GetIntFromJS(contents,
+                                            "onMostVisitedChangedCalls",
+                                            &on_most_visited_change_calls_) &&
+           instant_test_utils::GetIntFromJS(contents, "mostVisitedItemsCount",
+                                            &most_visited_items_count_) &&
+           instant_test_utils::GetIntFromJS(contents, "firstMostVisitedItemId",
+                                            &first_most_visited_item_id_) &&
+           instant_test_utils::GetIntFromJS(contents, "submitCount",
+                                            &submit_count_) &&
+           instant_test_utils::GetStringFromJS(contents, "apiHandle.value",
+                                               &query_value_) &&
+           instant_test_utils::GetIntFromJS(contents, "onFocusChangedCalls",
+                                            &on_focus_changed_calls_) &&
+           instant_test_utils::GetBoolFromJS(contents, "isFocused",
+                                             &is_focused_) &&
+           instant_test_utils::GetStringFromJS(contents, "prefetchQuery",
+                                               &prefetch_query_value_);
   }
 
   const TemplateURL* GetDefaultSearchProviderTemplateURL() {
     TemplateURLService* template_url_service =
         TemplateURLServiceFactory::GetForProfile(browser()->profile());
     if (template_url_service)
       return template_url_service->GetDefaultSearchProvider();
     return NULL;
   }
 
@@ skipping 68 matching lines @@
 
   // Used to mock default search provider suggest response.
   std::unique_ptr<net::FakeURLFetcherFactory> fake_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(InstantExtendedPrefetchTest);
 };
 
 // Test class used to verify chrome-search: scheme and access policy from the
 // Instant overlay.  This is a subclass of |ExtensionBrowserTest| because it
 // loads a theme that provides a background image.
-class InstantPolicyTest : public ExtensionBrowserTest, public InstantTestBase {
+class InstantPolicyTest : public ExtensionBrowserTest,
+                          public InstantUITestBase {
  public:
   InstantPolicyTest() {}
 
  protected:
   void SetUpInProcessBrowserTestFixture() override {
     ASSERT_TRUE(https_test_server().Start());
     GURL instant_url =
         https_test_server().GetURL("/instant_extended.html?strk=1&");
     GURL ntp_url =
         https_test_server().GetURL("/instant_extended_ntp.html?strk=1&");
@@ skipping 169 matching lines @@
       browser(), GURL(chrome::kChromeUINewTabURL),
       WindowOpenDisposition::NEW_FOREGROUND_TAB,
       ui_test_utils::BROWSER_TEST_WAIT_FOR_TAB |
           ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
   EXPECT_EQ(2, browser()->tab_strip_model()->count());
 
   content::WebContents* active_tab =
       browser()->tab_strip_model()->GetActiveWebContents();
   ASSERT_EQ(1, browser()->tab_strip_model()->active_index());
   int on_theme_changed_calls = 0;
-  EXPECT_TRUE(GetIntFromJS(active_tab, "onThemeChangedCalls",
-                           &on_theme_changed_calls));
+  EXPECT_TRUE(instant_test_utils::GetIntFromJS(
+      active_tab, "onThemeChangedCalls", &on_theme_changed_calls));
   EXPECT_EQ(1, on_theme_changed_calls);
 
   // Activate the previous tab.
   browser()->tab_strip_model()->ActivateTabAt(0, false);
   ASSERT_EQ(0, browser()->tab_strip_model()->active_index());
 
   // Switch back to new tab.
   browser()->tab_strip_model()->ActivateTabAt(1, false);
   ASSERT_EQ(1, browser()->tab_strip_model()->active_index());
 
   // Confirm that new tab got no onthemechanged event while switching tabs.
   active_tab = browser()->tab_strip_model()->GetActiveWebContents();
   on_theme_changed_calls = 0;
-  EXPECT_TRUE(GetIntFromJS(active_tab, "onThemeChangedCalls",
-                           &on_theme_changed_calls));
+  EXPECT_TRUE(instant_test_utils::GetIntFromJS(
+      active_tab, "onThemeChangedCalls", &on_theme_changed_calls));
   EXPECT_EQ(1, on_theme_changed_calls);
 }
 
 // Flaky on all bots. http://crbug.com/335297, http://crbug.com/265971.
 IN_PROC_BROWSER_TEST_F(InstantPolicyTest,
                        DISABLED_SendThemeBackgroundChangedEvent) {
   InstallThemeSource();
   ASSERT_NO_FATAL_FAILURE(SetupInstant(browser()));
 
   // Install a theme.
   ASSERT_NO_FATAL_FAILURE(InstallThemeAndVerify("theme", "camo theme"));
 
   // Open new tab.
   ui_test_utils::NavigateToURLWithDisposition(
       browser(), GURL(chrome::kChromeUINewTabURL),
       WindowOpenDisposition::NEW_FOREGROUND_TAB,
       ui_test_utils::BROWSER_TEST_WAIT_FOR_TAB |
           ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
   EXPECT_EQ(2, browser()->tab_strip_model()->count());
 
   // Make sure new tab received an onthemechanged event.
   content::WebContents* active_tab =
       browser()->tab_strip_model()->GetActiveWebContents();
   ASSERT_EQ(1, browser()->tab_strip_model()->active_index());
   int on_theme_changed_calls = 0;
-  EXPECT_TRUE(GetIntFromJS(active_tab, "onThemeChangedCalls",
-                           &on_theme_changed_calls));
+  EXPECT_TRUE(instant_test_utils::GetIntFromJS(
+      active_tab, "onThemeChangedCalls", &on_theme_changed_calls));
   EXPECT_EQ(1, on_theme_changed_calls);
 
   // Install a new theme.
   ASSERT_NO_FATAL_FAILURE(InstallThemeAndVerify("theme2", "snowflake theme"));
 
   // Confirm that new tab is notified about the theme changed event.
   on_theme_changed_calls = 0;
-  EXPECT_TRUE(GetIntFromJS(active_tab, "onThemeChangedCalls",
-                           &on_theme_changed_calls));
+  EXPECT_TRUE(instant_test_utils::GetIntFromJS(
+      active_tab, "onThemeChangedCalls", &on_theme_changed_calls));
   EXPECT_EQ(2, on_theme_changed_calls);
 }
 
 // Flaky on all bots since re-enabled in r208032, crbug.com/253092
 IN_PROC_BROWSER_TEST_F(InstantExtendedTest, DISABLED_NavigateBackToNTP) {
   ASSERT_NO_FATAL_FAILURE(SetupInstant(browser()));
   FocusOmnibox();
 
   // Open a new tab page.
   ui_test_utils::NavigateToURLWithDisposition(
@@ skipping 208 matching lines @@
   stream << "link.href = \"" << result_url.spec() << "\";";
   stream << "document.body.appendChild(link);";
   stream << "link.click();";
   EXPECT_TRUE(content::ExecuteScript(contents, stream.str()));
 
   content::WaitForLoadStop(contents);
   std::string expected_title =
       "Referrer is " + instant_url().GetWithEmptyPath().spec();
   EXPECT_EQ(ASCIIToUTF16(expected_title), contents->GetTitle());
 }