Commit 2f7f86fc authored by Christian Dullweber's avatar Christian Dullweber Committed by Commit Bot

Fix crash due to invalid Window entry on Mac

A few users have invalid Window entries in TabRestoreServiceHelper when
deleting history. These entries are created when individual tabs are
restored from a closed window. This is only possible on Mac.
It is fixed by adjusting the selected_tab_index after a deletion.

Bug: 840508
Change-Id: I429d117df8cf06ee1613220112901590f129d529
Reviewed-on: https://chromium-review.googlesource.com/1049976Reviewed-by: default avatarScott Violet <sky@chromium.org>
Commit-Queue: Christian Dullweber <dullweber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558744}
parent 333819e9
...@@ -510,6 +510,10 @@ IN_PROC_BROWSER_TEST_F(TabRestoreTest, RestoreTabFromClosedWindowByID) { ...@@ -510,6 +510,10 @@ IN_PROC_BROWSER_TEST_F(TabRestoreTest, RestoreTabFromClosedWindowByID) {
static_cast<sessions::TabRestoreService::Window*>(entry); static_cast<sessions::TabRestoreService::Window*>(entry);
auto& tabs = entry_win->tabs; auto& tabs = entry_win->tabs;
EXPECT_EQ(3u, tabs.size()); EXPECT_EQ(3u, tabs.size());
EXPECT_EQ(url::kAboutBlankURL, tabs[0]->navigations.front().virtual_url());
EXPECT_EQ(url1_, tabs[1]->navigations.front().virtual_url());
EXPECT_EQ(url2_, tabs[2]->navigations.front().virtual_url());
EXPECT_EQ(2, entry_win->selected_tab_index);
// Find the Tab to restore. // Find the Tab to restore.
SessionID tab_id_to_restore = SessionID::InvalidValue(); SessionID tab_id_to_restore = SessionID::InvalidValue();
...@@ -541,6 +545,12 @@ IN_PROC_BROWSER_TEST_F(TabRestoreTest, RestoreTabFromClosedWindowByID) { ...@@ -541,6 +545,12 @@ IN_PROC_BROWSER_TEST_F(TabRestoreTest, RestoreTabFromClosedWindowByID) {
EXPECT_EQ(2, browser->tab_strip_model()->count()); EXPECT_EQ(2, browser->tab_strip_model()->count());
EXPECT_EQ(url1_, EXPECT_EQ(url1_,
browser->tab_strip_model()->GetActiveWebContents()->GetURL()); browser->tab_strip_model()->GetActiveWebContents()->GetURL());
// Check that the window entry was adjusted.
EXPECT_EQ(2u, tabs.size());
EXPECT_EQ(url::kAboutBlankURL, tabs[0]->navigations.front().virtual_url());
EXPECT_EQ(url2_, tabs[1]->navigations.front().virtual_url());
EXPECT_EQ(1, entry_win->selected_tab_index);
} }
// Tests that a duplicate history entry is not created when we restore a page // Tests that a duplicate history entry is not created when we restore a page
......
...@@ -319,11 +319,10 @@ std::vector<LiveTab*> TabRestoreServiceHelper::RestoreEntryById( ...@@ -319,11 +319,10 @@ std::vector<LiveTab*> TabRestoreServiceHelper::RestoreEntryById(
} else { } else {
// Restore a single tab from the window. Find the tab that matches the // Restore a single tab from the window. Find the tab that matches the
// ID in the window and restore it. // ID in the window and restore it.
for (auto tab_i = window.tabs.begin(); tab_i != window.tabs.end(); for (size_t tab_i = 0; tab_i < window.tabs.size(); tab_i++) {
++tab_i) {
SessionID::id_type restored_tab_browser_id; SessionID::id_type restored_tab_browser_id;
{ {
const Tab& tab = **tab_i; const Tab& tab = *window.tabs[tab_i];
if (tab.id != id) if (tab.id != id)
continue; continue;
...@@ -331,13 +330,20 @@ std::vector<LiveTab*> TabRestoreServiceHelper::RestoreEntryById( ...@@ -331,13 +330,20 @@ std::vector<LiveTab*> TabRestoreServiceHelper::RestoreEntryById(
LiveTab* restored_tab = nullptr; LiveTab* restored_tab = nullptr;
context = RestoreTab(tab, context, disposition, &restored_tab); context = RestoreTab(tab, context, disposition, &restored_tab);
live_tabs.push_back(restored_tab); live_tabs.push_back(restored_tab);
window.tabs.erase(tab_i); DCHECK(ValidateWindow(window));
window.tabs.erase(window.tabs.begin() + tab_i);
} }
// If restoring the tab leaves the window with nothing else, delete it // If restoring the tab leaves the window with nothing else, delete it
// as well. // as well.
if (window.tabs.empty()) { if (window.tabs.empty()) {
entries_.erase(entry_iterator); entries_.erase(entry_iterator);
} else { } else {
// Adjust |selected_tab index| to keep the window in a valid state.
if (static_cast<int>(tab_i) <= window.selected_tab_index) {
window.selected_tab_index =
std::max(0, window.selected_tab_index - 1);
}
DCHECK(ValidateWindow(window));
// Update the browser ID of the rest of the tabs in the window so if // Update the browser ID of the rest of the tabs in the window so if
// any one is restored, it goes into the same window as the tab // any one is restored, it goes into the same window as the tab
// being restored now. // being restored now.
......
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