Commit 108b2013 authored by Lijin Shen's avatar Lijin Shen Committed by Commit Bot

[Messages] Dismiss messages when entering overview mode

1. Dismiss currently showing message when entering overview mode
2. refactor delegate so that it can do work after hiding animation is
finished

Bug: 1123947
Change-Id: I71d7bd1868972ba93505f1a5373f8e716176b89c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2529617Reviewed-by: default avatarPavel Yatsuk <pavely@chromium.org>
Reviewed-by: default avatarTheresa  <twellington@chromium.org>
Reviewed-by: default avatarSinan Sahin <sinansahin@google.com>
Commit-Queue: Lijin Shen <lazzzis@google.com>
Cr-Commit-Position: refs/heads/master@{#826646}
parent f5a1b7a6
......@@ -4,12 +4,16 @@
package org.chromium.chrome.browser.messages;
import org.chromium.base.supplier.OneshotSupplier;
import org.chromium.chrome.browser.browser_controls.BrowserControlsStateProvider;
import org.chromium.chrome.browser.browser_controls.BrowserControlsUtils;
import org.chromium.chrome.browser.fullscreen.BrowserControlsManager;
import org.chromium.chrome.browser.fullscreen.FullscreenManager;
import org.chromium.chrome.browser.fullscreen.FullscreenManager.Observer;
import org.chromium.chrome.browser.fullscreen.FullscreenOptions;
import org.chromium.chrome.browser.layouts.LayoutStateProvider;
import org.chromium.chrome.browser.layouts.LayoutStateProvider.LayoutStateObserver;
import org.chromium.chrome.browser.layouts.LayoutType;
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.components.messages.ManagedMessageDispatcher;
import org.chromium.components.messages.MessageQueueDelegate;
......@@ -27,6 +31,7 @@ public class ChromeMessageQueueMediator implements MessageQueueDelegate {
private FullscreenManager mFullscreenManager;
private int mBrowserControlsToken = TokenHolder.INVALID_TOKEN;
private BrowserControlsObserver mBrowserControlsObserver;
private LayoutStateProvider mLayoutStateProvider;
private FullscreenManager.Observer mFullScreenObserver = new Observer() {
private int mToken = TokenHolder.INVALID_TOKEN;
......@@ -43,16 +48,37 @@ public class ChromeMessageQueueMediator implements MessageQueueDelegate {
}
};
private LayoutStateObserver mLayoutStateObserver = new LayoutStateObserver() {
private int mToken = TokenHolder.INVALID_TOKEN;
@Override
public void onStartedShowing(int layoutType, boolean showToolbar) {
if (layoutType == LayoutType.TAB_SWITCHER) {
mToken = suspendQueue();
}
}
@Override
public void onFinishedHiding(int layoutType) {
if (layoutType == LayoutType.TAB_SWITCHER) {
resumeQueue(mToken);
}
}
};
/**
* @param browserControlsManager The browser controls manager able to toggle the visibility of
* browser controls.
* @param messageContainerCoordinator The coordinator able to show and hide message container.
* @param fullscreenManager The full screen manager able to notify the fullscreen mode change.
* @param layoutStateProviderOneShotSupplier Supplier of the {@link LayoutStateProvider}.
* @param messageDispatcher The {@link ManagedMessageDispatcher} able to suspend/resume queue.
*/
public ChromeMessageQueueMediator(BrowserControlsManager browserControlsManager,
MessageContainerCoordinator messageContainerCoordinator,
FullscreenManager fullscreenManager, ManagedMessageDispatcher messageDispatcher) {
FullscreenManager fullscreenManager,
OneshotSupplier<LayoutStateProvider> layoutStateProviderOneShotSupplier,
ManagedMessageDispatcher messageDispatcher) {
mBrowserControlsManager = browserControlsManager;
mContainerCoordinator = messageContainerCoordinator;
mFullscreenManager = fullscreenManager;
......@@ -60,11 +86,16 @@ public class ChromeMessageQueueMediator implements MessageQueueDelegate {
mFullscreenManager.addObserver(mFullScreenObserver);
mBrowserControlsObserver = new BrowserControlsObserver();
mBrowserControlsManager.addObserver(mBrowserControlsObserver);
layoutStateProviderOneShotSupplier.onAvailable(this::setLayoutStateProvider);
}
public void destroy() {
mFullscreenManager.removeObserver(mFullScreenObserver);
mBrowserControlsManager.removeObserver(mBrowserControlsObserver);
if (mLayoutStateProvider != null) {
mLayoutStateProvider.removeObserver(mLayoutStateObserver);
}
mLayoutStateProvider = null;
mQueueController = null;
mContainerCoordinator = null;
mBrowserControlsManager = null;
......@@ -72,7 +103,7 @@ public class ChromeMessageQueueMediator implements MessageQueueDelegate {
}
@Override
public void prepareToShow(Runnable runnable) {
public void onStartShowing(Runnable runnable) {
mBrowserControlsToken =
mBrowserControlsManager.getBrowserVisibilityDelegate().showControlsPersistent();
mContainerCoordinator.showMessageContainer();
......@@ -84,9 +115,7 @@ public class ChromeMessageQueueMediator implements MessageQueueDelegate {
}
@Override
public void prepareToHide(Runnable runnable) {
// TODO(crbug.com/1123947): wait message hiding animation to be finished.
runnable.run();
public void onFinishHiding() {
mBrowserControlsManager.getBrowserVisibilityDelegate().releasePersistentShowingToken(
mBrowserControlsToken);
mContainerCoordinator.hideMessageContainer();
......@@ -107,6 +136,14 @@ public class ChromeMessageQueueMediator implements MessageQueueDelegate {
mQueueController.resume(token);
}
/**
* @param layoutStateProvider The provider able to add observer to observe overview mode.
*/
private void setLayoutStateProvider(LayoutStateProvider layoutStateProvider) {
mLayoutStateProvider = layoutStateProvider;
mLayoutStateProvider.addObserver(mLayoutStateObserver);
}
class BrowserControlsObserver implements BrowserControlsStateProvider.Observer {
private Runnable mRunOnControlsFullyVisible;
......
......@@ -398,9 +398,10 @@ public class RootUiCoordinator
new MessageContainerCoordinator(container, getBrowserControlsManager());
mMessageDispatcher = MessagesFactory.createMessageDispatcher(
container, mMessageContainerCoordinator::getMessageMaxTranslation);
mMessageQueueMediator = new ChromeMessageQueueMediator(
mActivity.getBrowserControlsManager(), mMessageContainerCoordinator,
mActivity.getFullscreenManager(), mMessageDispatcher);
mMessageQueueMediator =
new ChromeMessageQueueMediator(mActivity.getBrowserControlsManager(),
mMessageContainerCoordinator, mActivity.getFullscreenManager(),
mLayoutStateProviderOneShotSupplier, mMessageDispatcher);
mMessageDispatcher.setDelegate(mMessageQueueMediator);
MessagesFactory.attachMessageDispatcher(
mActivity.getWindowAndroid(), mMessageDispatcher);
......@@ -688,7 +689,7 @@ public class RootUiCoordinator
public void onFinishedHiding(int layoutType) {
if (layoutType != LayoutType.TAB_SWITCHER) {
hideAppMenu();
};
}
}
};
mLayoutStateProvider.addObserver(mLayoutStateObserver);
......
......@@ -54,14 +54,19 @@ class MessageQueueManager {
if (message == null) return;
mMessageMap.remove(key);
mMessageQueue.remove(message);
Runnable onAnimationFinished = () -> {
message.dismiss();
updateCurrentDisplayedMessage();
};
if (mCurrentDisplayedMessage == message) {
mMessageQueueDelegate.prepareToHide(() -> {
mCurrentDisplayedMessage.hide();
mCurrentDisplayedMessage.hide(true, () -> {
mMessageQueueDelegate.onFinishHiding();
mCurrentDisplayedMessage = null;
onAnimationFinished.run();
});
} else {
onAnimationFinished.run();
}
message.dismiss();
updateCurrentDisplayedMessage();
}
public int suspend() {
......@@ -86,10 +91,10 @@ class MessageQueueManager {
}
if (mCurrentDisplayedMessage == null && !mTokenHolder.hasTokens()) {
mCurrentDisplayedMessage = mMessageQueue.element();
mMessageQueueDelegate.prepareToShow(() -> mCurrentDisplayedMessage.show());
mMessageQueueDelegate.onStartShowing(mCurrentDisplayedMessage::show);
} else if (mCurrentDisplayedMessage != null && mTokenHolder.hasTokens()) {
mMessageQueueDelegate.prepareToHide(() -> {
mCurrentDisplayedMessage.hide();
mCurrentDisplayedMessage.hide(false, () -> {
mMessageQueueDelegate.onFinishHiding();
mCurrentDisplayedMessage = null;
});
}
......
......@@ -5,6 +5,7 @@
package org.chromium.components.messages;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
......@@ -24,15 +25,27 @@ import org.chromium.base.test.BaseRobolectricTestRunner;
public class MessageQueueManagerTest {
private MessageQueueDelegate mEmptyDelegate = new MessageQueueDelegate() {
@Override
public void prepareToShow(Runnable callback) {
public void onStartShowing(Runnable callback) {
callback.run();
}
@Override
public void prepareToHide(Runnable callback) {
callback.run();
}
public void onFinishHiding() {}
};
private class EmptyMessageStateHandler implements MessageStateHandler {
@Override
public void show() {}
@Override
public void hide(boolean animate, Runnable hiddenCallback) {
hiddenCallback.run();
}
@Override
public void dismiss() {}
}
/**
* Tests lifecycle of a single message:
* - enqueueMessage() calls show()
......@@ -43,19 +56,19 @@ public class MessageQueueManagerTest {
public void testEnqueueMessage() {
MessageQueueManager queueManager = new MessageQueueManager();
queueManager.setDelegate(mEmptyDelegate);
MessageStateHandler m1 = Mockito.mock(MessageStateHandler.class);
MessageStateHandler m2 = Mockito.mock(MessageStateHandler.class);
MessageStateHandler m1 = Mockito.spy(new EmptyMessageStateHandler());
MessageStateHandler m2 = Mockito.spy(new EmptyMessageStateHandler());
queueManager.enqueueMessage(m1, m1);
verify(m1).show();
queueManager.dismissMessage(m1);
verify(m1).hide();
verify(m1).hide(anyBoolean(), any());
verify(m1).dismiss();
queueManager.enqueueMessage(m2, m2);
verify(m2).show();
queueManager.dismissMessage(m2);
verify(m2).hide();
verify(m2).hide(anyBoolean(), any());
verify(m2).dismiss();
}
......@@ -67,8 +80,8 @@ public class MessageQueueManagerTest {
public void testOneMessageShownAtATime() {
MessageQueueManager queueManager = new MessageQueueManager();
queueManager.setDelegate(mEmptyDelegate);
MessageStateHandler m1 = Mockito.mock(MessageStateHandler.class);
MessageStateHandler m2 = Mockito.mock(MessageStateHandler.class);
MessageStateHandler m1 = Mockito.spy(new EmptyMessageStateHandler());
MessageStateHandler m2 = Mockito.spy(new EmptyMessageStateHandler());
queueManager.enqueueMessage(m1, m1);
queueManager.enqueueMessage(m2, m2);
......@@ -76,7 +89,7 @@ public class MessageQueueManagerTest {
verify(m2, never()).show();
queueManager.dismissMessage(m1);
verify(m1).hide();
verify(m1).hide(anyBoolean(), any());
verify(m1).dismiss();
verify(m2).show();
}
......@@ -103,7 +116,7 @@ public class MessageQueueManagerTest {
queueManager.dismissMessage(m1);
verify(m2, never()).show();
verify(m2, never()).hide();
verify(m2, never()).hide(anyBoolean(), any());
}
/**
......@@ -131,7 +144,7 @@ public class MessageQueueManagerTest {
public void testDismissMessageTwice() {
MessageQueueManager queueManager = new MessageQueueManager();
queueManager.setDelegate(mEmptyDelegate);
MessageStateHandler m1 = Mockito.mock(MessageStateHandler.class);
MessageStateHandler m1 = Mockito.spy(new EmptyMessageStateHandler());
queueManager.enqueueMessage(m1, m1);
queueManager.dismissMessage(m1);
queueManager.dismissMessage(m1);
......@@ -149,20 +162,20 @@ public class MessageQueueManagerTest {
MessageQueueManager queueManager = new MessageQueueManager();
queueManager.setDelegate(delegate);
int token = queueManager.suspend();
MessageStateHandler m1 = Mockito.mock(MessageStateHandler.class);
MessageStateHandler m1 = Mockito.spy(new EmptyMessageStateHandler());
queueManager.enqueueMessage(m1, m1);
verify(delegate, never()).prepareToShow(any());
verify(delegate, never()).prepareToHide(any());
verify(delegate, never()).onStartShowing(any());
verify(delegate, never()).onFinishHiding();
verify(m1, never()).show();
verify(m1, never()).hide();
verify(m1, never()).hide(anyBoolean(), any());
queueManager.resume(token);
verify(delegate).prepareToShow(any());
verify(delegate).onStartShowing(any());
verify(m1).show();
queueManager.suspend();
verify(delegate).prepareToHide(any());
verify(m1).hide();
verify(delegate).onFinishHiding();
verify(m1).hide(anyBoolean(), any());
}
/**
......@@ -178,15 +191,15 @@ public class MessageQueueManagerTest {
queueManager.suspend();
MessageStateHandler m1 = Mockito.mock(MessageStateHandler.class);
queueManager.enqueueMessage(m1, m1);
verify(delegate, never()).prepareToShow(any());
verify(delegate, never()).prepareToHide(any());
verify(delegate, never()).onStartShowing(any());
verify(delegate, never()).onFinishHiding();
verify(m1, never()).show();
verify(m1, never()).hide();
verify(m1, never()).hide(anyBoolean(), any());
queueManager.dismissMessage(m1);
verify(delegate, never()).prepareToShow(any());
verify(delegate, never()).prepareToHide(any());
verify(delegate, never()).onStartShowing(any());
verify(delegate, never()).onFinishHiding();
verify(m1, never()).show();
verify(m1, never()).hide();
verify(m1, never()).hide(anyBoolean(), any());
}
}
......@@ -18,9 +18,9 @@ public interface MessageDispatcher {
void enqueueMessage(PropertyModel messageProperties);
/**
* Dismisses a message referenced by its PropertyModel. Hdes the message if it is currently
* Dismisses a message referenced by its PropertyModel. Hides the message if it is currently
* displayed. Displays the next message in the queue if available.
* @param messageProperties The PropertyModel of the messageto be dismissed.
* @param messageProperties The PropertyModel of the message to be dismissed.
*/
void dismissMessage(PropertyModel messageProperties);
}
......@@ -10,14 +10,13 @@ package org.chromium.components.messages;
*/
public interface MessageQueueDelegate {
/**
* Call to do preparation work before showing a message.
* Called before a message is shown to allow the delegate to do preparation work.
* @param callback The callback called after all the preparation work has been done.
*/
void prepareToShow(Runnable callback);
void onStartShowing(Runnable callback);
/**
* Call to do preparation work after hiding a message.
* @param callback The callback called before all the preparation work has been done.
* Called after a message is finished hiding.
*/
void prepareToHide(Runnable callback);
void onFinishHiding();
}
......@@ -15,8 +15,11 @@ public interface MessageStateHandler {
/**
* Signals that the message needs to hide its UI.
* @param animate Whether animation should be run or not.
* @param hiddenCallback Called when message has finished hiding. This will run no matter
* whether animation is skipped or not.
*/
void hide();
void hide(boolean animate, Runnable hiddenCallback);
/**
* Notify that the message is about to be dismissed from the queue.
......
......@@ -67,10 +67,18 @@ public class SingleActionMessage implements MessageStateHandler {
* Hide the message view shown on the given {@link MessageContainer}.
*/
@Override
public void hide() {
public void hide(boolean animate, Runnable hiddenCallback) {
mAutoDismissTimer.cancelTimer();
mMessageBanner.setOnTouchRunnable(null);
mMessageBanner.hide(() -> mContainer.removeMessage(mView));
Runnable hiddenRunnable = () -> {
mContainer.removeMessage(mView);
if (hiddenCallback != null) hiddenCallback.run();
};
if (animate) {
mMessageBanner.hide(hiddenRunnable);
} else {
hiddenRunnable.run();
}
}
/**
......
......@@ -63,7 +63,7 @@ public class SingleActionMessageTest extends DummyUiActivityTestCase {
container.getChildCount());
final ArgumentCaptor<Runnable> runnableCaptor = ArgumentCaptor.forClass(Runnable.class);
doNothing().when(messageBanner).hide(runnableCaptor.capture());
message.hide();
message.hide(true, () -> {});
// Let's pretend the animation ended, and the mediator called the callback as a result.
runnableCaptor.getValue().run();
Assert.assertEquals(
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment