As we already support Ctrl+T and recent closed tab. This should be straight forward to add.

+1. Missing Ctrl+Shift+T on the Pixel C.

Assigning to Alex.  Looks like a good starter bug.  Ted let me know if you have someone else in mind for this.  Thanks!

Go for it.  It "could" be larger than expected, but maybe not.

We don't use the same undo controller as desktop, so we need to figure out how to regenerate tabs.  If we just use recently closed, then the tab ordering won't be preserved which isn't terribly nice.

Also, we have our undo snackbar and undo ui in the accessibility tab switcher that we'd want to delegate to first if active, so that complicates thing a bit more.

Good opportunity to explore all aspects of the code base though.

According to sky's comment on the CL, also implementing LiveTabContext interface for android, it serve as adapter so we can nicely access tab restore component data.

Have a question on a UI behavior, should the restored tab in the foreground when user hits ctrl+shift+T shortcut or presses Undo button in the undo snack bar?

Currently undo snack bar didn't bring tab to foreground. And ctrl+shift+T does the same thing. 

rolfe@ - can you please weigh in with some UX expertise?

To elaborate: Right now tabs restored when the undo snackbar is showing are restored in the background (current mobile behavior when tapping on the snackbar) and tabs restored when the snackbar is not showing are restored in the foreground and become the selected tab (desktop behavior on ctrl+shift+T).

In playing around with this, I found it a little odd (although not blocking?) that ctrl+shift+T had different background/foreground restore behavior based on whether the snackbar for the tab was still showing.

I think there are a couple of questions on what the ideal UX should be:
1. Should we always restore ctrl+shift+T in the foreground (desktop) or background (snackbar behavior)?
2. If we choose always in the foreground, should we change a tap on the undo snackbar to restore in the foreground?

Speaking from a completely non-technical perspective!
1) Restore for foreground tab on tablet sounds great to me.
2) This means changing the snackbar tap to restore to foreground.

rolfe@ - Just double checking, ideally we would restore in the background for phones?

I think so, if background means keep current behavior (basically re-open the previous stack.) You can kind of be focused on one tab in mobile but really since you're looking at a full stack in a preview format it doesn't much help to open in foreground (and I don't think matters a ton since the keyboard shortcut won't apply yeah?)

I'm not sure I followed the question. 

For the undo snack bar on phones, we commit all tab closures and hide the snackbar when the tab switcher view is hidden so you can only undo tab closures while in the tab switcher (unless you're really really really really fast and tap while the animation to hide the tab switcher is still in progress). The re-opened tab shows up in it's original position in the stack. I think we should retain the positioning behavior & not exit the tab switcher when a tab closure is undone (no tab gets selected).

Ctrl + shift + t changes when you can undo a tab closure on phones. It will work when focused on a single tab or when in the tab switcher. Foreground vs background in this context refers to whether the re-opened tab is selected. In the background means that if I'm looking at http://google.com and undo a tab closure for http://facebook.com, I'll still be looking at http://google.com (presumably the animation we run when opening new tabs via the long press context menu will run in this scenario). In the foreground means that after undoing the tab closure for http://facebook.com, I'll be looking at http://facebook.com.


Would it help if we grab some videos or swing by your desk sometime today/early next week?

Oh.. I get the question now. Need coffee :)

Keyboard shortcuts still work on phones. It's a less common use case but it's technically possible to connect a keyboard to a phone.

OK let's just go with "as long as the tab stack exists the way it does" we would restore in background for phones. Foreground restore just doesn't work as well there as it does with the tab strip style.

Let me know if I missed any other questions here. Thanks for your patience!

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

commit 6fbb24a66cb8bb459fed7a4db23428620edb9048
Author: xingliu <xingliu@chromium.org>
Date: Thu Jul 14 23:24:56 2016

Shortcut ctrl+shift+T added on android.

This CL first check if we can undo the recent closure in java by using
TabModelImpl.

If nothing we can do with undo snackbar logic, then use
recently_closed_tabs_bridge in native code to retrive data from
tab_restore_service.

BUG= 602559 
https://bugs.chromium.org/p/chromium/issues/detail?id=602559#c7

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

