Commit a3adc73d authored by Lijin Shen's avatar Lijin Shen Committed by Chromium LUCI CQ

[Messages] Remove static message utils to fix NPE

MessageUtils resets the accessibility utils to null when one of the
activities is destroyed, which causes the crash when another activity
is about to show a message.

This CL removes static message utils and passes in the a11y util
directly to the classes which need a11y info.

Bug: 1167098
Change-Id: I5404f35ec173ab25334e8108a27b4886f1bb9478
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2634122Reviewed-by: default avatarTheresa  <twellington@chromium.org>
Reviewed-by: default avatarPavel Yatsuk <pavely@chromium.org>
Commit-Queue: Lijin Shen <lazzzis@google.com>
Cr-Commit-Position: refs/heads/master@{#844307}
parent 6a0677b8
...@@ -96,7 +96,6 @@ import org.chromium.components.browser_ui.widget.scrim.ScrimCoordinator; ...@@ -96,7 +96,6 @@ import org.chromium.components.browser_ui.widget.scrim.ScrimCoordinator;
import org.chromium.components.feature_engagement.EventConstants; import org.chromium.components.feature_engagement.EventConstants;
import org.chromium.components.messages.ManagedMessageDispatcher; import org.chromium.components.messages.ManagedMessageDispatcher;
import org.chromium.components.messages.MessageContainer; import org.chromium.components.messages.MessageContainer;
import org.chromium.components.messages.MessageUtils;
import org.chromium.components.messages.MessagesFactory; import org.chromium.components.messages.MessagesFactory;
import org.chromium.content_public.browser.ActionModeCallbackHelper; import org.chromium.content_public.browser.ActionModeCallbackHelper;
import org.chromium.content_public.browser.LoadUrlParams; import org.chromium.content_public.browser.LoadUrlParams;
...@@ -290,8 +289,6 @@ public class RootUiCoordinator ...@@ -290,8 +289,6 @@ public class RootUiCoordinator
mActivity.getLayoutManagerSupplier().removeObserver(mLayoutManagerSupplierCallback); mActivity.getLayoutManagerSupplier().removeObserver(mLayoutManagerSupplierCallback);
MessageUtils.setAccessibilityUtil(null);
if (mMessageDispatcher != null) { if (mMessageDispatcher != null) {
MessagesFactory.detachMessageDispatcher(mMessageDispatcher); MessagesFactory.detachMessageDispatcher(mMessageDispatcher);
mMessageDispatcher = null; mMessageDispatcher = null;
...@@ -434,12 +431,12 @@ public class RootUiCoordinator ...@@ -434,12 +431,12 @@ public class RootUiCoordinator
@CallSuper @CallSuper
public void onFinishNativeInitialization() { public void onFinishNativeInitialization() {
if (ChromeFeatureList.isEnabled(ChromeFeatureList.MESSAGES_FOR_ANDROID_INFRASTRUCTURE)) { if (ChromeFeatureList.isEnabled(ChromeFeatureList.MESSAGES_FOR_ANDROID_INFRASTRUCTURE)) {
MessageUtils.setAccessibilityUtil(ChromeAccessibilityUtil.get());
MessageContainer container = mActivity.findViewById(R.id.message_container); MessageContainer container = mActivity.findViewById(R.id.message_container);
mMessageContainerCoordinator = mMessageContainerCoordinator =
new MessageContainerCoordinator(container, getBrowserControlsManager()); new MessageContainerCoordinator(container, getBrowserControlsManager());
mMessageDispatcher = MessagesFactory.createMessageDispatcher( mMessageDispatcher = MessagesFactory.createMessageDispatcher(container,
container, mMessageContainerCoordinator::getMessageMaxTranslation); mMessageContainerCoordinator::getMessageMaxTranslation,
ChromeAccessibilityUtil.get());
mMessageQueueMediator = new ChromeMessageQueueMediator( mMessageQueueMediator = new ChromeMessageQueueMediator(
mActivity.getBrowserControlsManager(), mMessageContainerCoordinator, mActivity.getBrowserControlsManager(), mMessageContainerCoordinator,
mActivity.getFullscreenManager(), mLayoutStateProviderOneShotSupplier, mActivity.getFullscreenManager(), mLayoutStateProviderOneShotSupplier,
......
...@@ -20,7 +20,6 @@ android_library("java") { ...@@ -20,7 +20,6 @@ android_library("java") {
"java/src/org/chromium/components/messages/MessageDispatcherBridge.java", "java/src/org/chromium/components/messages/MessageDispatcherBridge.java",
"java/src/org/chromium/components/messages/MessageDispatcherProvider.java", "java/src/org/chromium/components/messages/MessageDispatcherProvider.java",
"java/src/org/chromium/components/messages/MessageStateHandler.java", "java/src/org/chromium/components/messages/MessageStateHandler.java",
"java/src/org/chromium/components/messages/MessageUtils.java",
"java/src/org/chromium/components/messages/MessageWrapper.java", "java/src/org/chromium/components/messages/MessageWrapper.java",
"java/src/org/chromium/components/messages/SingleActionMessage.java", "java/src/org/chromium/components/messages/SingleActionMessage.java",
] ]
......
...@@ -6,6 +6,7 @@ package org.chromium.components.messages; ...@@ -6,6 +6,7 @@ package org.chromium.components.messages;
import org.chromium.base.supplier.Supplier; import org.chromium.base.supplier.Supplier;
import org.chromium.ui.modelutil.PropertyModel; import org.chromium.ui.modelutil.PropertyModel;
import org.chromium.ui.util.AccessibilityUtil;
/** /**
* This class implements public MessageDispatcher interface, delegating the actual work to * This class implements public MessageDispatcher interface, delegating the actual work to
...@@ -15,23 +16,27 @@ public class MessageDispatcherImpl implements ManagedMessageDispatcher { ...@@ -15,23 +16,27 @@ public class MessageDispatcherImpl implements ManagedMessageDispatcher {
private final MessageQueueManager mMessageQueueManager = new MessageQueueManager(); private final MessageQueueManager mMessageQueueManager = new MessageQueueManager();
private final MessageContainer mMessageContainer; private final MessageContainer mMessageContainer;
private final Supplier<Integer> mMessageMaxTranslationSupplier; private final Supplier<Integer> mMessageMaxTranslationSupplier;
private final AccessibilityUtil mAccessibilityUtil;
/** /**
* Build a new message dispatcher * Build a new message dispatcher
* @param messageContainer A container view for displaying message banners. * @param messageContainer A container view for displaying message banners.
* @param messageMaxTranslationSupplier A {@link Supplier} that supplies the maximum translation * @param messageMaxTranslationSupplier A {@link Supplier} that supplies the maximum translation
* Y value the message banner can have as a result of the animations or the gestures. * Y value the message banner can have as a result of the animations or the gestures.
* @param accessibilityUtil A util to expose information related to system accessibility state.
*/ */
public MessageDispatcherImpl( public MessageDispatcherImpl(MessageContainer messageContainer,
MessageContainer messageContainer, Supplier<Integer> messageMaxTranslation) { Supplier<Integer> messageMaxTranslation, AccessibilityUtil accessibilityUtil) {
mMessageContainer = messageContainer; mMessageContainer = messageContainer;
mMessageMaxTranslationSupplier = messageMaxTranslation; mMessageMaxTranslationSupplier = messageMaxTranslation;
mAccessibilityUtil = accessibilityUtil;
} }
@Override @Override
public void enqueueMessage(PropertyModel messageProperties) { public void enqueueMessage(PropertyModel messageProperties) {
MessageStateHandler messageStateHandler = new SingleActionMessage(mMessageContainer, MessageStateHandler messageStateHandler =
messageProperties, this::dismissMessage, mMessageMaxTranslationSupplier); new SingleActionMessage(mMessageContainer, messageProperties, this::dismissMessage,
mMessageMaxTranslationSupplier, mAccessibilityUtil);
mMessageQueueManager.enqueueMessage(messageStateHandler, messageProperties); mMessageQueueManager.enqueueMessage(messageStateHandler, messageProperties);
} }
......
...@@ -6,6 +6,7 @@ package org.chromium.components.messages; ...@@ -6,6 +6,7 @@ package org.chromium.components.messages;
import org.chromium.base.supplier.Supplier; import org.chromium.base.supplier.Supplier;
import org.chromium.ui.base.WindowAndroid; import org.chromium.ui.base.WindowAndroid;
import org.chromium.ui.util.AccessibilityUtil;
/** A factory for constructing different Messages related objects. */ /** A factory for constructing different Messages related objects. */
public class MessagesFactory { public class MessagesFactory {
...@@ -17,11 +18,12 @@ public class MessagesFactory { ...@@ -17,11 +18,12 @@ public class MessagesFactory {
* to the MessageContainer. When messages are shown, they will be animated down the * to the MessageContainer. When messages are shown, they will be animated down the
* screen, starting at the negative |messageMaxTranslation| y translation to the resting * screen, starting at the negative |messageMaxTranslation| y translation to the resting
* position in the MessageContainer. * position in the MessageContainer.
* @param accessibilityUtil A util to expose information related to system accessibility state.
* @return The constructed ManagedMessageDispatcher. * @return The constructed ManagedMessageDispatcher.
*/ */
public static ManagedMessageDispatcher createMessageDispatcher( public static ManagedMessageDispatcher createMessageDispatcher(MessageContainer container,
MessageContainer container, Supplier<Integer> messageMaxTranslation) { Supplier<Integer> messageMaxTranslation, AccessibilityUtil accessibilityUtil) {
return new MessageDispatcherImpl(container, messageMaxTranslation); return new MessageDispatcherImpl(container, messageMaxTranslation, accessibilityUtil);
} }
/** /**
......
// Copyright 2020 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.
package org.chromium.components.messages;
import org.chromium.ui.util.AccessibilityUtil;
/**
* A util class designed for messages.
*/
public class MessageUtils {
private static AccessibilityUtil sUtil;
/**
* @return True if a11y is enabled.
*/
public static boolean isA11yEnabled() {
assert sUtil != null;
return sUtil.isAccessibilityEnabled();
}
/**
* Set an {@link AccessibilityUtil} to know if a11y is enabled.
* Should set null when message infra is destroyed.
* @param util The {@link AccessibilityUtil} instance.
*/
public static void setAccessibilityUtil(AccessibilityUtil util) {
sUtil = util;
}
}
...@@ -14,6 +14,7 @@ import androidx.annotation.VisibleForTesting; ...@@ -14,6 +14,7 @@ import androidx.annotation.VisibleForTesting;
import org.chromium.base.Callback; import org.chromium.base.Callback;
import org.chromium.base.supplier.Supplier; import org.chromium.base.supplier.Supplier;
import org.chromium.ui.modelutil.PropertyModel; import org.chromium.ui.modelutil.PropertyModel;
import org.chromium.ui.util.AccessibilityUtil;
/** /**
* Coordinator to show / hide a banner message on given container and delegate events. * Coordinator to show / hide a banner message on given container and delegate events.
...@@ -29,6 +30,7 @@ public class SingleActionMessage implements MessageStateHandler { ...@@ -29,6 +30,7 @@ public class SingleActionMessage implements MessageStateHandler {
private final Callback<PropertyModel> mDismissHandler; private final Callback<PropertyModel> mDismissHandler;
private MessageAutoDismissTimer mAutoDismissTimer; private MessageAutoDismissTimer mAutoDismissTimer;
private final Supplier<Integer> mMaxTranslationSupplier; private final Supplier<Integer> mMaxTranslationSupplier;
private final AccessibilityUtil mAccessibilityUtil;
/** /**
* @param container The container holding messages. * @param container The container holding messages.
...@@ -37,13 +39,15 @@ public class SingleActionMessage implements MessageStateHandler { ...@@ -37,13 +39,15 @@ public class SingleActionMessage implements MessageStateHandler {
* @param dismissHandler The {@link Callback<PropertyModel>} able to dismiss a message by given * @param dismissHandler The {@link Callback<PropertyModel>} able to dismiss a message by given
* property model. * property model.
* @param maxTranslationSupplier A {@link Supplier} that supplies the maximum translation Y * @param maxTranslationSupplier A {@link Supplier} that supplies the maximum translation Y
* value the message banner can have as a result of the animations or the gestures. * @param accessibilityUtil A util to expose information related to system accessibility state.
*/ */
public SingleActionMessage(MessageContainer container, PropertyModel model, public SingleActionMessage(MessageContainer container, PropertyModel model,
Callback<PropertyModel> dismissHandler, Supplier<Integer> maxTranslationSupplier) { Callback<PropertyModel> dismissHandler, Supplier<Integer> maxTranslationSupplier,
AccessibilityUtil accessibilityUtil) {
mModel = model; mModel = model;
mContainer = container; mContainer = container;
mDismissHandler = dismissHandler; mDismissHandler = dismissHandler;
mAccessibilityUtil = accessibilityUtil;
mAutoDismissTimer = new MessageAutoDismissTimer(getAutoDismissDuration()); mAutoDismissTimer = new MessageAutoDismissTimer(getAutoDismissDuration());
mMaxTranslationSupplier = maxTranslationSupplier; mMaxTranslationSupplier = maxTranslationSupplier;
...@@ -110,8 +114,9 @@ public class SingleActionMessage implements MessageStateHandler { ...@@ -110,8 +114,9 @@ public class SingleActionMessage implements MessageStateHandler {
mDismissHandler.onResult(mModel); mDismissHandler.onResult(mModel);
} }
private long getAutoDismissDuration() { @VisibleForTesting
return MessageUtils.isA11yEnabled() ? DURATION_ON_A11Y : DURATION; long getAutoDismissDuration() {
return mAccessibilityUtil.isAccessibilityEnabled() ? DURATION_ON_A11Y : DURATION;
} }
@VisibleForTesting @VisibleForTesting
......
...@@ -41,13 +41,14 @@ public class SingleActionMessageTest extends DummyUiActivityTestCase { ...@@ -41,13 +41,14 @@ public class SingleActionMessageTest extends DummyUiActivityTestCase {
private CallbackHelper mDismissCallback; private CallbackHelper mDismissCallback;
private Callback<PropertyModel> mEmptyDismissCallback = (model) -> {}; private Callback<PropertyModel> mEmptyDismissCallback = (model) -> {};
private AccessibilityUtil mAccessibilityUtil;
@Override @Override
public void setUpTest() throws Exception { public void setUpTest() throws Exception {
super.setUpTest(); super.setUpTest();
mDismissCallback = new CallbackHelper(); mDismissCallback = new CallbackHelper();
AccessibilityUtil util = Mockito.mock(AccessibilityUtil.class); mAccessibilityUtil = Mockito.mock(AccessibilityUtil.class);
when(util.isAccessibilityEnabled()).thenReturn(false); when(mAccessibilityUtil.isAccessibilityEnabled()).thenReturn(false);
MessageUtils.setAccessibilityUtil(util);
} }
@Test @Test
...@@ -55,8 +56,8 @@ public class SingleActionMessageTest extends DummyUiActivityTestCase { ...@@ -55,8 +56,8 @@ public class SingleActionMessageTest extends DummyUiActivityTestCase {
public void testAddAndRemoveSingleActionMessage() throws Exception { public void testAddAndRemoveSingleActionMessage() throws Exception {
MessageContainer container = new MessageContainer(getActivity(), null); MessageContainer container = new MessageContainer(getActivity(), null);
PropertyModel model = createBasicSingleActionMessageModel(); PropertyModel model = createBasicSingleActionMessageModel();
SingleActionMessage message = SingleActionMessage message = new SingleActionMessage(
new SingleActionMessage(container, model, mEmptyDismissCallback, () -> 0); container, model, mEmptyDismissCallback, () -> 0, mAccessibilityUtil);
final MessageBannerCoordinator messageBanner = Mockito.mock(MessageBannerCoordinator.class); final MessageBannerCoordinator messageBanner = Mockito.mock(MessageBannerCoordinator.class);
doNothing().when(messageBanner).show(any(Runnable.class)); doNothing().when(messageBanner).show(any(Runnable.class));
doNothing().when(messageBanner).setOnTouchRunnable(any(Runnable.class)); doNothing().when(messageBanner).setOnTouchRunnable(any(Runnable.class));
...@@ -81,14 +82,29 @@ public class SingleActionMessageTest extends DummyUiActivityTestCase { ...@@ -81,14 +82,29 @@ public class SingleActionMessageTest extends DummyUiActivityTestCase {
"Dismiss callback should be called when message is dismissed"); "Dismiss callback should be called when message is dismissed");
} }
@Test
@MediumTest
public void testAutoDismissDuration() {
MessageContainer container = new MessageContainer(getActivity(), null);
PropertyModel model = createBasicSingleActionMessageModel();
SingleActionMessage message = new SingleActionMessage(
container, model, mEmptyDismissCallback, () -> 0, mAccessibilityUtil);
when(mAccessibilityUtil.isAccessibilityEnabled()).thenReturn(true);
long durationOnA11y = message.getAutoDismissDuration();
when(mAccessibilityUtil.isAccessibilityEnabled()).thenReturn(false);
long duration = message.getAutoDismissDuration();
Assert.assertTrue(
"Message duration should be longer when a11y is on", durationOnA11y > duration);
}
@Test(expected = IllegalStateException.class) @Test(expected = IllegalStateException.class)
@MediumTest @MediumTest
public void testAddMultipleSingleActionMessage() { public void testAddMultipleSingleActionMessage() {
MessageContainer container = new MessageContainer(getActivity(), null); MessageContainer container = new MessageContainer(getActivity(), null);
PropertyModel m1 = createBasicSingleActionMessageModel(); PropertyModel m1 = createBasicSingleActionMessageModel();
PropertyModel m2 = createBasicSingleActionMessageModel(); PropertyModel m2 = createBasicSingleActionMessageModel();
SingleActionMessage message1 = SingleActionMessage message1 = new SingleActionMessage(
new SingleActionMessage(container, m1, mEmptyDismissCallback, () -> 0); container, m1, mEmptyDismissCallback, () -> 0, mAccessibilityUtil);
final MessageBannerCoordinator messageBanner1 = final MessageBannerCoordinator messageBanner1 =
Mockito.mock(MessageBannerCoordinator.class); Mockito.mock(MessageBannerCoordinator.class);
doNothing().when(messageBanner1).show(any(Runnable.class)); doNothing().when(messageBanner1).show(any(Runnable.class));
...@@ -96,8 +112,8 @@ public class SingleActionMessageTest extends DummyUiActivityTestCase { ...@@ -96,8 +112,8 @@ public class SingleActionMessageTest extends DummyUiActivityTestCase {
view1.setId(R.id.message_banner); view1.setId(R.id.message_banner);
message1.setMessageBannerForTesting(messageBanner1); message1.setMessageBannerForTesting(messageBanner1);
message1.setViewForTesting(view1); message1.setViewForTesting(view1);
SingleActionMessage message2 = SingleActionMessage message2 = new SingleActionMessage(
new SingleActionMessage(container, m2, mEmptyDismissCallback, () -> 0); container, m2, mEmptyDismissCallback, () -> 0, mAccessibilityUtil);
final MessageBannerCoordinator messageBanner2 = final MessageBannerCoordinator messageBanner2 =
Mockito.mock(MessageBannerCoordinator.class); Mockito.mock(MessageBannerCoordinator.class);
doNothing().when(messageBanner2).show(any(Runnable.class)); doNothing().when(messageBanner2).show(any(Runnable.class));
......
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