Commit ad80ebd2 authored by Ioana Pandele's avatar Ioana Pandele Committed by Commit Bot

[Android]Fix image still taking space when visibility is GONE on KitKat & Lollipop

This is fixed by making sure the change to ILLUSTRATION_VISIBLE happens outside
of the layout pass.

Bug: 1013141
Change-Id: I325e37a2ab137553ac511d4cbad77b643ca13471
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1862692
Commit-Queue: Ioana Pandele <ioanap@chromium.org>
Reviewed-by: default avatarBoris Sazonov <bsazonov@chromium.org>
Reviewed-by: default avatarFriedrich [CET] <fhorschig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#707338}
parent ba974b0b
...@@ -20,9 +20,11 @@ import androidx.annotation.DrawableRes; ...@@ -20,9 +20,11 @@ import androidx.annotation.DrawableRes;
import org.chromium.base.Callback; import org.chromium.base.Callback;
import org.chromium.base.VisibleForTesting; import org.chromium.base.VisibleForTesting;
import org.chromium.base.task.PostTask;
import org.chromium.chrome.R; import org.chromium.chrome.R;
import org.chromium.chrome.browser.fullscreen.ChromeFullscreenManager; import org.chromium.chrome.browser.fullscreen.ChromeFullscreenManager;
import org.chromium.chrome.browser.modaldialog.TabModalPresenter; import org.chromium.chrome.browser.modaldialog.TabModalPresenter;
import org.chromium.content_public.browser.UiThreadTaskTraits;
import org.chromium.ui.modaldialog.DialogDismissalCause; import org.chromium.ui.modaldialog.DialogDismissalCause;
import org.chromium.ui.modaldialog.ModalDialogManager; import org.chromium.ui.modaldialog.ModalDialogManager;
import org.chromium.ui.modaldialog.ModalDialogProperties; import org.chromium.ui.modaldialog.ModalDialogProperties;
...@@ -120,7 +122,9 @@ class PasswordManagerDialogMediator implements View.OnLayoutChangeListener { ...@@ -120,7 +122,9 @@ class PasswordManagerDialogMediator implements View.OnLayoutChangeListener {
int oldHeight = oldBottom - oldTop; int oldHeight = oldBottom - oldTop;
int newHeight = bottom - top; int newHeight = bottom - top;
if (newHeight == oldHeight) return; if (newHeight == oldHeight) return;
mModel.set(ILLUSTRATION_VISIBLE, hasSufficientSpaceForIllustration(newHeight)); PostTask.postTask(UiThreadTaskTraits.DEFAULT, () -> {
mModel.set(ILLUSTRATION_VISIBLE, hasSufficientSpaceForIllustration(newHeight));
});
} }
void showDialog(@ModalDialogManager.ModalDialogType int type) { void showDialog(@ModalDialogManager.ModalDialogType int type) {
......
...@@ -45,8 +45,6 @@ public class PasswordManagerDialogView extends ScrollView { ...@@ -45,8 +45,6 @@ public class PasswordManagerDialogView extends ScrollView {
mIllustrationView.setVisibility(VISIBLE); mIllustrationView.setVisibility(VISIBLE);
} else { } else {
mIllustrationView.setVisibility(GONE); mIllustrationView.setVisibility(GONE);
requestLayout(); // Sometimes the visibility isn't propagated correctly and the image
// appears as INVISIBLE.
} }
} }
......
...@@ -8,12 +8,10 @@ import static android.support.test.espresso.Espresso.onView; ...@@ -8,12 +8,10 @@ import static android.support.test.espresso.Espresso.onView;
import static android.support.test.espresso.Espresso.pressBack; import static android.support.test.espresso.Espresso.pressBack;
import static android.support.test.espresso.action.ViewActions.click; import static android.support.test.espresso.action.ViewActions.click;
import static android.support.test.espresso.assertion.ViewAssertions.matches; import static android.support.test.espresso.assertion.ViewAssertions.matches;
import static android.support.test.espresso.matcher.ViewMatchers.assertThat;
import static android.support.test.espresso.matcher.ViewMatchers.isDisplayed; import static android.support.test.espresso.matcher.ViewMatchers.isDisplayed;
import static android.support.test.espresso.matcher.ViewMatchers.withId; import static android.support.test.espresso.matcher.ViewMatchers.withId;
import static android.support.test.espresso.matcher.ViewMatchers.withText; import static android.support.test.espresso.matcher.ViewMatchers.withText;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.not;
import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verify;
...@@ -37,6 +35,7 @@ import org.chromium.chrome.browser.ChromeActivity; ...@@ -37,6 +35,7 @@ import org.chromium.chrome.browser.ChromeActivity;
import org.chromium.chrome.browser.ChromeSwitches; import org.chromium.chrome.browser.ChromeSwitches;
import org.chromium.chrome.test.ChromeActivityTestRule; import org.chromium.chrome.test.ChromeActivityTestRule;
import org.chromium.chrome.test.ChromeJUnit4ClassRunner; import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
import org.chromium.content_public.browser.test.util.CriteriaHelper;
import org.chromium.content_public.browser.test.util.TestThreadUtils; import org.chromium.content_public.browser.test.util.TestThreadUtils;
import org.chromium.ui.modaldialog.DialogDismissalCause; import org.chromium.ui.modaldialog.DialogDismissalCause;
import org.chromium.ui.modaldialog.ModalDialogManager; import org.chromium.ui.modaldialog.ModalDialogManager;
...@@ -135,7 +134,7 @@ public class PasswordManagerDialogTest { ...@@ -135,7 +134,7 @@ public class PasswordManagerDialogTest {
mMediator.onLayoutChange(null, 0, 0, (int) (testWidthDipLandscape * dipScale), mMediator.onLayoutChange(null, 0, 0, (int) (testWidthDipLandscape * dipScale),
(int) (testHeightDipLandscape * dipScale), 0, 0, 0, 0); (int) (testHeightDipLandscape * dipScale), 0, 0, 0, 0);
}); });
assertThat(mModel.get(ILLUSTRATION_VISIBLE), is(false)); CriteriaHelper.pollUiThread(() -> !mModel.get(ILLUSTRATION_VISIBLE));
// Dimensions resembling portrait orientation. // Dimensions resembling portrait orientation.
final int testHeightDipPortrait = 500; final int testHeightDipPortrait = 500;
...@@ -144,7 +143,7 @@ public class PasswordManagerDialogTest { ...@@ -144,7 +143,7 @@ public class PasswordManagerDialogTest {
mMediator.onLayoutChange(null, 0, 0, (int) (testWidthDipPortrait * dipScale), mMediator.onLayoutChange(null, 0, 0, (int) (testWidthDipPortrait * dipScale),
(int) (testHeightDipPortrait * dipScale), 0, 0, 0, 0); (int) (testHeightDipPortrait * dipScale), 0, 0, 0, 0);
}); });
assertThat(mModel.get(ILLUSTRATION_VISIBLE), is(true)); CriteriaHelper.pollUiThread(() -> mModel.get(ILLUSTRATION_VISIBLE));
// Dimensions resembling multi-window mode. // Dimensions resembling multi-window mode.
final int testHeightDipMultiWindow = 250; final int testHeightDipMultiWindow = 250;
...@@ -153,6 +152,6 @@ public class PasswordManagerDialogTest { ...@@ -153,6 +152,6 @@ public class PasswordManagerDialogTest {
mMediator.onLayoutChange(null, 0, 0, (int) (testWidthDipMultiWindow * dipScale), mMediator.onLayoutChange(null, 0, 0, (int) (testWidthDipMultiWindow * dipScale),
(int) (testHeightDipMultiWindow * dipScale), 0, 0, 0, 0); (int) (testHeightDipMultiWindow * dipScale), 0, 0, 0, 0);
}); });
assertThat(mModel.get(ILLUSTRATION_VISIBLE), is(false)); CriteriaHelper.pollUiThread(() -> !mModel.get(ILLUSTRATION_VISIBLE));
} }
} }
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