There's a few other related subtle issues:

- Closing a window containing a single tab creates a "tab" entry in the UI list, rather than a window entry.
- Restoring this with Ctrl-Shift-T will restore the tab in the currently open window. (Broken)
- Click restoring will do the same. (Works)
- Shift-click restoring will open to a new window, but without the previous window position and size. (Broken)

This is due to special case handling put into tab_restore_service.cc as part of  issue 56744 .

> We shouldn't need the special case here. Whether you get a tab vs a window
> should depend upon how you closed the window. If you close the window, then you
> should get a window entry, if you close the last tab in a window then I would
> expect a tab entry.

> 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.

I honestly don't think that's a distinction a typical user would make, but I
don't have any data to back that up. And we do already make that distinction in the current code, simply in the other direction (closing a single-tab window is treated like closing the last tab in a window). What currently happens is the following:

- Closing the last tab in a window OR closing a window containing a single 
 tab
  both result in the creation of "Tab" entry, and favicon/title entry in the
  "Recent Tabs" menu.
- Ctrl-Shift-T (open with UNKNOWN disposition) results in the tab opening
  in the current window, visible and with focus.
- Click (open with NEW_FOREGROUND_TAB disposition) results in the tab opening
  in the current window, visible and with focus.
- Shift-click (open with SINGLETON_TAB disposition) results in the tab opening
  in a new window with the same parameters as the current window.
- Ctrl-click (open with NEW_BACKGROUND_TAB disposition) results in the tab
  opening in the current window, without focus.

My feeling has always been that Ctrl-Shift-T is effectively an "undo last operation".
In which case the principle of least surprise dictates that it should restore the
tab and the window, regardless of whether I closed the tab or the window. What I'm
proposing is the following:

- Closing the last tab in a window OR closing a window containing a single tab
  both result in the creation of "Window" entry, but with a favicon/title entry
  in the "Recent Tabs" menu.
- Ctrl-Shift-T (open with UNKNOWN disposition) results in the tab opening
  in a new window with the same parameters as its previous window.
  (DIFFERENT FROM CURRENT BEHAVIOUR)
- Click (open with NEW_FOREGROUND_TAB disposition) results in the tab opening
  in the current window, visible and with focus.
  (SAME AS CURRENT BEHAVIOUR)
- Shift-click (open with SINGLETON_TAB disposition) results in the tab opening
  in a new window with the same parameters as its previous window.
  (DIFFERENT FROM CURRENT BEHAVIOUR)
- Ctrl-click (open with NEW_BACKGROUND_TAB disposition) results in the tab
  opening in the current window, without focus.
  (SAME AS CURRENT BEHAVIOUR)

Fully distinguishing between "tab closed" and "window closed" would look like the
following:

- Closing the last tab in a window results in the creation of a "Tab" entry,
  with a favicon/title entry in the "Recent Tabs" menu.
- Ctrl-Shift-T (open with UNKNOWN disposition) results in the tab opening
  in the current window, visible and with focus.
  (CURRENT BEHAVIOUR, VIOLATES PRINCIPLE OF LEAST SURPRISE IMO)
- Click (open with NEW_FOREGROUND_TAB disposition) results in the tab opening
  in the current window, visible and with focus.
  (CURRENT BEHAVIOUR, APPROPRIATE IMO)
- Shift-click (open with SINGLETON_TAB disposition) results in the tab opening
  in a new window with the same parameters as the current window.
  (CURRENT BEHAVIOUR, WOULD BE BETTER TO RESTORE THE PREVIOUS WINDOW
   PARAMETERS IMO)
- Ctrl-click (open with NEW_BACKGROUND_TAB disposition) results in the tab
  opening in the current window, without focus.
  (CURRENT BEHAVIOUR, APPROPRIATE IMO)

- Closing a window containing a single tab result in the creation of "Window"
  entry, and creates a "1 Tab" entry in the "Recent Tabs" menu.
- Ctrl-Shift-T (open with UNKNOWN disposition) results in the tab opening
  in a new window with the same parameters as its previous window.
  (DIFFERENT FROM CURRENT BEHAVIOUR, CORRECT IMO)
- Click (open with NEW_FOREGROUND_TAB disposition) results in the tab opening
  in a new window with the same parameters as its previous window.
  (DIFFERENT FROM CURRENT BEHAVIOUR, INCORRECT IMO)
- Shift-click (open with SINGLETON_TAB disposition) results in the tab opening
  in a new window with the same parameters as its previous window.
  (DIFFERENT FROM CURRENT BEHAVIOUR, CORRECT IMO)
