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

chrome/browser/instant/instant_extended_browsertest.cc

Index: chrome/browser/instant/instant_extended_browsertest.cc
diff --git a/chrome/browser/instant/instant_extended_browsertest.cc b/chrome/browser/instant/instant_extended_browsertest.cc
index 803122a7f61512109e2f7d42ec0eb978108d13f0..a55ead4e9795cc390cc60b17efe33240f4857550 100644
--- a/chrome/browser/instant/instant_extended_browsertest.cc
+++ b/chrome/browser/instant/instant_extended_browsertest.cc
@@ -18,6 +18,9 @@
 #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 {
@@ -73,7 +76,8 @@ IN_PROC_BROWSER_TEST_F(InstantExtendedTest, OmniboxFocusLoadsInstant) {
 
   // 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());
@@ -126,6 +130,53 @@ IN_PROC_BROWSER_TEST_F(InstantExtendedTest, InputShowsOverlay) {
   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/
 //