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

Unified Diff: components/sessions/core/tab_restore_service_helper.cc

Issue 2868983003: Ensure History > Recent Tabs restore preserves window disposition. (Closed)
Patch Set: Small cleanup. Created 3 years, 7 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
Index: components/sessions/core/tab_restore_service_helper.cc
diff --git a/components/sessions/core/tab_restore_service_helper.cc b/components/sessions/core/tab_restore_service_helper.cc
index e792dd6b67f9fd726f6b013b37e7130b39c4343c..5475d6f59d7d451f5a26744b96b5f8eb71bf973e 100644
--- a/components/sessions/core/tab_restore_service_helper.cc
+++ b/components/sessions/core/tab_restore_service_helper.cc
@@ -82,10 +82,22 @@ void TabRestoreServiceHelper::CreateHistoricalTab(LiveTab* live_tab,
if (restoring_)
return;
+ // If an entire window is being closed than all of the tabs have already
+ // been persisted via "BrowserClosing". Ignore the subsequent tab closing
+ // notifications.
LiveTabContext* context = client_->FindLiveTabContextForTab(live_tab);
if (closing_contexts_.find(context) != closing_contexts_.end())
return;
+ // If this is the last tab being closed in a window it is equivalent to a
sky 2017/05/08 22:51:36 We differentiate between closing a tab and closing
chrisha 2017/05/09 15:44:54 Ditto with moving this discussion to the bug.
+ // window close action, thus save a "window" entry. This ensures that a
+ // "ctrl-shift-t" or "shift-click" will restore the tab to a window with the
+ // original display parameters.
+ if (context->GetTabCount() == 1) {
+ BrowserClosing(context);
+ return;
+ }
+
auto local_tab = base::MakeUnique<Tab>();
PopulateTab(local_tab.get(), index, context, live_tab);
if (local_tab->navigations.empty())
@@ -101,6 +113,9 @@ void TabRestoreServiceHelper::BrowserClosing(LiveTabContext* context) {
window->selected_tab_index = context->GetSelectedIndex();
window->timestamp = TimeNow();
window->app_name = context->GetAppName();
+ window->bounds = context->GetBounds();
+ window->show_state = context->GetShowState();
+ window->workspace = context->GetWorkspace();
for (int tab_index = 0; tab_index < context->GetTabCount(); ++tab_index) {
auto tab = base::MakeUnique<Tab>();
@@ -111,11 +126,8 @@ void TabRestoreServiceHelper::BrowserClosing(LiveTabContext* context) {
window->tabs.push_back(std::move(tab));
}
}
- if (window->tabs.size() == 1 && window->app_name.empty()) {
- // Short-circuit creating a Window if only 1 tab was present. This fixes
- // http://crbug.com/56744.
- AddEntry(std::move(window->tabs[0]), true, true);
- } else if (!window->tabs.empty()) {
+
+ if (!window->tabs.empty()) {
window->selected_tab_index = std::min(
static_cast<int>(window->tabs.size() - 1), window->selected_tab_index);
AddEntry(std::move(window), true, true);
@@ -168,11 +180,27 @@ std::vector<LiveTab*> TabRestoreServiceHelper::RestoreEntryById(
// Don't hoark here, we allow an invalid id.
return std::vector<LiveTab*>();
}
+ auto& entry = **entry_iterator;
+
+ // If a window containing a single tab is being restored with "tab"
+ // disposition then treat it as restoring that tab rather than as restoring
+ // the entire window. This ensures that clicking or ctrl-clicking on 1-tab
+ // windows (which are visually presented as tabs in the "History > Recent
+ // Tabs" menu) are opened in the current browser window, like other tab items.
+ //
+ // However, shift-clicking or Ctrl-Shift-T activating these items will ensure
+ // that they are opened as a separate window with restored display parameters.
+ if (entry.id == id && entry.type == TabRestoreService::WINDOW &&
+ (disposition == WindowOpenDisposition::NEW_FOREGROUND_TAB ||
+ disposition == WindowOpenDisposition::NEW_BACKGROUND_TAB)) {
+ auto& window = static_cast<Window&>(entry);
+ if (window.tabs.size() == 1)
+ return RestoreEntryById(context, window.tabs[0]->id, disposition);
+ }
if (observer_)
observer_->OnRestoreEntryById(id, entry_iterator);
restoring_ = true;
- auto& entry = **entry_iterator;
// If the entry's ID does not match the ID that is being restored, then the
// entry is a window from which a single tab will be restored.
@@ -199,7 +227,9 @@ std::vector<LiveTab*> TabRestoreServiceHelper::RestoreEntryById(
// single tab within it. If the entry's ID matches the one to restore,
// then the entire window will be restored.
if (!restoring_tab_in_window) {
- context = client_->CreateLiveTabContext(window.app_name);
+ context =
+ client_->CreateLiveTabContext(window.app_name, window.bounds,
+ window.show_state, window.workspace);
for (size_t tab_i = 0; tab_i < window.tabs.size(); ++tab_i) {
const Tab& tab = *window.tabs[tab_i];
LiveTab* restored_tab = context->AddRestoredTab(
@@ -220,8 +250,7 @@ std::vector<LiveTab*> TabRestoreServiceHelper::RestoreEntryById(
}
} else {
// 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();
++tab_i) {
SessionID::id_type restored_tab_browser_id;
@@ -445,7 +474,7 @@ LiveTabContext* TabRestoreServiceHelper::RestoreTab(
tab.navigations, tab.current_navigation_index, tab.from_last_session,
tab.extension_app_id, tab.platform_data.get(), tab.user_agent_override);
} else {
- // We only respsect the tab's original browser if there's no disposition.
+ // We only respect the tab's original browser if there's no disposition.
if (disposition == WindowOpenDisposition::UNKNOWN && tab.browser_id) {
context = client_->FindLiveTabContextWithID(tab.browser_id);
}
@@ -458,7 +487,8 @@ LiveTabContext* TabRestoreServiceHelper::RestoreTab(
if (context && disposition != WindowOpenDisposition::NEW_WINDOW) {
tab_index = tab.tabstrip_index;
} else {
- context = client_->CreateLiveTabContext(std::string());
+ context = client_->CreateLiveTabContext(std::string(), gfx::Rect(),
+ ui::SHOW_STATE_NORMAL, "");
if (tab.browser_id)
UpdateTabBrowserIDs(tab.browser_id, context->GetSessionID().id());
}

Powered by Google App Engine
This is Rietveld 408576698