Update: I removed TYPE_ANY to see if I could fool the builders into letting this pass without crashing in browser_tests. It makes the code uglier, but I want to debug and figure out what's going on.

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.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=30531
Review URL: http://codereview.chromium.org/330013

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

1 files changed, 25 insertions, 29 deletions

diff --git a/chrome/browser/browser_list.cc b/chrome/browser/browser_list.cc
index aba80b7..c5ef49c 100644
--- a/chrome/browser/browser_list.cc
+++ b/chrome/browser/browser_list.cc
@@ -79,6 +79,16 @@ class BrowserActivityObserver : public NotificationObserver {
 
 BrowserActivityObserver* activity_observer = NULL;
 
+// Returns true if the specified |browser| has a matching profile and type to
+// those specified. If |any_type| is true, only |profile| must be matched.
+bool BrowserMatchesProfileAndType(Browser* browser,
+                                  Profile* profile,
+                                  Browser::Type type,
+                                  bool any_type) {
+  return (any_type || browser->type() == type) &&
+      browser->profile() == profile;
+}
+
 }  // namespace
 
 BrowserList::list_type BrowserList::browsers_;
@@ -242,8 +252,10 @@ 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_NORMAL, true)) {
       return true;
+    }
   }
   return false;
 }
@@ -272,44 +284,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_NORMAL, true))
       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, false))
+      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_NORMAL, true))
+      return *browser;
   }
   return NULL;
 }
@@ -329,8 +325,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, false))
+      ++result;
   }
   return result;
 }
@@ -340,7 +336,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_NORMAL, true))
       result++;
   }
   return result;