Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(6781)

Unified Diff: chrome/browser/sessions/session_restore.cc

Issue 2714483003: cros: Fix restoring session windows after crash inconsistent minimized window counts (Closed)
Patch Set: improved comments Created 3 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | chrome/browser/sessions/session_restore_browsertest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome/browser/sessions/session_restore.cc
diff --git a/chrome/browser/sessions/session_restore.cc b/chrome/browser/sessions/session_restore.cc
index eeafc2d4fed5bd8ac73abeea509a04cf4f662b5f..b6d19f0a417d48e25c8f280c4daed5d0c21b317c 100644
--- a/chrome/browser/sessions/session_restore.cc
+++ b/chrome/browser/sessions/session_restore.cc
@@ -395,20 +395,45 @@ class SessionRestoreImpl : public content::NotificationObserver {
bool has_visible_browser = false;
for (const auto& window : *windows) {
sky 2017/03/07 22:41:33 This function is getting rather long and hard to r
if (window->show_state != ui::SHOW_STATE_MINIMIZED ||
- window->window_id.id() == active_window_id)
+ window->window_id.id() == active_window_id) {
has_visible_browser = true;
+ break;
+ }
+ }
+
+ // If |browser_| exists, is on-record, and tabbed, we need to firstly
+ // consider restoring tabs from active window to |browser_|. If active
+ // tabbed window doesn't exist, restore the first set of tabs instead.
+ bool active_window_on_existing_browser = false;
sky 2017/03/07 22:41:33 Renane to browser_is_active_window.
+ bool first_window_on_existing_browser = false;
sky 2017/03/07 22:41:32 Rename to restore_first_window_to_browser.
+ if (browser_ && browser_->is_type_tabbed() &&
+ !browser_->profile()->IsOffTheRecord()) {
+ for (auto i = windows->begin(); i != windows->end(); ++i) {
+ if ((*i)->type != sessions::SessionWindow::TYPE_TABBED)
+ continue;
+
+ if ((*i)->window_id.id() == active_window_id) {
+ active_window_on_existing_browser = true;
sky 2017/03/07 22:41:33 I think you should only do this logic if the brows
Qiang(Joe) Xu 2017/03/08 04:59:38 I checked the visibility of |browser_|'s native wi
+ break;
+ }
+ }
+
+ if (!active_window_on_existing_browser &&
+ (*windows->begin())->type == sessions::SessionWindow::TYPE_TABBED) {
+ first_window_on_existing_browser = true;
+ }
}
for (auto i = windows->begin(); i != windows->end(); ++i) {
Browser* browser = nullptr;
if (!has_tabbed_browser &&
- (*i)->type == sessions::SessionWindow::TYPE_TABBED)
+ (*i)->type == sessions::SessionWindow::TYPE_TABBED) {
has_tabbed_browser = true;
- if (i == windows->begin() &&
- (*i)->type == sessions::SessionWindow::TYPE_TABBED && browser_ &&
- browser_->is_type_tabbed() &&
- !browser_->profile()->IsOffTheRecord()) {
- // The first set of tabs is added to the existing browser.
+ }
+
+ if ((active_window_on_existing_browser &&
+ (*i)->window_id.id() == active_window_id) ||
+ (first_window_on_existing_browser && i == windows->begin())) {
sky 2017/03/07 22:41:33 That you have to check windows->begin() here is co
browser = browser_;
} else {
#if defined(OS_CHROMEOS)
@@ -656,7 +681,8 @@ class SessionRestoreImpl : public content::NotificationObserver {
// TODO(jcampan): http://crbug.com/8123 we should not need to set the
// initial focus explicitly.
- browser->tab_strip_model()->GetActiveWebContents()->SetInitialFocus();
+ if (browser->window()->IsActive())
+ browser->tab_strip_model()->GetActiveWebContents()->SetInitialFocus();
}
// Appends the urls in |urls| to |browser|.
« no previous file with comments | « no previous file | chrome/browser/sessions/session_restore_browsertest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698