Convert banned calls to use Navigate in ash app list.

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

 // Copyright (c) 2012 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/ui/views/ash/app_list/search_builder.h"
 
 #include <string>
 
 #include "base/command_line.h"
 #include "chrome/browser/autocomplete/autocomplete_controller.h"
 #include "chrome/browser/autocomplete/autocomplete_input.h"
 #include "chrome/browser/autocomplete/autocomplete_match.h"
 #include "chrome/browser/autocomplete/autocomplete_result.h"
 #include "chrome/browser/autocomplete/extension_app_provider.h"
 #include "chrome/browser/event_disposition.h"
 #include "chrome/browser/extensions/extension_service.h"
 #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"
 #include "chrome/common/extensions/extension_icon_set.h"
 #include "chrome/common/url_constants.h"
 #include "content/public/browser/web_contents.h"
 #include "grit/generated_resources.h"
 #include "grit/theme_resources.h"
 #include "ui/app_list/app_list_switches.h"
 #include "ui/app_list/search_box_model.h"
@@ skipping 200 matching lines @@
   const SearchBuilderResult* builder_result =
       static_cast<const SearchBuilderResult*>(&result);
   const AutocompleteMatch& match = builder_result->match();
 
   if (match.type == AutocompleteMatch::EXTENSION_APP) {
     const extensions::Extension* extension =
         GetExtensionByURL(profile_, match.destination_url);
     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);
   }
 }
 
 void SearchBuilder::PopulateFromACResult(const AutocompleteResult& ac_result) {
   results_->DeleteAll();
   for (ACMatches::const_iterator it = ac_result.begin();
        it != ac_result.end();
        ++it) {
     results_->Add(new SearchBuilderResult(profile_, *it));
   }
 }
 
 void SearchBuilder::OnResultChanged(bool default_match_changed) {
   // TODO(xiyuan): Handle default match properly.
   const AutocompleteResult& ac_result = controller_->result();
   PopulateFromACResult(ac_result);
 }