- Ctrl-click (open with NEW_BACKGROUND_TAB disposition) results in the tab
  opening in a new window with the same parameters as its previous window.
  (DIFFERENT FROM CURRENT BEHAVIOUR, INCORRECT IMO)

I could also see an argument for the following:

- Closing the last tab in a window OR closing a window containing a single tab
  both result in the creation of "Window" entry, but with a favicon/title entry
  in the "Recent Tabs" menu.
- Ctrl-Shift-T (open with UNKNOWN disposition) results in the tab opening
  in a new window with the same parameters as its previous window.
  (DIFFERENT FROM CURRENT BEHAVIOUR)
- Click (open with NEW_FOREGROUND_TAB disposition) results in the tab opening
  in the current window, visible and with focus.
  (SAME AS CURRENT BEHAVIOUR)
- Shift-click (open with SINGLETON_TAB disposition) results in the tab opening
  in a new window with the same parameters as the current window.
  (SAME AS CURRENT BEHAVIOUR)
- Ctrl-click (open with NEW_BACKGROUND_TAB disposition) results in the tab
  opening in the current window, without focus.
  (SAME AS CURRENT BEHAVIOUR)

This minor change would ensure that a Shift-click of "tab" entries and "single tab window" entries result in the same behaviour, rather than slightly different behaviour as would be the case with my earlier proposed logic.

In both cases I'm mainly trying to ensure that "Ctrl-Shift-T" remains as a kind of "undo" operation.

I think what is created should mirror the action that was taken. By that I mean if the user closes the window, they get a Window entry. If the user closes the Tab (that just so happens to be the last tab) they get a tab entry. This is how we originally had it, but as you mentioned that was changed in 56744. Perhaps a better fix for that would be to still show a window entry, but some how include the title.

When I create a new window, navigate the page and close the window either by clicking the close button or control-w, and then use control-shift-t to restore I get a new window. You said you don't see this. I'm using windows with canary. The reason a new browser is created is because TabRestoreServiceHelper::RestoreTab() ends up in client_->FindLiveTabContextWithID(tab.browser_id) , which returns null (line 450), then on line 461 client_->CreateLiveTabContext() is called, which creates a new browser. I don't see any ifdefs in this code, so I'm surprised you would be seeing otherwise. When you step through in a debugger how does the restored tab end up getting added to an existing browser?

I would be fine with showing a window entry that also indicates the title/favicon somehow. I think it would be good to get an opinion here from a UX person. I feel the distinction between a "1-tab window" and "last tab in a window" is somewhat artificial and potentially confusing.

Regarding Ctrl-Shift-T, I just double-checked on a stock canary build, and you are right. I must have already had some modifying code in place when I was running my tests. The code path around this (restoring a single tab with disposition UNKNOWN, belonging to a window that no longer exists) was also added as part of the 56744 changes. The question then becomes whether or not the previous window position/size should be maintained, or whether it should simply be offset from the current window (current behaviour). If we want the former then we'd have to either create a "Window" entry in this case, or augment "Tab" entries to also store additional (optional) data about their window position/size.

I'll one up and say that currently the menu does not do a good job of portraying what is going to happen when you click on an entry. This is especially true if you restore things out of order. For example, consider a window with two tabs, A and B. If you close A, then B the menu shows two entries, one for B and one for A. They both look exactly the same (by that I mean they look like closed tabs), but clicking on B results in a new window with the restored tab, where as clicking on A (before clicking on B) results in the restored tab being added to the current window.

I feel like a better fix for 56744 would have been to change how the menu shows windows with one tab, not to change what is created internally.

I just tried your example on canary:

Right now, closing A then B, you see entry B first in the list, followed by entry A. Both are "tab" entries. Clicking on either of them creates a new tab in the current window.

There is, however, a large inconsistency in how "clicks" act versus Ctrl-Shift-T. We label the first entry with the keyboard shortcut, but entirely different things happen when you click vs when you use the shortcut. Clicking creates a tab in the current window; using the shortcut creates a new window with the tab in it.

Overall agreed that we don't provide a clear visual indication of what the effect of any of these actions will be.

In general my position is that Ctrl-Shift-T means "undo last close tab action" and if the last tab was alone in a window, it should restore into the same window size/position. If users reason about this, they probably think closing the last tab causes closing the window - so it's desirable to use "closing the window" behaviors for this scenario. I don't believe many people reason about method of closing a single-tab window.

