Separate scrim/sheet custom lifecycles in onSheetClosed

This patch fixes a subtle bug in the BottomSheetController where a
custom scrim lifecycle inadvertently blocked high priority content
from suppressing low priority content. The block of code no longer
returns early, each scrim and sheet check are now gated on
BottomSheetContent's #hasCustomScrimLifecycle and #hasCustomLifecycle
respectively.

Bug: 986310
Change-Id: I506874b0f8d63e5d00177b409a71b92a51cfb454
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1779103
Commit-Queue: Matthew Jones <mdjones@chromium.org>
Reviewed-by: Theresa  <twellington@chromium.org>
Cr-Commit-Position: refs/heads/master@{#751450}

chrome/android/java/src/org/chromium/chrome/browser/widget/bottomsheet/BottomSheetController.java

@@ -327,21 +327,24 @@ public class BottomSheetController implements Destroyable {
 
             @Override
             public void onSheetClosed(@StateChangeReason int reason) {
-                if (mBottomSheet.getCurrentSheetContent() != null
-                        && mBottomSheet.getCurrentSheetContent().hasCustomScrimLifecycle()) {
-                    return;
+                // Hide the scrim if the current content doesn't have a custom scrim lifecycle.
+                if (mBottomSheet.getCurrentSheetContent() == null
+                        || !mBottomSheet.getCurrentSheetContent().hasCustomScrimLifecycle()) {
+                    scrim.get().hideScrim(true);
                 }
 
-                scrim.get().hideScrim(true);
-
-                // If the sheet is closed, it is an opportunity for another content to try to
-                // take its place if it is a higher priority.
-                BottomSheetContent content = mBottomSheet.getCurrentSheetContent();
-                BottomSheetContent nextContent = mContentQueue.peek();
-                if (content != null && nextContent != null
-                        && nextContent.getPriority() < content.getPriority()) {
-                    mContentQueue.add(content);
-                    mBottomSheet.setSheetState(SheetState.HIDDEN, true);
+                // Try to swap contents unless the sheet's content has a custom lifecycle.
+                if (mBottomSheet.getCurrentSheetContent() != null
+                        && !mBottomSheet.getCurrentSheetContent().hasCustomLifecycle()) {
+                    // If the sheet is closed, it is an opportunity for another content to try to
+                    // take its place if it is a higher priority.
+                    BottomSheetContent content = mBottomSheet.getCurrentSheetContent();
+                    BottomSheetContent nextContent = mContentQueue.peek();
+                    if (content != null && nextContent != null
+                            && nextContent.getPriority() < content.getPriority()) {
+                        mContentQueue.add(content);
+                        mBottomSheet.setSheetState(SheetState.HIDDEN, true);
+                    }
                 }
             }
 

chrome/android/javatests/src/org/chromium/chrome/browser/widget/bottomsheet/BottomSheetControllerTest.java

@@ -11,6 +11,7 @@ import static org.junit.Assert.assertTrue;
 import android.support.test.InstrumentationRegistry;
 import android.support.test.filters.MediumTest;
 import android.support.test.filters.SmallTest;
+import android.view.View;
 import android.view.ViewGroup;
 
 import org.junit.Before;
@@ -346,6 +347,20 @@ public class BottomSheetControllerTest {
         assertEquals(customLifecycleContent, mSheetController.getCurrentSheetContent());
     }
 
+    @Test
+    @MediumTest
+    public void testCustomScrimLifecycle() throws TimeoutException {
+        TestBottomSheetContent customScrimContent = new TestBottomSheetContent(
+                mActivityTestRule.getActivity(), BottomSheetContent.ContentPriority.LOW, true);
+        customScrimContent.setHasCustomScrimLifecycle(true);
+        requestContentInSheet(customScrimContent, true);
+
+        expandSheet();
+
+        assertEquals("The scrim should not be visible with a custom scrim lifecycle.", View.GONE,
+                mScrimView.getVisibility());
+    }
+
     @Test
     @MediumTest
     public void testCloseEvent() throws TimeoutException {

chrome/android/javatests/src/org/chromium/chrome/browser/widget/bottomsheet/TestBottomSheetContent.java

@@ -35,6 +35,9 @@ public class TestBottomSheetContent implements BottomSheetContent {
     /** Whether this content is browser specific. */
     private boolean mHasCustomLifecycle;
 
+    /** Whether this content has a custom scrim lifecycle. */
+    private boolean mHasCustomScrimLifecycle;
+
     /** The peek height of this content. */
     private int mPeekHeight;
 
@@ -151,6 +154,15 @@ public class TestBottomSheetContent implements BottomSheetContent {
         return mFullHeight;
     }
 
+    public void setHasCustomScrimLifecycle(boolean hasCustomScrimLifecycle) {
+        mHasCustomScrimLifecycle = hasCustomScrimLifecycle;
+    }
+
+    @Override
+    public boolean hasCustomScrimLifecycle() {
+        return mHasCustomScrimLifecycle;
+    }
+
     @Override
     public boolean hasCustomLifecycle() {
         return mHasCustomLifecycle;