Rework the way the FindBrowserWithProfile/Type methods work.

We now always walk the last active list backwards rather than consulting the last active then walking the registered browser list forwards.

This ensures that when the last active browser is a popup or app frame the last active TYPE_NORMAL browser is located when opening a new tab.

http://crbug.com/17498
TEST=Open an app frame. Open a browser window (Ctrl+N) and load a page. Minimize it. Open another browser window and minimize it. Activate the app frame. Press Ctrl+T. The second browser window should be restored and have a new tab added to it rather than the first.

Review URL: http://codereview.chromium.org/330013

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

4 files changed, 39 insertions, 40 deletions

diff --git a/chrome/browser/browser.cc b/chrome/browser/browser.cc
index 83f697f..f0ad701 100644
--- a/chrome/browser/browser.cc
+++ b/chrome/browser/browser.cc
@@ -1589,7 +1589,8 @@ void Browser::DuplicateContentsAt(int index) {
   } else {
     Browser* browser = NULL;
     if (type_ & TYPE_APP) {
-      browser = Browser::CreateForApp(app_name_, profile_, type_ & TYPE_POPUP);
+      browser = Browser::CreateForApp(app_name_, profile_,
+                                      !!(type_ & TYPE_POPUP));
     } else if (type_ == TYPE_POPUP) {
       browser = Browser::CreateForPopup(profile_);
     }
@@ -1947,7 +1948,7 @@ void Browser::DetachContents(TabContents* source) {
 
 bool Browser::IsPopup(TabContents* source) {
   // A non-tabbed BROWSER is an unconstrained popup.
-  return (type() & TYPE_POPUP);
+  return !!(type() & TYPE_POPUP);
 }
 
 void Browser::ToolbarSizeChanged(TabContents* source, bool is_animating) {
diff --git a/chrome/browser/browser.h b/chrome/browser/browser.h
index c3a2552..2781abf 100644
--- a/chrome/browser/browser.h
+++ b/chrome/browser/browser.h
@@ -48,10 +48,11 @@ class Browser : public TabStripModelDelegate,
   // TODO(sky): move into a common place that is referenced by both ui_tests
   // and chrome.
   enum Type {
-    TYPE_NORMAL = 0,
-    TYPE_POPUP = 1,
-    TYPE_APP = 2,
+    TYPE_NORMAL = 1,
+    TYPE_POPUP = 2,
+    TYPE_APP = 4,
     TYPE_APP_POPUP = TYPE_APP | TYPE_POPUP,
+    TYPE_ANY = TYPE_NORMAL | TYPE_POPUP | TYPE_APP
   };
 
   // Possible elements of the Browser window.
diff --git a/chrome/browser/browser_list.cc b/chrome/browser/browser_list.cc
index aba80b7..c8e3646 100644
--- a/chrome/browser/browser_list.cc
+++ b/chrome/browser/browser_list.cc
@@ -79,6 +79,19 @@ class BrowserActivityObserver : public NotificationObserver {
 
 BrowserActivityObserver* activity_observer = NULL;
 
+// Returns true if the specified |browser| has a matching profile and type to
+// those specified. |type| can also be TYPE_ANY, which means only |profile|
+// must be matched.
+bool BrowserMatchesProfileAndType(Browser* browser,
+                                  Profile* profile,
+                                  Browser::Type type) {
+  if (type == Browser::TYPE_ANY || browser->type() == type) {
+    if (browser->profile() == profile)
+      return true;
+  }
+  return false;
+}
+
 }  // namespace
 
 BrowserList::list_type BrowserList::browsers_;
@@ -242,7 +255,7 @@ void BrowserList::WindowsSessionEnding() {
 bool BrowserList::HasBrowserWithProfile(Profile* profile) {
   BrowserList::const_iterator iter;
   for (size_t i = 0; i < browsers_.size(); ++i) {
-    if (browsers_[i]->profile() == profile)
+    if (BrowserMatchesProfileAndType(browsers_[i], profile, Browser::TYPE_ANY))
       return true;
   }
   return false;
@@ -272,44 +285,28 @@ Browser* BrowserList::GetLastActive() {
 Browser* BrowserList::GetLastActiveWithProfile(Profile* p) {
   list_type::reverse_iterator browser = last_active_browsers_.rbegin();
   for (; browser != last_active_browsers_.rend(); ++browser) {
-    if ((*browser)->profile() == p) {
+    if (BrowserMatchesProfileAndType(*browser, p, Browser::TYPE_ANY))
       return *browser;
-    }
   }
-
   return NULL;
 }
 
 // static
 Browser* BrowserList::FindBrowserWithType(Profile* p, Browser::Type t) {
-  Browser* last_active = GetLastActive();
-  if (last_active && last_active->profile() == p && last_active->type() == t)
-    return last_active;
-
-  BrowserList::const_iterator i;
-  for (i = BrowserList::begin(); i != BrowserList::end(); ++i) {
-    if (*i == last_active)
-      continue;
-
-    if ((*i)->profile() == p && (*i)->type() == t)
-      return *i;
+  list_type::reverse_iterator browser = last_active_browsers_.rbegin();
+  for (; browser != last_active_browsers_.rend(); ++browser) {
+    if (BrowserMatchesProfileAndType(*browser, p, t))
+      return *browser;
   }
   return NULL;
 }
 
 // static
 Browser* BrowserList::FindBrowserWithProfile(Profile* p) {
-  Browser* last_active = GetLastActive();
-  if (last_active && last_active->profile() == p)
-    return last_active;
-
-  BrowserList::const_iterator i;
-  for (i = BrowserList::begin(); i != BrowserList::end(); ++i) {
-    if (*i == last_active)
-      continue;
-
-    if ((*i)->profile() == p)
-      return *i;
+  list_type::reverse_iterator browser = last_active_browsers_.rbegin();
+  for (; browser != last_active_browsers_.rend(); ++browser) {
+    if (BrowserMatchesProfileAndType(*browser, p, Browser::TYPE_ANY))
+      return *browser;
   }
   return NULL;
 }
@@ -329,8 +326,8 @@ size_t BrowserList::GetBrowserCountForType(Profile* p, Browser::Type type) {
   BrowserList::const_iterator i;
   size_t result = 0;
   for (i = BrowserList::begin(); i != BrowserList::end(); ++i) {
-    if ((*i)->profile() == p && (*i)->type() == type)
-      result++;
+    if (BrowserMatchesProfileAndType(*i, p, type))
+      ++result;
   }
   return result;
 }
@@ -340,7 +337,7 @@ size_t BrowserList::GetBrowserCount(Profile* p) {
   BrowserList::const_iterator i;
   size_t result = 0;
   for (i = BrowserList::begin(); i != BrowserList::end(); ++i) {
-    if ((*i)->profile() == p)
+    if (BrowserMatchesProfileAndType(*i, p, Browser::TYPE_ANY))
       result++;
   }
   return result;
diff --git a/chrome/browser/browser_list.h b/chrome/browser/browser_list.h
index b510b8c..45a0246 100644
--- a/chrome/browser/browser_list.h
+++ b/chrome/browser/browser_list.h
@@ -58,14 +58,14 @@ class BrowserList {
   // open browser owned by |profile| is returned. If none exist, returns NULL.
   static Browser* GetLastActiveWithProfile(Profile *profile);
 
-  // Find an existing browser window with the provided type. If the last active
-  // has the right type, it is returned. Otherwise, the next available browser
-  // is returned. Returns NULL if no such browser currently exists.
+  // Find an existing browser window with the provided type. Searches in the
+  // order of last activation. Only browsers that have been active can be
+  // returned. Returns NULL if no such browser currently exists.
   static Browser* FindBrowserWithType(Profile* p, Browser::Type t);
 
-  // Find an existing browser window with the provided profile. If the last
-  // active has the right profile, it is returned.  Returns NULL if no such
-  // browser currently exists.
+  // Find an existing browser window with the provided profile. Searches in the
+  // order of last activation. Only browsers that have been active can be
+  // returned. Returns NULL if no such browser currently exists.
   static Browser* FindBrowserWithProfile(Profile* p);
 
   // Find an existing browser with the provided ID. Returns NULL if no such