I agree that click behavior in the recently closed tabs menu is unpredicatable today, and it would be worth revisiting this UI as resources allow. I'm cc'ing designers thinking about tab management as FYI but don't expect this to get addressed right away. Can we split that out into a separate issue?

I'm going to punt on the UX questions for now, and land a CL that simply fixes the window size/position/workspace for restored windows. This leaves in place the existing behaviour otherwise. Happy to tackle behaviour and UX changes at a later time.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b1ba090f0f5fa59c98bd4d8b5c17d6ae7192c749

commit b1ba090f0f5fa59c98bd4d8b5c17d6ae7192c749
Author: chrisha <chrisha@chromium.org>
Date: Fri Aug 18 22:00:50 2017

Ensure History > Recent Tabs restore preserves window disposition.

This plumbs the window bounds, show state and workspace through the tab restore stack.
This ensure that "Window" entries are restored with their previous window parameters.

This is being submitted with NOPRESUBMIT, because the code uses a deprecated API (Time::To/FromInternal). Unfortunately, it can't be updated to not use it as their exists user data with these values. The code isn't "new", simply refactored.

BUG= 718042 
NOPRESUBMIT=true

Review-Url: https://codereview.chromium.org/2868983003
Cr-Commit-Position: refs/heads/master@{#495707}

[modify] https://crrev.com/b1ba090f0f5fa59c98bd4d8b5c17d6ae7192c749/chrome/browser/sessions/chrome_tab_restore_service_client.cc
[modify] https://crrev.com/b1ba090f0f5fa59c98bd4d8b5c17d6ae7192c749/chrome/browser/sessions/chrome_tab_restore_service_client.h
[modify] https://crrev.com/b1ba090f0f5fa59c98bd4d8b5c17d6ae7192c749/chrome/browser/sessions/persistent_tab_restore_service_unittest.cc
[modify] https://crrev.com/b1ba090f0f5fa59c98bd4d8b5c17d6ae7192c749/chrome/browser/sessions/tab_restore_browsertest.cc
[modify] https://crrev.com/b1ba090f0f5fa59c98bd4d8b5c17d6ae7192c749/chrome/browser/ui/android/tab_model/android_live_tab_context.cc
[modify] https://crrev.com/b1ba090f0f5fa59c98bd4d8b5c17d6ae7192c749/chrome/browser/ui/android/tab_model/android_live_tab_context.h
[modify] https://crrev.com/b1ba090f0f5fa59c98bd4d8b5c17d6ae7192c749/chrome/browser/ui/browser_live_tab_context.cc
[modify] https://crrev.com/b1ba090f0f5fa59c98bd4d8b5c17d6ae7192c749/chrome/browser/ui/browser_live_tab_context.h
[modify] https://crrev.com/b1ba090f0f5fa59c98bd4d8b5c17d6ae7192c749/chrome/browser/ui/toolbar/recent_tabs_sub_menu_model.cc
[modify] https://crrev.com/b1ba090f0f5fa59c98bd4d8b5c17d6ae7192c749/components/sessions/core/live_tab_context.h
[modify] https://crrev.com/b1ba090f0f5fa59c98bd4d8b5c17d6ae7192c749/components/sessions/core/persistent_tab_restore_service.cc
[modify] https://crrev.com/b1ba090f0f5fa59c98bd4d8b5c17d6ae7192c749/components/sessions/core/tab_restore_service.h
[modify] https://crrev.com/b1ba090f0f5fa59c98bd4d8b5c17d6ae7192c749/components/sessions/core/tab_restore_service_client.h
[modify] https://crrev.com/b1ba090f0f5fa59c98bd4d8b5c17d6ae7192c749/components/sessions/core/tab_restore_service_helper.cc
[modify] https://crrev.com/b1ba090f0f5fa59c98bd4d8b5c17d6ae7192c749/ios/chrome/browser/sessions/ios_chrome_tab_restore_service_client.h
[modify] https://crrev.com/b1ba090f0f5fa59c98bd4d8b5c17d6ae7192c749/ios/chrome/browser/sessions/ios_chrome_tab_restore_service_client.mm
[modify] https://crrev.com/b1ba090f0f5fa59c98bd4d8b5c17d6ae7192c749/ios/chrome/browser/sessions/tab_restore_service_delegate_impl_ios.h
[modify] https://crrev.com/b1ba090f0f5fa59c98bd4d8b5c17d6ae7192c749/ios/chrome/browser/sessions/tab_restore_service_delegate_impl_ios.mm

Validated that this is fixed on latest canary builds.