[ShareButtonInToolbar] add enabled state to optional button

screenshot: https://hsv.googleplex.com/5667084082085888

Bug: 1106145
Change-Id: I51655f571ba9e4c61da5807a7c208b9c163de788
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2307648
Commit-Queue: Jeffrey Cohen <jeffreycohen@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: Patrick Noland <pnoland@chromium.org>
Cr-Commit-Position: refs/heads/master@{#791056}

chrome/android/java/src/org/chromium/chrome/browser/identity_disc/IdentityDiscController.java

@@ -115,7 +115,8 @@ public class IdentityDiscController implements NativeInitObserver, ProfileDataCa
                 R.string.accessibility_toolbar_btn_identity_disc, false,
                 new IPHCommandBuilder(mContext.getResources(),
                         FeatureConstants.IDENTITY_DISC_FEATURE, R.string.iph_identity_disc_text,
-                        R.string.iph_identity_disc_accessibility_text));
+                        R.string.iph_identity_disc_accessibility_text),
+                true);
     }
 
     /**

chrome/android/java/src/org/chromium/chrome/browser/share/ShareButtonController.java

@@ -24,6 +24,9 @@ import org.chromium.chrome.browser.tab.Tab;
 import org.chromium.chrome.browser.toolbar.ButtonData;
 import org.chromium.chrome.browser.toolbar.ButtonDataProvider;
 import org.chromium.chrome.browser.toolbar.bottom.BottomToolbarVariationManager;
+import org.chromium.ui.modaldialog.ModalDialogManager;
+import org.chromium.ui.modaldialog.ModalDialogManager.ModalDialogManagerObserver;
+import org.chromium.ui.modelutil.PropertyModel;
 
 /**
  * Handles displaying share button on toolbar depending on several conditions (e.g.,device width,
@@ -52,6 +55,9 @@ public class ShareButtonController implements ButtonDataProvider, ConfigurationC
     private final ObservableSupplier<Boolean> mBottomToolbarVisibilitySupplier;
     private OnClickListener mOnClickListener;
 
+    private ModalDialogManager mModalDialogManager;
+    private ModalDialogManagerObserver mModalDialogManagerObserver;
+
     private Integer mMinimumWidthDp;
     private int mScreenWidthDp;
 
@@ -68,11 +74,13 @@ public class ShareButtonController implements ButtonDataProvider, ConfigurationC
      * the bottom toolbar.
      * @param activityLifecycleDispatcher Dispatcher for activity lifecycle events, e.g.
      * configuration changes.
+     * @param modalDialogManager dispatcher for modal lifecycles events
      */
     public ShareButtonController(Context context, ActivityTabProvider tabProvider,
             ObservableSupplier<ShareDelegate> shareDelegateSupplier, ShareUtils shareUtils,
             ObservableSupplier<Boolean> bottomToolbarVisibilitySupplier,
-            ActivityLifecycleDispatcher activityLifecycleDispatcher) {
+            ActivityLifecycleDispatcher activityLifecycleDispatcher,
+            ModalDialogManager modalDialogManager) {
         mContext = context;
         mBottomToolbarVisibilitySupplier = bottomToolbarVisibilitySupplier;
         mBottomToolbarVisibilityObserver = (bottomToolbarIsVisible)
@@ -98,9 +106,25 @@ public class ShareButtonController implements ButtonDataProvider, ConfigurationC
             shareDelegate.share(tab, /*shareDirectly=*/false);
         });
 
