Skip to content

Commit

Permalink
Android: Close gesture navigation sheet upon tab closure
Browse files Browse the repository at this point in the history
Navigation Sheet should be closed when its Tab is closed. In most cases
the sheet is close prior to Tab, but the opposite can happen if incognito
tabs are closed via Android notification. This CL ensures the sheet gets
closed upon tab closure.

(cherry picked from commit f21ce88)

Bug: 1011073
Change-Id: Id9729c2d8987bfe37ff81f47a8070d40a86e00e7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1842134
Reviewed-by: Matthew Jones <mdjones@chromium.org>
Reviewed-by: Theresa  <twellington@chromium.org>
Commit-Queue: Jinsuk Kim <jinsukkim@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#704483}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1858544
Reviewed-by: Jinsuk Kim <jinsukkim@chromium.org>
Cr-Commit-Position: refs/branch-heads/3904@{#743}
Cr-Branched-From: 675968a-refs/heads/master@{#693954}
  • Loading branch information
JinsukKim committed Oct 13, 2019
1 parent 5e8bb5e commit c7e28be
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,12 @@ interface Delegate {
* Navigates to the page associated with the given index.
*/
void navigateToIndex(int index);

/**
* Sets a runnable to execute when the associated Tab is closed.
* @param runnable Runnable to execute.
*/
void setTabCloseRunnable(Runnable runnable);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ public void onSheetClosed(@StateChangeReason int reason) {
private final int mContentPadding;
private final View mParentView;

private final Runnable mCloseRunnable = () -> close(true);

private static class NavigationItemViewBinder {
public static void bind(PropertyModel model, View view, PropertyKey propertyKey) {
if (ItemProperties.ICON == propertyKey) {
Expand Down Expand Up @@ -165,6 +167,7 @@ private void openSheet(boolean fullyExpand, boolean animate) {

private void expandSheet() {
mBottomSheetController.get().expandSheet();
mDelegate.setTabCloseRunnable(mCloseRunnable);
GestureNavMetrics.recordHistogram("GestureNavigation.Sheet.Viewed", mForward);
}

Expand Down Expand Up @@ -225,6 +228,7 @@ public void release() {
private void close(boolean animate) {
if (!isHidden()) mBottomSheetController.get().hideContent(this, animate);
mBottomSheetController.get().getBottomSheet().removeObserver(mSheetObserver);
mDelegate.setTabCloseRunnable(null);
mMediator.clear();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import org.chromium.chrome.R;
import org.chromium.chrome.browser.history.HistoryManagerUtils;
import org.chromium.chrome.browser.tab.EmptyTabObserver;
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.util.UrlConstants;
import org.chromium.content_public.browser.NavigationEntry;
Expand All @@ -22,6 +23,25 @@ public class TabbedSheetDelegate implements NavigationSheet.Delegate {
private final Tab mTab;
private final String mFullHistoryMenu;

// TabObserver that monitors tab closing event to close the navigation sheet
// together if open. For now this is necessary when closing all incognito tabs
// through Android notification.
private static class CloseTabObserver extends EmptyTabObserver {
private Runnable mCloseRunnable;

public void setCloseRunnable(Runnable runnable) {
mCloseRunnable = runnable;
}

@Override
public void onDestroyed(Tab tab) {
mCloseRunnable.run();
tab.removeObserver(this);
}
}

private final CloseTabObserver mCloseTabObserver = new CloseTabObserver();

public TabbedSheetDelegate(Tab tab) {
mTab = tab;
mFullHistoryMenu = tab.getActivity().getResources().getString(R.string.show_full_history);
Expand All @@ -45,4 +65,14 @@ public void navigateToIndex(int index) {
mTab.getWebContents().getNavigationController().goToNavigationIndex(index);
}
}

@Override
public void setTabCloseRunnable(Runnable runnable) {
if (runnable != null) {
mCloseTabObserver.setCloseRunnable(runnable);
mTab.addObserver(mCloseTabObserver);
} else {
mTab.removeObserver(mCloseTabObserver);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,9 @@ public NavigationHistory getHistory(boolean forward) {
public void navigateToIndex(int index) {
mNavigationController.goToNavigationIndex(index);
}

@Override
public void setTabCloseRunnable(Runnable runnable) {}
}

@Test
Expand Down

0 comments on commit c7e28be

Please sign in to comment.