Commit 29d0e902 authored by Sinan Sahin's avatar Sinan Sahin Committed by Commit Bot

[Offline indicator v2] Wait for omnibox unfocus to show/hide

If we show or hide the offline indicator when the omnibox is focused,
the omnibox loses focus. This is because the toolbar's visibility
changes during the animation (compositor offsets change), which clears
the view's focus.

The fix (or workaround) this CL introduces is to wait until the user
unfocuses the omnibox before showing/hiding the offline indicator.

Bug: 1069552
Change-Id: Id462d76390895b02226c6404bcb432f0d74948db
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2165872
Commit-Queue: Sinan Sahin <sinansahin@google.com>
Reviewed-by: default avatarTheresa  <twellington@chromium.org>
Cr-Commit-Position: refs/heads/master@{#763550}
parent 637de82c
......@@ -11,6 +11,8 @@ import android.os.Handler;
import androidx.vectordrawable.graphics.drawable.VectorDrawableCompat;
import org.chromium.base.ApiCompatibilityUtils;
import org.chromium.base.Callback;
import org.chromium.base.supplier.ObservableSupplier;
import org.chromium.chrome.R;
import org.chromium.chrome.browser.status_indicator.StatusIndicatorCoordinator;
......@@ -24,24 +26,26 @@ public class OfflineIndicatorControllerV2 implements ConnectivityDetector.Observ
private Context mContext;
private StatusIndicatorCoordinator mStatusIndicator;
private ConnectivityDetector mConnectivityDetector;
private ObservableSupplier<Boolean> mIsUrlBarFocusedSupplier;
private Callback<Boolean> mOnUrlBarFocusChanged;
private Runnable mShowRunnable;
private Runnable mHideRunnable;
private Runnable mOnUrlBarUnfocusedRunnable;
/**
* Constructs the offline indicator.
* @param context The {@link Context}.
* @param statusIndicator The {@link StatusIndicatorCoordinator} instance this controller will
* control based on the connectivity.
* @param isUrlBarFocusedSupplier The {@link ObservableSupplier} that will supply the UrlBar's
* focus state and notify a listener when it changes.
*/
public OfflineIndicatorControllerV2(
Context context, StatusIndicatorCoordinator statusIndicator) {
public OfflineIndicatorControllerV2(Context context, StatusIndicatorCoordinator statusIndicator,
ObservableSupplier<Boolean> isUrlBarFocusedSupplier) {
mContext = context;
mStatusIndicator = statusIndicator;
mConnectivityDetector = new ConnectivityDetector(this);
}
@Override
public void onConnectionStateChanged(int connectionState) {
final boolean offline = connectionState != ConnectivityDetector.ConnectionState.VALIDATED;
if (offline) {
mShowRunnable = () -> {
final int backgroundColor = ApiCompatibilityUtils.getColor(
mContext.getResources(), R.color.offline_indicator_offline_color);
final int textColor = ApiCompatibilityUtils.getColor(
......@@ -52,7 +56,9 @@ public class OfflineIndicatorControllerV2 implements ConnectivityDetector.Observ
mContext.getResources(), R.color.default_icon_color_light);
mStatusIndicator.show(mContext.getString(R.string.offline_indicator_v2_offline_text),
statusIcon, backgroundColor, textColor, iconTint);
} else {
};
mHideRunnable = () -> {
final int backgroundColor = ApiCompatibilityUtils.getColor(
mContext.getResources(), R.color.offline_indicator_back_online_color);
final int textColor = ApiCompatibilityUtils.getColor(
......@@ -69,6 +75,30 @@ public class OfflineIndicatorControllerV2 implements ConnectivityDetector.Observ
mStatusIndicator.updateContent(
mContext.getString(R.string.offline_indicator_v2_back_online_text), statusIcon,
backgroundColor, textColor, iconTint, hide);
};
mIsUrlBarFocusedSupplier = isUrlBarFocusedSupplier;
// TODO(crbug.com/1075793): Move the UrlBar focus related code to the widget or glue code.
mOnUrlBarFocusChanged = (hasFocus) -> {
if (!hasFocus && mOnUrlBarUnfocusedRunnable != null) {
mOnUrlBarUnfocusedRunnable.run();
mOnUrlBarUnfocusedRunnable = null;
}
};
mIsUrlBarFocusedSupplier.addObserver(mOnUrlBarFocusChanged);
mConnectivityDetector = new ConnectivityDetector(this);
}
@Override
public void onConnectionStateChanged(int connectionState) {
final boolean offline = connectionState != ConnectivityDetector.ConnectionState.VALIDATED;
if (mIsUrlBarFocusedSupplier.get()) {
mOnUrlBarUnfocusedRunnable = offline ? mShowRunnable : mHideRunnable;
} else {
(offline ? mShowRunnable : mHideRunnable).run();
assert mOnUrlBarUnfocusedRunnable == null;
}
}
......@@ -77,5 +107,12 @@ public class OfflineIndicatorControllerV2 implements ConnectivityDetector.Observ
mConnectivityDetector.destroy();
mConnectivityDetector = null;
}
if (mIsUrlBarFocusedSupplier != null) {
mIsUrlBarFocusedSupplier.removeObserver(mOnUrlBarFocusChanged);
mIsUrlBarFocusedSupplier = null;
}
mOnUrlBarFocusChanged = null;
}
}
......@@ -29,6 +29,7 @@ import org.chromium.chrome.browser.lifecycle.NativeInitObserver;
import org.chromium.chrome.browser.locale.LocaleManager;
import org.chromium.chrome.browser.multiwindow.MultiWindowUtils;
import org.chromium.chrome.browser.offlinepages.indicator.OfflineIndicatorControllerV2;
import org.chromium.chrome.browser.omnibox.UrlFocusChangeListener;
import org.chromium.chrome.browser.preferences.ChromePreferenceKeys;
import org.chromium.chrome.browser.preferences.SharedPreferencesManager;
import org.chromium.chrome.browser.share.ShareDelegate;
......@@ -59,6 +60,7 @@ public class TabbedRootUiCoordinator extends RootUiCoordinator implements Native
private StatusIndicatorCoordinator mStatusIndicatorCoordinator;
private StatusIndicatorCoordinator.StatusIndicatorObserver mStatusIndicatorObserver;
private OfflineIndicatorControllerV2 mOfflineIndicatorController;
private UrlFocusChangeListener mUrlFocusChangeListener;
private @Nullable ToolbarButtonInProductHelpController mToolbarButtonInProductHelpController;
private boolean mIntentWithEffect;
......@@ -91,6 +93,11 @@ public class TabbedRootUiCoordinator extends RootUiCoordinator implements Native
mOfflineIndicatorController.destroy();
}
if (mToolbarManager != null) {
mToolbarManager.getFakeboxDelegate().removeUrlFocusChangeListener(
mUrlFocusChangeListener);
}
if (mStatusIndicatorCoordinator != null) {
mStatusIndicatorCoordinator.removeObserver(mStatusIndicatorObserver);
mStatusIndicatorCoordinator.removeObserver(mActivity.getStatusBarColorController());
......@@ -233,8 +240,28 @@ public class TabbedRootUiCoordinator extends RootUiCoordinator implements Native
return;
}
mOfflineIndicatorController =
new OfflineIndicatorControllerV2(mActivity, mStatusIndicatorCoordinator);
ObservableSupplierImpl<Boolean> isUrlBarFocusedSupplier = new ObservableSupplierImpl<>();
isUrlBarFocusedSupplier.set(mToolbarManager.getFakeboxDelegate().isUrlBarFocused());
mUrlFocusChangeListener = new UrlFocusChangeListener() {
@Override
public void onUrlFocusChange(boolean hasFocus) {
// Offline indicator should assume the UrlBar is focused if it's focusing.
if (hasFocus) {
isUrlBarFocusedSupplier.set(true);
}
}
@Override
public void onUrlAnimationFinished(boolean hasFocus) {
// Wait for the animation to finish before notifying that UrlBar is unfocused.
if (!hasFocus) {
isUrlBarFocusedSupplier.set(false);
}
}
};
mOfflineIndicatorController = new OfflineIndicatorControllerV2(
mActivity, mStatusIndicatorCoordinator, isUrlBarFocusedSupplier);
mToolbarManager.getFakeboxDelegate().addUrlFocusChangeListener(mUrlFocusChangeListener);
}
@VisibleForTesting
......
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