+        mModalDialogManagerObserver = new ModalDialogManagerObserver() {
+            @Override
+            public void onDialogShown(PropertyModel model) {
+                mButtonData.isEnabled = false;
+                notifyObservers(mButtonData.canShow);
+            }
+
+            @Override
+            public void onDialogHidden(PropertyModel model) {
+                mButtonData.isEnabled = true;
+                notifyObservers(mButtonData.canShow);
+            }
+        };
+        mModalDialogManager = modalDialogManager;
+        mModalDialogManager.addObserver(mModalDialogManagerObserver);
+
         mButtonData = new ButtonData(false,
                 AppCompatResources.getDrawable(mContext, R.drawable.ic_toolbar_share_offset_24dp),
-                mOnClickListener, R.string.share, true, null);
+                mOnClickListener, R.string.share, true, null, true);
 
         mScreenWidthDp = mContext.getResources().getConfiguration().screenWidthDp;
     }
@@ -125,6 +149,11 @@ public class ShareButtonController implements ButtonDataProvider, ConfigurationC
             mBottomToolbarVisibilitySupplier.removeObserver(mBottomToolbarVisibilityObserver);
             mBottomToolbarVisibilityObserver = null;
         }
+        if (mModalDialogManagerObserver != null && mModalDialogManager != null) {
+            mModalDialogManager.removeObserver(mModalDialogManagerObserver);
+            mModalDialogManagerObserver = null;
+            mModalDialogManager = null;
+        }
     }
 
     @Override

chrome/android/java/src/org/chromium/chrome/browser/toolbar/ButtonData.java

@@ -18,6 +18,8 @@ public class ButtonData {
     public View.OnClickListener onClickListener;
     public int contentDescriptionResId;
     public boolean supportsTinting;
+    // This controls the enabled state of the button. Disabled buttons are also not clickable.
+    public boolean isEnabled;
     /**
      * Builder used to show IPH on the button as it's shown. This should include at a minimum the
      * feature name, content string, and accessibility text, but not the anchor view.
@@ -26,12 +28,13 @@ public class ButtonData {
 
     public ButtonData(boolean canShow, Drawable drawable, View.OnClickListener onClickListener,
             int contentDescriptionResId, boolean supportsTinting,
-            IPHCommandBuilder iphCommandBuilder) {
+            IPHCommandBuilder iphCommandBuilder, boolean isEnabled) {
         this.canShow = canShow;
         this.drawable = drawable;
         this.onClickListener = onClickListener;
         this.contentDescriptionResId = contentDescriptionResId;
         this.supportsTinting = supportsTinting;
         this.iphCommandBuilder = iphCommandBuilder;
+        this.isEnabled = isEnabled;
     }
 }

chrome/android/java/src/org/chromium/chrome/browser/toolbar/top/ToolbarPhone.java

@@ -2577,6 +2577,7 @@ public class ToolbarPhone extends ToolbarLayout implements Invalidator.Client, O
         mOptionalButton.setImageDrawable(buttonData.drawable);
         mOptionalButton.setContentDescription(
                 getContext().getResources().getString(buttonData.contentDescriptionResId));
+        mOptionalButton.setEnabled(buttonData.isEnabled);
 
         mOptionalButtonUsesTint = buttonData.supportsTinting;
         if (mOptionalButtonUsesTint) {

chrome/android/java/src/org/chromium/chrome/browser/toolbar/top/ToolbarTablet.java

@@ -582,6 +582,7 @@ public class ToolbarTablet extends ToolbarLayout
         mOptionalButton.setContentDescription(
                 getContext().getResources().getString(buttonData.contentDescriptionResId));
         mOptionalButton.setVisibility(View.VISIBLE);
+        mOptionalButton.setEnabled(buttonData.isEnabled);
     }
 
     @Override

chrome/android/java/src/org/chromium/chrome/browser/ui/RootUiCoordinator.java

@@ -501,7 +501,8 @@ public class RootUiCoordinator
                     mActivity, mActivity.getLifecycleDispatcher(), bottomToolbarVisibilitySupplier);
             ShareButtonController shareButtonController = new ShareButtonController(mActivity,
                     mActivityTabProvider, mShareDelegateSupplier, new ShareUtils(),
-                    bottomToolbarVisibilitySupplier, mActivity.getLifecycleDispatcher());
+                    bottomToolbarVisibilitySupplier, mActivity.getLifecycleDispatcher(),
+                    mActivity.getModalDialogManager());
             mButtonDataProviders = Arrays.asList(mIdentityDiscController, shareButtonController);
             mActionModeControllerCallback = new ToolbarActionModeCallback();
             mToolbarManager = new ToolbarManager(mActivity, mActivity.getBrowserControlsManager(),

chrome/android/javatests/src/org/chromium/chrome/browser/share/ShareButtonControllerTest.java

@@ -12,6 +12,7 @@ import static org.hamcrest.CoreMatchers.allOf;
 import static org.hamcrest.CoreMatchers.anyOf;
 import static org.hamcrest.Matchers.not;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 
@@ -39,6 +40,10 @@ import org.chromium.chrome.test.util.browser.Features.EnableFeatures;
 import org.chromium.chrome.test.util.browser.signin.AccountManagerTestRule;
 import org.chromium.components.embedder_support.util.UrlConstants;
 import org.chromium.content_public.browser.test.util.TestThreadUtils;
+import org.chromium.ui.modaldialog.DialogDismissalCause;
+import org.chromium.ui.modaldialog.ModalDialogManager.ModalDialogType;
+import org.chromium.ui.modaldialog.ModalDialogProperties;
+import org.chromium.ui.modelutil.PropertyModel;
 import org.chromium.ui.test.util.UiRestriction;
 
 /** Tests {@link ShareButtonController}. */
@@ -177,5 +182,60 @@ public final class ShareButtonControllerTest {
         }
     }
 
