Added support for passing WindowOpenDisposition into BrowserInstantController::OpenURL() from the o…

chrome/browser/instant/instant_extended_browsertest.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/instant/instant_commit_type.h"
 #include "chrome/browser/instant/instant_ntp.h"
 #include "chrome/browser/instant/instant_overlay.h"
 #include "chrome/browser/instant/instant_service.h"
 #include "chrome/browser/instant/instant_service_factory.h"
 #include "chrome/browser/instant/instant_test_utils.h"
 #include "chrome/browser/ui/search/search.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/test/base/interactive_test_utils.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/web_contents.h"
 
+#include "content/public/browser/web_contents.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"

[sreeram, 2013/02/20 22:53:32]

Delete lines 20 and 21 (unnecessary blank line and repeated include). Move line
22 into its correct place in the sorted order.

[dougw, 2013/02/20 23:35:11]

Done.

+
 class InstantExtendedTest : public InstantTestBase {
  protected:
   virtual void SetUpInProcessBrowserTestFixture() OVERRIDE {
     chrome::search::EnableInstantExtendedAPIForTesting();
     ASSERT_TRUE(https_test_server_.Start());
     instant_url_ = https_test_server_.
         GetURL("files/instant_extended.html?strk=1&");
   }
 
   void FocusOmniboxAndWaitForInstantSupport() {
@@ skipping 35 matching lines @@
   ASSERT_NO_FATAL_FAILURE(SetupInstant());
   EXPECT_TRUE(instant()->extended_enabled_);
 }
 
 // Test that Instant is preloaded when the omnibox is focused.
 IN_PROC_BROWSER_TEST_F(InstantExtendedTest, OmniboxFocusLoadsInstant) {
   ASSERT_NO_FATAL_FAILURE(SetupInstant());
 
   // Explicitly unfocus the omnibox.
   EXPECT_TRUE(ui_test_utils::BringBrowserWindowToFront(browser()));
-  ui_test_utils::ClickOnView(browser(), VIEW_ID_TAB_CONTAINER);
+  ui_test_utils::ClickOnView(browser(),
+ VIEW_ID_TAB_CONTAINER);

[sreeram, 2013/02/20 22:53:32]

Revert this change.

[dougw, 2013/02/20 23:35:11]

Done.

 
   EXPECT_TRUE(ui_test_utils::IsViewFocused(browser(), VIEW_ID_TAB_CONTAINER));
   EXPECT_FALSE(omnibox()->model()->has_focus());
 
   // Delete any existing preview.
   instant()->overlay_.reset();
   EXPECT_FALSE(instant()->GetPreviewContents());
 
   // Refocus the omnibox. The InstantController should've preloaded Instant.
   FocusOmniboxAndWaitForInstantSupport();
@@ skipping 32 matching lines @@
   EXPECT_TRUE(preview_tab);
   EXPECT_FALSE(instant()->IsPreviewingSearchResults());
   EXPECT_TRUE(instant()->model()->mode().is_default());
 
   // Typing in the omnibox should show the overlay.
   SetOmniboxTextAndWaitForInstantToShow("query");
   EXPECT_TRUE(instant()->model()->mode().is_search_suggestions());
   EXPECT_EQ(preview_tab, instant()->GetPreviewContents());
 }
 
+// Test that middle clicking on a suggestion opens the result in a new tab.
+IN_PROC_BROWSER_TEST_F(
+    InstantExtendedTest, MiddleClickOnSuggestionOpensInNewTab) {

[sreeram, 2013/02/20 22:53:32]

Reformat like so:
...(InstantExtendedTest,
    MiddleClick...) {

[dougw, 2013/02/20 23:35:11]

Done.

+  ASSERT_NO_FATAL_FAILURE(SetupInstant());
+  FocusOmniboxAndWaitForInstantSupport();
+  EXPECT_TRUE(ui_test_utils::BringBrowserWindowToFront(browser()));
+
+  EXPECT_EQ(1, browser()->tab_strip_model()->count());
+
+  // Typing in the omnibox should show the overlay.
+  SetOmniboxTextAndWaitForInstantToShow("santa");
+  EXPECT_TRUE(instant()->model()->mode().is_search_suggestions());
+
+  // Create an event listener that opens the top suggestion in a new tab.
+  EXPECT_TRUE(ExecuteScript(
+      "var nativeSuggestions = getApiHandle().nativeSuggestions;"
+      "var rid = nativeSuggestions[0].rid;"
+      "document.body.addEventListener(\"click\", function() {"
+      "if (nativeSuggestions.length > 0) {"

[sreeram, 2013/02/20 22:53:32]

Why are you testing for nativeSuggestions here? You shouldn't touch
nativeSuggestions at all, as we discussed.

[dougw, 2013/02/20 23:35:11]

Done. That's a cached copy of nativeSuggestions - we need it to access the rid,
if nothing else. I've removed the if statement since it isn't necessary.

On 2013/02/20 22:53:32, sreeram wrote:

+      "chrome.embeddedSearch.navigateContentWindow("
+      "rid, 2);}});"

[sreeram, 2013/02/20 22:53:32]

The indentation of this code block (within the string) is wrong. Please fix.

[dougw, 2013/02/20 23:35:11]

Done.

On 2013/02/20 22:53:32, sreeram wrote:

+      ));
+
+  EXPECT_EQ(100, instant()->model()->height());
+  EXPECT_EQ(INSTANT_SIZE_PERCENT, instant()->model()->height_units());
+  EXPECT_TRUE(instant()->IsPreviewingSearchResults());

[sreeram, 2013/02/20 22:53:32]

No need for these expectations. Perhaps just delete line 156-157 and replace
line 144 with line 158.

[dougw, 2013/02/20 23:35:11]

Done.

+
+  content::WindowedNotificationObserver observer(
+        chrome::NOTIFICATION_TAB_ADDED,
+        content::NotificationService::AllSources());
+
+  // Click to trigger the event listener.
+  ui_test_utils::ClickOnView(browser(), VIEW_ID_TAB_CONTAINER);
+
+  // Wait for the new tab to be added.
+  observer.Wait();
+
+  // Check that the new tab url is as expected.

[sreeram, 2013/02/20 22:53:32]

url -> URL

[dougw, 2013/02/20 23:35:11]

Done.

+  content::WebContents* newTabContents =

[sreeram, 2013/02/20 22:53:32]

newTabContents -> new_tab_contents

[dougw, 2013/02/20 23:35:11]

Done.

+      browser()->tab_strip_model()->GetWebContentsAt(1);
+  EXPECT_EQ(newTabContents->GetURL().possibly_invalid_spec(),
+      instant_url_.possibly_invalid_spec()+"q=santa");

[sreeram, 2013/02/20 22:53:32]

I think you can just use ".spec()" here, since we expect this to succeed. If the
.spec() fails, our expectation would also have failed, so it's fine.

[dougw, 2013/02/20 23:35:11]

Done.

+
+  // Check that there are now two tabs.
+  EXPECT_EQ(2, browser()->tab_strip_model()->count());
+}
+
 // TODO(sreeram): Enable this test once @mathp's CL lands:
 //     https://codereview.chromium.org/12179025/
 //
 // Test that omnibox text is correctly set when overlay is committed with Enter.
 IN_PROC_BROWSER_TEST_F(InstantExtendedTest,
                        DISABLED_OmniboxTextUponEnterCommit) {
   ASSERT_NO_FATAL_FAILURE(SetupInstant());
   FocusOmniboxAndWaitForInstantSupport();
 
   // The page will autocomplete once we set the omnibox value.
@@ skipping 264 matching lines @@
   content::WebContents* active_tab =
       browser()->tab_strip_model()->GetActiveWebContents();
   EXPECT_TRUE(instant_service->IsInstantProcess(
       active_tab->GetRenderProcessHost()->GetID()));
 
   // Navigating elsewhere should not use the Instant render process.
   ui_test_utils::NavigateToURL(browser(), GURL(chrome::kChromeUIAboutURL));
   EXPECT_FALSE(instant_service->IsInstantProcess(
       active_tab->GetRenderProcessHost()->GetID()));
 }