Convert banned calls to use Navigate in ash app list.

The ash app list uses FindLastActiveWithProfile and FindOrCreateTabbedBrowser, which are banned. This change replaces them with calls to chrome::Navigate.

BUG=138632
TEST=Check the app list on ChromeOS can be used to show application options and links from the search box, with and without chrome browser windows open.


Review URL: https://chromiumcodereview.appspot.com/10825317

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@152018 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/ui/ash/app_list/extension_app_item.cc

@@ -14,7 +14,7 @@
 #include "chrome/browser/ui/ash/extension_utils.h"
 #include "chrome/browser/ui/ash/launcher/chrome_launcher_controller.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/browser_window.h"
 #include "chrome/common/extensions/extension.h"
@@ -168,16 +168,10 @@ void ExtensionAppItem::ShowExtensionOptions() {
   if (!extension)
     return;
 
-  // TODO(beng): use Navigate()!
-  Browser* browser = browser::FindLastActiveWithProfile(profile_);
-  if (!browser) {
-    browser = new Browser(Browser::CreateParams(profile_));
-    browser->window()->Show();
-  }
-
-  chrome::AddSelectedTabWithURL(browser, extension->options_url(),
+  chrome::NavigateParams params(profile_,
+                                extension->options_url(),
                                 content::PAGE_TRANSITION_LINK);
-  browser->window()->Activate();
+  chrome::Navigate(&params);
 }
 
 void ExtensionAppItem::StartExtensionUninstall() {

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

@@ -18,7 +18,7 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/ash/extension_utils.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/common/extensions/extension.h"
 #include "chrome/common/extensions/extension_constants.h"
@@ -246,27 +246,12 @@ void SearchBuilder::OpenResult(const app_list::SearchResult& result,
       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(profile_,
+                                  match.destination_url,
+                                  match.transition);
+    params.disposition = chrome::DispositionFromEventFlags(event_flags);
+    chrome::Navigate(&params);
   }
 }
 

chrome/browser/ui/browser_navigator.cc

@@ -74,7 +74,7 @@ bool AdjustNavigateParamsForURL(chrome::NavigateParams* params) {
     return true;
   }
 
-  Profile* profile = params->browser->profile();
+  Profile* profile = params->initiating_profile;
 
   if (profile->IsOffTheRecord() || params->disposition == OFF_THE_RECORD) {
     profile = profile->GetOriginalProfile();
@@ -102,19 +102,23 @@ Browser* GetBrowserForDisposition(chrome::NavigateParams* params) {
   // the target browser. This must happen first, before
   // GetBrowserForDisposition() has a chance to replace |params->browser| with
   // another one.
-  if (!params->source_contents)
+  if (!params->source_contents && params->browser)
     params->source_contents = chrome::GetActiveTabContents(params->browser);
 
-  Profile* profile = params->browser->profile();
+  Profile* profile = params->initiating_profile;
 
   switch (params->disposition) {
     case CURRENT_TAB:
-      return params->browser;
+      if (params->browser)
+        return params->browser;
+      // Find a compatible window and re-execute this command in it. Otherwise
+      // re-run with NEW_WINDOW.
+      return GetOrCreateBrowser(profile);
     case SINGLETON_TAB:
     case NEW_FOREGROUND_TAB:
     case NEW_BACKGROUND_TAB:
       // See if we can open the tab in the window this navigator is bound to.
-      if (WindowCanOpenTabs(params->browser))
+      if (params->browser && WindowCanOpenTabs(params->browser))
         return params->browser;
       // Find a compatible window and re-execute this command in it. Otherwise
       // re-run with NEW_WINDOW.
@@ -204,14 +208,11 @@ void NormalizeDisposition(chrome::NavigateParams* params) {
 }
 
 // Obtain the profile used by the code that originated the Navigate() request.
-// |source_browser| represents the Browser that was supplied in |params| before
-// it was modified.
-Profile* GetSourceProfile(chrome::NavigateParams* params,
-                          Browser* source_browser) {
+Profile* GetSourceProfile(chrome::NavigateParams* params) {
   if (params->source_contents)
     return params->source_contents->profile();
 
-  return source_browser->profile();
+  return params->initiating_profile;
 }
 
 void LoadURLInContents(WebContents* target_contents,
@@ -344,8 +345,8 @@ NavigateParams::NavigateParams(Browser* a_browser,
       user_gesture(true),
       path_behavior(RESPECT),
       ref_behavior(IGNORE_REF),
-      browser(a_browser) {
-}
+      browser(a_browser),
+      initiating_profile(NULL) {}
 
 NavigateParams::NavigateParams(Browser* a_browser,
                                TabContents* a_target_contents)
@@ -360,20 +361,42 @@ NavigateParams::NavigateParams(Browser* a_browser,
       user_gesture(true),
       path_behavior(RESPECT),
       ref_behavior(IGNORE_REF),
-      browser(a_browser) {
-}
+      browser(a_browser),
+      initiating_profile(NULL) {}
 
-NavigateParams::~NavigateParams() {
-}
+NavigateParams::NavigateParams(Profile* a_profile,
+                               const GURL& a_url,
+                               content::PageTransition a_transition)
+    : url(a_url),
+      target_contents(NULL),
+      source_contents(NULL),
+      disposition(NEW_FOREGROUND_TAB),
+      transition(a_transition),
+      is_renderer_initiated(false),
+      tabstrip_index(-1),
+      tabstrip_add_types(TabStripModel::ADD_ACTIVE),
+      window_action(SHOW_WINDOW),
+      user_gesture(true),
+      path_behavior(RESPECT),
+      ref_behavior(IGNORE_REF),
+      browser(NULL),
+      initiating_profile(a_profile) {}
+
+NavigateParams::~NavigateParams() {}
 
 void Navigate(NavigateParams* params) {
   Browser* source_browser = params->browser;
+  if (source_browser)
+    params->initiating_profile = source_browser->profile();
+  DCHECK(params->initiating_profile);
 
   if (!AdjustNavigateParamsForURL(params))
     return;
 
   // The browser window may want to adjust the disposition.
-  if (params->disposition == NEW_POPUP && source_browser->window()) {
+  if (params->disposition == NEW_POPUP &&
+      source_browser &&
+      source_browser->window()) {
     params->disposition =
         source_browser->window()->GetDispositionForPopupBounds(
             params->window_bounds);
@@ -384,6 +407,7 @@ void Navigate(NavigateParams* params) {
   if (params->source_contents &&
       params->source_contents->web_contents()->IsCrashed() &&
       params->disposition == CURRENT_TAB &&
+      params->browser &&
       params->browser->is_type_panel()) {
     params->disposition = NEW_FOREGROUND_TAB;
   }
@@ -395,7 +419,7 @@ void Navigate(NavigateParams* params) {
 
   // Navigate() must not return early after this point.
 
-  if (GetSourceProfile(params, source_browser) != params->browser->profile()) {
+  if (GetSourceProfile(params) != params->browser->profile()) {
     // A tab is being opened from a link from a different profile, we must reset
     // source information that may cause state to be shared.
     params->source_contents = NULL;

chrome/browser/ui/browser_navigator.h

@@ -15,6 +15,7 @@
 #include "webkit/glue/window_open_disposition.h"
 
 class Browser;
+class Profile;
 class TabContents;
 
 namespace chrome {
@@ -46,6 +47,9 @@ struct NavigateParams {
                  const GURL& a_url,
                  content::PageTransition a_transition);
   NavigateParams(Browser* browser, TabContents* a_target_contents);
+  NavigateParams(Profile* profile,
+                 const GURL& a_url,
+                 content::PageTransition a_transition);
   ~NavigateParams();
 
   // The URL/referrer to be loaded. Ignored if |target_contents| is non-NULL.
@@ -96,7 +100,8 @@ struct NavigateParams {
   // constructor.
   content::PageTransition transition;
 
-  // Whether this navigation was initiated by the renderer process.
+  // Whether this navigation was initiated by the renderer process. Default is
+  // false.
   bool is_renderer_initiated;
 
   // The index the caller would like the tab to be positioned at in the
@@ -166,7 +171,8 @@ struct NavigateParams {
 
   // [in]  Specifies a Browser object where the navigation could occur or the
   //       tab could be added. Navigate() is not obliged to use this Browser if
-  //       it is not compatible with the operation being performed.
+  //       it is not compatible with the operation being performed. This can be
+  //       NULL, in which case |initiating_profile| must be provided.
   // [out] Specifies the Browser object where the navigation occurred or the
   //       tab was added. Guaranteed non-NULL unless the disposition did not
   //       require a navigation, in which case this is set to NULL
@@ -177,6 +183,10 @@ struct NavigateParams {
   //       objects are deleted when the user closes a visible browser window).
   Browser* browser;
 
+  // The profile that is initiating the navigation. If there is a non-NULL
+  // browser passed in via |browser|, it's profile will be used instead.
+  Profile* initiating_profile;
+
   // Refers to a navigation that was parked in the browser in order to be
   // transferred to another RVH. Only used in case of a redirection of a request
   // to a different site that created a new RVH.

chrome/browser/ui/browser_navigator_browsertest.cc

@@ -1227,6 +1227,24 @@ IN_PROC_BROWSER_TEST_F(BrowserNavigatorTest,
             chrome::GetActiveWebContents(browser())->GetURL());
 }
 
+IN_PROC_BROWSER_TEST_F(BrowserNavigatorTest,
+                       NavigateWithoutBrowser) {
+  // First navigate using the profile of the existing browser window, and
+  // check that the window is reused.
+  chrome::NavigateParams params(browser()->profile(), GetGoogleURL(),
+                                content::PAGE_TRANSITION_LINK);
+  ui_test_utils::NavigateToURL(&params);
+  EXPECT_EQ(1u, BrowserList::size());
+
+  // Now navigate using the incognito profile and check that a new window
+  // is created.
+  chrome::NavigateParams params_incognito(
+      browser()->profile()->GetOffTheRecordProfile(),
+      GetGoogleURL(), content::PAGE_TRANSITION_LINK);
+  ui_test_utils::NavigateToURL(&params_incognito);
+  EXPECT_EQ(2u, BrowserList::size());
+}
+
 // This test makes sure any link in a crashed panel page navigates to a tabbed
 // window.
 class PanelBrowserNavigatorTest : public BrowserNavigatorTest {

ui/app_list/app_list_view.cc

@@ -191,7 +191,6 @@ void AppListView::QueryChanged(SearchBoxView* sender) {
 void AppListView::OpenResult(const SearchResult& result, int event_flags) {
   if (delegate_.get())
     delegate_->OpenSearchResult(result, event_flags);
-  Close();
 }
 
 }  // namespace app_list