+    @Test
+    @MediumTest
+    public void testShareButtonInToolbarIsDisabledOnUpdate() {
+        View experimentalButton = mActivityTestRule.getActivity()
+                                          .getToolbarManager()
+                                          .getToolbarLayoutForTesting()
+                                          .getOptionalButtonView();
+
+        ModalDialogProperties.Controller controller = new ModalDialogProperties.Controller() {
+            @Override
+            public void onClick(PropertyModel model, int buttonType) {}
+
+            @Override
+            public void onDismiss(PropertyModel model, int dismissalCause) {}
+        };
+
+        PropertyModel dialogModel = (new PropertyModel.Builder(ModalDialogProperties.ALL_KEYS)
+                                             .with(ModalDialogProperties.CONTROLLER, controller)
+                                             .build());
+
+        TestThreadUtils.runOnUiThreadBlocking(() -> {
+            mActivityTestRule.getActivity().getModalDialogManager().showDialog(
+                    dialogModel, ModalDialogType.APP);
+        });
+
+        if (!mButtonExpected) {
+            assertTrue(
+                    experimentalButton == null || View.GONE == experimentalButton.getVisibility());
+        } else {
+            assertNotNull("experimental button not found", experimentalButton);
+            assertEquals(View.VISIBLE, experimentalButton.getVisibility());
+            String shareString =
+                    mActivityTestRule.getActivity().getResources().getString(R.string.share);
+
+            assertTrue(shareString.equals(experimentalButton.getContentDescription()));
+            assertFalse(experimentalButton.isEnabled());
+        }
+        TestThreadUtils.runOnUiThreadBlocking(() -> {
+            mActivityTestRule.getActivity().getModalDialogManager().dismissDialog(
+                    dialogModel, DialogDismissalCause.UNKNOWN);
+        });
+        if (!mButtonExpected) {
+            assertTrue(
+                    experimentalButton == null || View.GONE == experimentalButton.getVisibility());
+        } else {
+            assertNotNull("experimental button not found", experimentalButton);
+            assertEquals(View.VISIBLE, experimentalButton.getVisibility());
+            String shareString =
+                    mActivityTestRule.getActivity().getResources().getString(R.string.share);
+
+            assertTrue(shareString.equals(experimentalButton.getContentDescription()));
+            assertTrue(experimentalButton.isEnabled());
+        }
+    }
+
     // TODO(crbug/1036023) Add a test that checks that expected intents are fired.
 }

