Commit 008a7737 authored by Sinan Sahin's avatar Sinan Sahin Committed by Commit Bot

[Offline indicator v2] Fix cc layer invalidation issue

When we're hiding the status indicator, we make the Java view GONE and
start animating the cc layer. However, at this point, the cc layer can
have an old texture (or a texture that makes no sense).

With this CL, we invalidate the cc texture when the Java animation
ends and request a render from the LayoutUpdateHost.

Please note that this CL doesn't completely fix the issue. However, it
makes it much less common.

Bug: 1068346
Change-Id: I8e889fec82d5f33c9f5ba30695715a6d6fa67171
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2144629
Commit-Queue: Sinan Sahin <sinansahin@google.com>
Reviewed-by: default avatarTheresa  <twellington@chromium.org>
Cr-Commit-Position: refs/heads/master@{#762014}
parent 6fed5f6a
......@@ -58,11 +58,12 @@ public class StatusIndicatorCoordinator {
* browser controls can be animated. This will be false
* where we can't have a reliable cc::BCOM instance, e.g.
* tab switcher.
* @param requestRender Runnable to request a render when the cc-layer needs to be updated.
*/
public StatusIndicatorCoordinator(Activity activity, ResourceManager resourceManager,
ChromeFullscreenManager fullscreenManager,
Supplier<Integer> statusBarColorWithoutStatusIndicatorSupplier,
Supplier<Boolean> canAnimateNativeBrowserControls) {
Supplier<Boolean> canAnimateNativeBrowserControls, Runnable requestRender) {
// TODO(crbug.com/1005843): Create this view lazily if/when we need it. This is a task for
// when we have the public API figured out. First, we should avoid inflating the view here
// in case it's never used.
......@@ -77,8 +78,14 @@ public class StatusIndicatorCoordinator {
PropertyModelChangeProcessor.create(model,
new StatusIndicatorViewBinder.ViewHolder(root, mSceneLayer),
StatusIndicatorViewBinder::bind);
Runnable invalidateCompositorView = () -> {
root.getResourceAdapter().invalidate(null);
requestRender.run();
};
mMediator = new StatusIndicatorMediator(model, fullscreenManager,
statusBarColorWithoutStatusIndicatorSupplier, canAnimateNativeBrowserControls);
statusBarColorWithoutStatusIndicatorSupplier, canAnimateNativeBrowserControls,
invalidateCompositorView);
resourceManager.getDynamicResourceLoader().registerResource(
root.getId(), root.getResourceAdapter());
root.addOnLayoutChangeListener(mMediator);
......
......@@ -37,6 +37,7 @@ class StatusIndicatorMediator
private Supplier<Integer> mStatusBarWithoutIndicatorColorSupplier;
private Runnable mOnCompositorShowAnimationEnd;
private Supplier<Boolean> mCanAnimateNativeBrowserControls;
private Runnable mInvalidateCompositorView;
private int mIndicatorHeight;
private int mJavaLayoutHeight;
......@@ -54,14 +55,16 @@ class StatusIndicatorMediator
* browser controls can be animated. This will be false
* where we can't have a reliable cc::BCOM instance, e.g.
* tab switcher.
* @param invalidateCompositorView Runnable to invalidate the compositor texture.
*/
StatusIndicatorMediator(PropertyModel model, ChromeFullscreenManager fullscreenManager,
Supplier<Integer> statusBarWithoutIndicatorColorSupplier,
Supplier<Boolean> canAnimateNativeBrowserControls) {
Supplier<Boolean> canAnimateNativeBrowserControls, Runnable invalidateCompositorView) {
mModel = model;
mFullscreenManager = fullscreenManager;
mStatusBarWithoutIndicatorColorSupplier = statusBarWithoutIndicatorColorSupplier;
mCanAnimateNativeBrowserControls = canAnimateNativeBrowserControls;
mInvalidateCompositorView = invalidateCompositorView;
}
@Override
......@@ -128,6 +131,7 @@ class StatusIndicatorMediator
mModel.set(StatusIndicatorProperties.ICON_TINT, iconTint);
mModel.set(StatusIndicatorProperties.ANDROID_VIEW_VISIBILITY, View.INVISIBLE);
mOnCompositorShowAnimationEnd = () -> animateTextFadeIn();
mInvalidateCompositorView.run();
};
final int statusBarColor = mStatusBarWithoutIndicatorColorSupplier.get();
......@@ -297,6 +301,7 @@ class StatusIndicatorMediator
animatorSet.addListener(new AnimatorListenerAdapter() {
@Override
public void onAnimationEnd(Animator animation) {
mInvalidateCompositorView.run();
updateVisibility(true);
}
});
......
......@@ -211,7 +211,7 @@ public class TabbedRootUiCoordinator extends RootUiCoordinator implements Native
mStatusIndicatorCoordinator = new StatusIndicatorCoordinator(mActivity,
mActivity.getCompositorViewHolder().getResourceManager(), fullscreenManager,
mActivity.getStatusBarColorController()::getStatusBarColorWithoutStatusIndicator,
canAnimateBrowserControls);
canAnimateBrowserControls, layoutManager::requestUpdate);
layoutManager.setStatusIndicatorSceneOverlay(mStatusIndicatorCoordinator.getSceneLayer());
mStatusIndicatorObserver = new StatusIndicatorCoordinator.StatusIndicatorObserver() {
@Override
......
......@@ -7,6 +7,7 @@ package org.chromium.chrome.browser.status_indicator;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
......@@ -48,6 +49,9 @@ public class StatusIndicatorMediatorTest {
@Mock
Supplier<Boolean> mCanAnimateNativeBrowserControls;
@Mock
Runnable mInvalidateCompositorView;
private PropertyModel mModel;
private StatusIndicatorMediator mMediator;
......@@ -55,12 +59,13 @@ public class StatusIndicatorMediatorTest {
public void setUp() {
MockitoAnnotations.initMocks(this);
when(mCanAnimateNativeBrowserControls.get()).thenReturn(true);
doNothing().when(mInvalidateCompositorView).run();
mModel = new PropertyModel.Builder(StatusIndicatorProperties.ALL_KEYS)
.with(StatusIndicatorProperties.ANDROID_VIEW_VISIBILITY, View.GONE)
.with(StatusIndicatorProperties.COMPOSITED_VIEW_VISIBLE, false)
.build();
mMediator = new StatusIndicatorMediator(
mModel, mFullscreenManager, () -> Color.WHITE, mCanAnimateNativeBrowserControls);
mMediator = new StatusIndicatorMediator(mModel, mFullscreenManager,
() -> Color.WHITE, mCanAnimateNativeBrowserControls, mInvalidateCompositorView);
}
@Test
......
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