[modify] https://crrev.com/6fbb24a66cb8bb459fed7a4db23428620edb9048/chrome/android/java/res/values/ids.xml
[modify] https://crrev.com/6fbb24a66cb8bb459fed7a4db23428620edb9048/chrome/android/java/src/org/chromium/chrome/browser/ChromeTabbedActivity.java
[modify] https://crrev.com/6fbb24a66cb8bb459fed7a4db23428620edb9048/chrome/android/java/src/org/chromium/chrome/browser/KeyboardShortcuts.java
[modify] https://crrev.com/6fbb24a66cb8bb459fed7a4db23428620edb9048/chrome/android/java/src/org/chromium/chrome/browser/ntp/RecentlyClosedBridge.java
[modify] https://crrev.com/6fbb24a66cb8bb459fed7a4db23428620edb9048/chrome/android/java/src/org/chromium/chrome/browser/tabmodel/EmptyTabModel.java
[modify] https://crrev.com/6fbb24a66cb8bb459fed7a4db23428620edb9048/chrome/android/java/src/org/chromium/chrome/browser/tabmodel/OffTheRecordTabModel.java
[modify] https://crrev.com/6fbb24a66cb8bb459fed7a4db23428620edb9048/chrome/android/java/src/org/chromium/chrome/browser/tabmodel/SingleTabModel.java
[modify] https://crrev.com/6fbb24a66cb8bb459fed7a4db23428620edb9048/chrome/android/java/src/org/chromium/chrome/browser/tabmodel/TabModel.java
[modify] https://crrev.com/6fbb24a66cb8bb459fed7a4db23428620edb9048/chrome/android/java/src/org/chromium/chrome/browser/tabmodel/TabModelImpl.java
[modify] https://crrev.com/6fbb24a66cb8bb459fed7a4db23428620edb9048/chrome/android/java/src/org/chromium/chrome/browser/tabmodel/document/DocumentTabModelImpl.java
[modify] https://crrev.com/6fbb24a66cb8bb459fed7a4db23428620edb9048/chrome/android/javatests/src/org/chromium/chrome/browser/multiwindow/MultiWindowUtilsTest.java
[modify] https://crrev.com/6fbb24a66cb8bb459fed7a4db23428620edb9048/chrome/android/javatests/src/org/chromium/chrome/browser/tabmodel/UndoTabModelTest.java
[modify] https://crrev.com/6fbb24a66cb8bb459fed7a4db23428620edb9048/chrome/android/javatests/src/org/chromium/chrome/browser/tabmodel/document/MockDocumentTabModel.java
[modify] https://crrev.com/6fbb24a66cb8bb459fed7a4db23428620edb9048/chrome/browser/android/recently_closed_tabs_bridge.cc
[modify] https://crrev.com/6fbb24a66cb8bb459fed7a4db23428620edb9048/chrome/browser/android/recently_closed_tabs_bridge.h
[modify] https://crrev.com/6fbb24a66cb8bb459fed7a4db23428620edb9048/chrome/browser/android/tab_android.cc
[modify] https://crrev.com/6fbb24a66cb8bb459fed7a4db23428620edb9048/chrome/browser/android/tab_android.h
[modify] https://crrev.com/6fbb24a66cb8bb459fed7a4db23428620edb9048/chrome/browser/sessions/chrome_tab_restore_service_client.cc
[add] https://crrev.com/6fbb24a66cb8bb459fed7a4db23428620edb9048/chrome/browser/ui/android/tab_model/android_live_tab_context.cc
[add] https://crrev.com/6fbb24a66cb8bb459fed7a4db23428620edb9048/chrome/browser/ui/android/tab_model/android_live_tab_context.h
[modify] https://crrev.com/6fbb24a66cb8bb459fed7a4db23428620edb9048/chrome/browser/ui/android/tab_model/tab_model.cc
[modify] https://crrev.com/6fbb24a66cb8bb459fed7a4db23428620edb9048/chrome/browser/ui/android/tab_model/tab_model.h
[modify] https://crrev.com/6fbb24a66cb8bb459fed7a4db23428620edb9048/chrome/browser/ui/android/tab_model/tab_model_jni_bridge.cc
[modify] https://crrev.com/6fbb24a66cb8bb459fed7a4db23428620edb9048/chrome/chrome_browser_ui.gypi
[modify] https://crrev.com/6fbb24a66cb8bb459fed7a4db23428620edb9048/tools/metrics/actions/actions.xml

 Issue 588514  has been merged into this issue.