chrome/android/junit/src/org/chromium/chrome/browser/toolbar/top/OptionalBrowsingModeButtonControllerTest.java

@@ -62,9 +62,9 @@ public class OptionalBrowsingModeButtonControllerTest {
     public void setUp() {
         MockitoAnnotations.initMocks(this);
 
-        mButtonData1 = new ButtonData(true, null, null, 0, false, null);
-        mButtonData2 = new ButtonData(true, null, null, 0, false, null);
-        mButtonData3 = new ButtonData(true, null, null, 0, false, null);
+        mButtonData1 = new ButtonData(true, null, null, 0, false, null, true);
+        mButtonData2 = new ButtonData(true, null, null, 0, false, null, true);
+        mButtonData3 = new ButtonData(true, null, null, 0, false, null, true);
         doReturn(mButtonData1).when(mButtonDataProvider1).get(mTab);
         doReturn(mButtonData2).when(mButtonDataProvider2).get(mTab);
         doReturn(mButtonData3).when(mButtonDataProvider3).get(mTab);
@@ -144,7 +144,7 @@ public class OptionalBrowsingModeButtonControllerTest {
         mButtonController.updateButtonVisibility();
         verify(mToolbarLayout, times(1)).updateOptionalButton(mButtonData1);
 
-        ButtonData newButtonData = new ButtonData(true, null, null, 0, false, null);
+        ButtonData newButtonData = new ButtonData(true, null, null, 0, false, null, true);
         doReturn(newButtonData).when(mButtonDataProvider1).get(mTab);
         mButtonController.buttonDataProviderChanged(mButtonDataProvider1, true);
         verify(mToolbarLayout, times(1)).updateOptionalButton(newButtonData);
@@ -159,7 +159,7 @@ public class OptionalBrowsingModeButtonControllerTest {
         mButtonController.updateButtonVisibility();
         verify(mToolbarLayout, times(1)).updateOptionalButton(mButtonData1);
 
-        ButtonData newButtonData = new ButtonData(true, null, null, 0, false, null);
+        ButtonData newButtonData = new ButtonData(true, null, null, 0, false, null, true);
         mButtonController.buttonDataProviderChanged(mButtonDataProvider2, true);
         verify(mToolbarLayout, times(0)).updateOptionalButton(newButtonData);
 
@@ -197,4 +197,11 @@ public class OptionalBrowsingModeButtonControllerTest {
         verify(mButtonDataProvider2, times(1)).removeObserver(mObserverCaptor2.getValue());
         verify(mButtonDataProvider3, times(1)).removeObserver(mObserverCaptor3.getValue());
     }
+
+    @Test
+    public void updateOptionalButtonIsOnEnabled() {
+        mButtonData1.isEnabled = false;
+        mButtonController.updateButtonVisibility();
+        verify(mToolbarLayout, times(1)).updateOptionalButton(mButtonData1);
+    }
 }

chrome/android/junit/src/org/chromium/chrome/browser/toolbar/top/StartSurfaceToolbarMediatorUnitTest.java

@@ -108,8 +108,8 @@ public class StartSurfaceToolbarMediatorUnitTest {
                         .with(StartSurfaceToolbarProperties.MENU_IS_VISIBLE, true)
                         .with(StartSurfaceToolbarProperties.IS_VISIBLE, true)
                         .build();
-        mButtonData = new ButtonData(false, mDrawable, mOnClickListener, 0, false, null);
-        mDisabledButtonData = new ButtonData(false, null, null, 0, false, null);
+        mButtonData = new ButtonData(false, mDrawable, mOnClickListener, 0, false, null, true);
+        mDisabledButtonData = new ButtonData(false, null, null, 0, false, null, true);
         doReturn(mButtonData)
                 .when(mIdentityDiscController)
                 .getForStartSurface(OverviewModeState.SHOWN_HOMEPAGE);