Commit 04882168 authored by rouslan@chromium.org's avatar rouslan@chromium.org

Revert 222124 "Makes BrowserStatusMonitor remove observers in de..."

Failed unit tests on chromeos-asan:
  ChromeLauncherControllerTest.UnpinWithUninstall
  ChromeLauncherControllerTest.PendingInsertionOrder
  ChromeLauncherControllerTest.PersistLauncherItemPositions

http://build.chromium.org/p/chromium.memory/builders/Linux%20Chromium%20OS%20ASAN%20Tests%20%283%29/builds/9780

> Makes BrowserStatusMonitor remove observers in destructor
> 
> I'm seeing a crash where we have a dangling TabStripModelObserver,
> this is one possible place. At the same time I'm making the class only
> look for HOST_DESKTOP_TYPE_ASH browsers.
> 
> BUG=286162
> TEST=none
> R=skuhne@chromium.org
> 
> Review URL: https://chromiumcodereview.appspot.com/23455037

TBR=sky@chromium.org

Review URL: https://codereview.chromium.org/23538010

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@222138 0039d316-1c4b-4281-b951-d872f2087c98
parent 7a6d53b0
...@@ -272,7 +272,7 @@ void BrowserShortcutLauncherItemController::ActivateOrAdvanceToNextBrowser() { ...@@ -272,7 +272,7 @@ void BrowserShortcutLauncherItemController::ActivateOrAdvanceToNextBrowser() {
items.push_back(*it); items.push_back(*it);
} }
// If there are no suitable browsers we create a new one. // If there are no suitable browsers we create a new one.
if (items.empty()) { if (!items.size()) {
launcher_controller()->CreateNewWindow(); launcher_controller()->CreateNewWindow();
return; return;
} }
...@@ -288,8 +288,8 @@ void BrowserShortcutLauncherItemController::ActivateOrAdvanceToNextBrowser() { ...@@ -288,8 +288,8 @@ void BrowserShortcutLauncherItemController::ActivateOrAdvanceToNextBrowser() {
browser = items[0]; browser = items[0];
} else { } else {
// If there is more then one suitable browser, we advance to the next if // If there is more then one suitable browser, we advance to the next if
// |browser| is already active - or - check the last used browser if it can // |current_browser| is already active - or - check the last used browser
// be used. // if it can be used.
std::vector<Browser*>::iterator i = std::vector<Browser*>::iterator i =
std::find(items.begin(), items.end(), browser); std::find(items.begin(), items.end(), browser);
if (i != items.end()) { if (i != items.end()) {
...@@ -311,7 +311,6 @@ void BrowserShortcutLauncherItemController::ActivateOrAdvanceToNextBrowser() { ...@@ -311,7 +311,6 @@ void BrowserShortcutLauncherItemController::ActivateOrAdvanceToNextBrowser() {
bool BrowserShortcutLauncherItemController::IsBrowserRepresentedInBrowserList( bool BrowserShortcutLauncherItemController::IsBrowserRepresentedInBrowserList(
Browser* browser) { Browser* browser) {
return (browser && return (browser &&
browser->host_desktop_type() == chrome::HOST_DESKTOP_TYPE_ASH &&
(browser->is_type_tabbed() || (browser->is_type_tabbed() ||
!browser->is_app() || !browser->is_app() ||
!browser->is_type_popup() || !browser->is_type_popup() ||
......
...@@ -27,7 +27,10 @@ BrowserStatusMonitor::BrowserStatusMonitor( ...@@ -27,7 +27,10 @@ BrowserStatusMonitor::BrowserStatusMonitor(
observed_activation_clients_(this), observed_activation_clients_(this),
observed_root_windows_(this) { observed_root_windows_(this) {
DCHECK(launcher_controller_); DCHECK(launcher_controller_);
BrowserList::AddObserver(this); BrowserList* browser_list =
BrowserList::GetInstance(chrome::HOST_DESKTOP_TYPE_ASH);
browser_list->AddObserver(this);
// This check needs for win7_aura. Without this, all tests in // This check needs for win7_aura. Without this, all tests in
// ChromeLauncherController will fail in win7_aura. // ChromeLauncherController will fail in win7_aura.
...@@ -54,14 +57,10 @@ BrowserStatusMonitor::~BrowserStatusMonitor() { ...@@ -54,14 +57,10 @@ BrowserStatusMonitor::~BrowserStatusMonitor() {
if (ash::Shell::HasInstance()) if (ash::Shell::HasInstance())
ash::Shell::GetInstance()->GetScreen()->RemoveObserver(this); ash::Shell::GetInstance()->GetScreen()->RemoveObserver(this);
BrowserList::RemoveObserver(this);
BrowserList* browser_list = BrowserList* browser_list =
BrowserList::GetInstance(chrome::HOST_DESKTOP_TYPE_ASH); BrowserList::GetInstance(chrome::HOST_DESKTOP_TYPE_ASH);
for (BrowserList::const_iterator i = browser_list->begin();
i != browser_list->end(); ++i) { browser_list->RemoveObserver(this);
OnBrowserRemoved(*i);
}
} }
void BrowserStatusMonitor::OnWindowActivated(aura::Window* gained_active, void BrowserStatusMonitor::OnWindowActivated(aura::Window* gained_active,
...@@ -87,9 +86,6 @@ void BrowserStatusMonitor::OnWindowDestroyed(aura::Window* window) { ...@@ -87,9 +86,6 @@ void BrowserStatusMonitor::OnWindowDestroyed(aura::Window* window) {
} }
void BrowserStatusMonitor::OnBrowserAdded(Browser* browser) { void BrowserStatusMonitor::OnBrowserAdded(Browser* browser) {
if (browser->host_desktop_type() != chrome::HOST_DESKTOP_TYPE_ASH)
return;
browser->tab_strip_model()->AddObserver(this); browser->tab_strip_model()->AddObserver(this);
if (browser->is_type_popup() && browser->is_app()) { if (browser->is_type_popup() && browser->is_app()) {
...@@ -103,9 +99,6 @@ void BrowserStatusMonitor::OnBrowserAdded(Browser* browser) { ...@@ -103,9 +99,6 @@ void BrowserStatusMonitor::OnBrowserAdded(Browser* browser) {
} }
void BrowserStatusMonitor::OnBrowserRemoved(Browser* browser) { void BrowserStatusMonitor::OnBrowserRemoved(Browser* browser) {
if (browser->host_desktop_type() != chrome::HOST_DESKTOP_TYPE_ASH)
return;
browser->tab_strip_model()->RemoveObserver(this); browser->tab_strip_model()->RemoveObserver(this);
if (browser_to_app_id_map_.find(browser) != browser_to_app_id_map_.end()) { if (browser_to_app_id_map_.find(browser) != browser_to_app_id_map_.end()) {
...@@ -148,9 +141,6 @@ void BrowserStatusMonitor::ActiveTabChanged(content::WebContents* old_contents, ...@@ -148,9 +141,6 @@ void BrowserStatusMonitor::ActiveTabChanged(content::WebContents* old_contents,
if (old_contents) if (old_contents)
browser = chrome::FindBrowserWithWebContents(old_contents); browser = chrome::FindBrowserWithWebContents(old_contents);
if (browser && browser->host_desktop_type() != chrome::HOST_DESKTOP_TYPE_ASH)
return;
// Update immediately on a tab change. // Update immediately on a tab change.
if (browser && if (browser &&
(TabStripModel::kNoTab != (TabStripModel::kNoTab !=
...@@ -200,9 +190,6 @@ void BrowserStatusMonitor::UpdateAppAndBrowserState( ...@@ -200,9 +190,6 @@ void BrowserStatusMonitor::UpdateAppAndBrowserState(
ChromeLauncherController::APP_STATE_INACTIVE; ChromeLauncherController::APP_STATE_INACTIVE;
Browser* browser = chrome::FindBrowserWithWebContents(contents); Browser* browser = chrome::FindBrowserWithWebContents(contents);
DCHECK(browser);
if (browser->host_desktop_type() != chrome::HOST_DESKTOP_TYPE_ASH)
return;
if (browser->tab_strip_model()->GetActiveWebContents() == contents) { if (browser->tab_strip_model()->GetActiveWebContents() == contents) {
if (browser->window()->IsActive()) if (browser->window()->IsActive())
app_state = ChromeLauncherController::APP_STATE_WINDOW_ACTIVE; app_state = ChromeLauncherController::APP_STATE_WINDOW_ACTIVE;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment