Convert banned calls to use Navigate in ash app list.

chrome/browser/ui/views/ash/app_list/search_builder.cc

Index: chrome/browser/ui/views/ash/app_list/search_builder.cc
diff --git a/chrome/browser/ui/views/ash/app_list/search_builder.cc b/chrome/browser/ui/views/ash/app_list/search_builder.cc
index ce0374f90e8e9455a671e50c8662c51400e6e041..435e2389685cb89652bddf36090816b2b21d58aa 100644
--- a/chrome/browser/ui/views/ash/app_list/search_builder.cc
+++ b/chrome/browser/ui/views/ash/app_list/search_builder.cc
@@ -17,7 +17,7 @@
 #include "chrome/browser/extensions/image_loading_tracker.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
-#include "chrome/browser/ui/browser_finder.h"
+#include "chrome/browser/ui/browser_navigator.h"
 #include "chrome/browser/ui/browser_tabstrip.h"
 #include "chrome/browser/ui/views/ash/extension_utils.h"
 #include "chrome/common/extensions/extension.h"
@@ -238,27 +238,12 @@ void SearchBuilder::OpenResult(const app_list::SearchResult& result,
     if (extension)
       extension_utils::OpenExtension(profile_, extension, event_flags);
   } else {
-    WindowOpenDisposition disposition =
-        chrome::DispositionFromEventFlags(event_flags);
-    Browser* browser = browser::FindOrCreateTabbedBrowser(profile_);
-
-    if (disposition == CURRENT_TAB) {
-      // If current tab is not NTP, change disposition to NEW_FOREGROUND_TAB.
-      const GURL& url = chrome::GetActiveWebContents(browser) ?
-          chrome::GetActiveWebContents(browser)->GetURL() : GURL();
-      if (!url.SchemeIs(chrome::kChromeUIScheme) ||
-          url.host() != chrome::kChromeUINewTabHost) {
-        disposition = NEW_FOREGROUND_TAB;
-      }
-    }
-
     // TODO(xiyuan): What should we do for alternate url case?
-    browser->OpenURL(
-        content::OpenURLParams(match.destination_url,
-                               content::Referrer(),
-                               disposition,
-                               match.transition,
-                               false));
+    chrome::NavigateParams params(NULL,

[jennb, 2012/08/14 00:33:01]

Does it work to pass a NULL browser here? One of the first things
chrome::Navigate() does is deref params->browser->profile().

[benwells, 2012/08/14 02:46:36]

This code from chrome::Navigate() makes it safe:
  params->browser = GetBrowserForDisposition(params);

  if (!params->browser)
    return;

Note that there is a block above which might try dereferencing
params->browser->is_type_panel, but only if params->source_contents is not NULL.

Having said that, I was getting a crash when using this, it is crashing in the
open gl layer where it tries to delete a context. I am assuming this crash is to
do with building / running chromeos directly on Linux.

+                                  match.destination_url,
+                                  match.transition);
+    params.disposition = chrome::DispositionFromEventFlags(event_flags);

[xiyuan, 2012/08/13 16:20:55]

Think we still need the special case for disposition so that we open the link in
a new tab instead of replacing current tab. See http://crbug.com/132326

[xiyuan, 2012/08/13 17:49:00]

One simple work around might be like this:

params.disposition = event_flags == ui::EF_NONE ?
    NEW_FOREGROUND_TAB :
    chrome::DispositionFromEventFlags(event_flags);

This should cover 99% of the case and not depending on banned calls.

[benwells, 2012/08/14 08:49:03]

The navigate function will change disposition from CURRENT_TAB to
NEW_FOREGROUND_TAB in NormalizeDisposition if params.source_contents is NULL
(which it is in our case).

However the behavior is a bit different now, as if the current tab is the NTP it
won't be used. Is that OK? It might make sense to update the Navigate call to
handle this in NormalizeDisposition, but that scares me a little.

[xiyuan, 2012/08/14 16:18:17]

Okay. Good to know that NormalizeDisposition will do that.

The behavior change should be okay. As long as we don't override the current
tab, it should cover most use cases. Let's see if tester/UX/user complains about
it.

+    chrome::Navigate(&params);
   }
 }