Ensure History > Recent Tabs restore preserves window disposition.

components/sessions/core/tab_restore_service_helper.cc

 // Copyright 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/sessions/core/tab_restore_service_helper.h"
 
 #include <inttypes.h>
 #include <stddef.h>
 
 #include <algorithm>
@@ skipping 64 matching lines @@
 void TabRestoreServiceHelper::RemoveObserver(
     TabRestoreServiceObserver* observer) {
   observer_list_.RemoveObserver(observer);
 }
 
 void TabRestoreServiceHelper::CreateHistoricalTab(LiveTab* live_tab,
                                                   int index) {
   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 a window in other places. I
think we should continue that differentiation here as well. When you close a
tab, you are closing the tab. That the tab happens to be the last tab in a
window is secondary to closing the tab.

[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())
     return;
 
   AddEntry(std::move(local_tab), true, true);
 }
 
 void TabRestoreServiceHelper::BrowserClosing(LiveTabContext* context) {
   closing_contexts_.insert(context);
 
   auto window = base::MakeUnique<Window>();
   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>();
     PopulateTab(tab.get(), tab_index, context,
                 context->GetLiveTabAt(tab_index));
     if (!tab->navigations.empty()) {
       tab->browser_id = context->GetSessionID().id();
       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);
   }
 }
 
 void TabRestoreServiceHelper::BrowserClosed(LiveTabContext* context) {
   closing_contexts_.erase(context);
 }
 
@@ skipping 32 matching lines @@
 
 std::vector<LiveTab*> TabRestoreServiceHelper::RestoreEntryById(
     LiveTabContext* context,
     SessionID::id_type id,
     WindowOpenDisposition disposition) {
   Entries::iterator entry_iterator = GetEntryIteratorById(id);
   if (entry_iterator == entries_.end()) {
     // 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.
   bool restoring_tab_in_window = entry.id != id;
 
   // |context| will be NULL in cases where one isn't already available (eg,
   // when invoked on Mac OS X with no windows open). In this case, create a
   // new browser into which we restore the tabs.
   std::vector<LiveTab*> live_tabs;
   switch (entry.type) {
     case TabRestoreService::TAB: {
       auto& tab = static_cast<const Tab&>(entry);
       LiveTab* restored_tab = nullptr;
       context = RestoreTab(tab, context, disposition, &restored_tab);
       live_tabs.push_back(restored_tab);
       context->ShowBrowserWindow();
       break;
     }
     case TabRestoreService::WINDOW: {
       LiveTabContext* current_context = context;
       auto& window = static_cast<Window&>(entry);
 
       // When restoring a window, either the entire window can be restored, or a
       // 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(
               tab.navigations, context->GetTabCount(),
               tab.current_navigation_index, tab.extension_app_id,
               static_cast<int>(tab_i) == window.selected_tab_index, tab.pinned,
               tab.from_last_session, tab.platform_data.get(),
               tab.user_agent_override);
           if (restored_tab) {
             client_->OnTabRestored(
                 tab.navigations.at(tab.current_navigation_index).virtual_url());
             live_tabs.push_back(restored_tab);
           }
         }
         // All the window's tabs had the same former browser_id.
         if (auto browser_id = window.tabs[0]->browser_id) {
           UpdateTabBrowserIDs(browser_id, context->GetSessionID().id());
         }
       } 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;
           {
             const Tab& tab = **tab_i;
             if (tab.id != id)
               continue;
 
             restored_tab_browser_id = tab.browser_id;
             LiveTab* restored_tab = nullptr;
@@ skipping 203 matching lines @@
     const Tab& tab,
     LiveTabContext* context,
     WindowOpenDisposition disposition,
     LiveTab** live_tab) {
   LiveTab* restored_tab;
   if (disposition == WindowOpenDisposition::CURRENT_TAB && context) {
     restored_tab = context->ReplaceRestoredTab(
         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);
     }
 
     int tab_index = -1;
 
     // |context| will be NULL in cases where one isn't already available (eg,
     // when invoked on Mac OS X with no windows open). In this case, create a
     // new browser into which we restore the tabs.
     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());
     }
 
     // Place the tab at the end if the tab index is no longer valid or
     // we were passed a specific disposition.
     if (tab_index < 0 || tab_index > context->GetTabCount() ||
         disposition != WindowOpenDisposition::UNKNOWN) {
       tab_index = context->GetTabCount();
     }
@@ skipping 77 matching lines @@
         tab.browser_id = new_id;
     }
   }
 }
 
 base::Time TabRestoreServiceHelper::TimeNow() const {
   return time_factory_ ? time_factory_->TimeNow() : base::Time::Now();
 }
 
 }  // namespace sessions