Commit 3347a460 authored by Liquan (Max) Gu's avatar Liquan (Max) Gu Committed by Commit Bot

[ExpandablePaymentHandler] Support half state; remove UI gap

Before:
1 PH UI on opening sometimes have a gap beneath ThinWebView.
2 PH UI set ThinWebView height after the toolbar is laid out
3 PH UI doesn't have half state.

Cause:
For 1, 2, PH UI initialize the height for ThinWebView only when the tab
layout changes. Although tab layout change callback is called the
when PH UI opens, sometimes the toolbar is not laid out yet so the
toolbar height is unknown
For 3, BottomSheet's half state is disabled when PH UI set WRAP_CONTENT
to fullHeightRatio.

Change:
* Set height for ThinWebView when tab layout changes and in PH's
constructor.
* Rather than getting toolbar height from toolbar's layout, get the
toolbar height from dimen const directly.
* Remove onToolbarLaidOut logic.

After:
* the gap is gone.
* reduced complexity to get toolbar height.
* PH UI supports half state.

Bug: 1059066, 1059269

Change-Id: I52435a9ec15f87369486ec0551af67878bb6a755
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2090836Reviewed-by: default avatarLiquan (Max) Gu <maxlg@chromium.org>
Reviewed-by: default avatarRouslan Solomakhin <rouslan@chromium.org>
Commit-Queue: Liquan (Max) Gu <maxlg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#747847}
parent 3ed94f62
...@@ -4,8 +4,12 @@ ...@@ -4,8 +4,12 @@
package org.chromium.chrome.browser.payments.handler; package org.chromium.chrome.browser.payments.handler;
import android.content.Context;
import android.view.View;
import androidx.annotation.VisibleForTesting; import androidx.annotation.VisibleForTesting;
import org.chromium.chrome.R;
import org.chromium.chrome.browser.ChromeActivity; import org.chromium.chrome.browser.ChromeActivity;
import org.chromium.chrome.browser.ChromeVersionInfo; import org.chromium.chrome.browser.ChromeVersionInfo;
import org.chromium.chrome.browser.WebContentsFactory; import org.chromium.chrome.browser.WebContentsFactory;
...@@ -88,7 +92,8 @@ public class PaymentHandlerCoordinator { ...@@ -88,7 +92,8 @@ public class PaymentHandlerCoordinator {
PropertyModel model = new PropertyModel.Builder(PaymentHandlerProperties.ALL_KEYS).build(); PropertyModel model = new PropertyModel.Builder(PaymentHandlerProperties.ALL_KEYS).build();
PaymentHandlerMediator mediator = new PaymentHandlerMediator(model, this::hide, PaymentHandlerMediator mediator = new PaymentHandlerMediator(model, this::hide,
mWebContents, uiObserver, activity.getActivityTab().getView(), mWebContents, uiObserver, activity.getActivityTab().getView(),
mToolbarCoordinator.getView(), mToolbarCoordinator.getShadowHeightPx()); mToolbarCoordinator.getToolbarHeightPx(),
calculateBottomSheetToolbarContainerTopPadding(activity));
activity.getWindow().getDecorView().addOnLayoutChangeListener(mediator); activity.getWindow().getDecorView().addOnLayoutChangeListener(mediator);
BottomSheetController bottomSheetController = activity.getBottomSheetController(); BottomSheetController bottomSheetController = activity.getBottomSheetController();
bottomSheetController.addObserver(mediator); bottomSheetController.addObserver(mediator);
...@@ -121,6 +126,18 @@ public class PaymentHandlerCoordinator { ...@@ -121,6 +126,18 @@ public class PaymentHandlerCoordinator {
return bottomSheetController.requestShowContent(view, /*animate=*/true); return bottomSheetController.requestShowContent(view, /*animate=*/true);
} }
// TODO(crbug.com/1059269): This approach introduces coupling by assuming BottomSheet toolbar's
// layout. Remove it once we can calculate visible content height with better ways.
/**
* @return bottom_sheet_toolbar_container's top padding. This is used to calculate the visible
* content height.
*/
private int calculateBottomSheetToolbarContainerTopPadding(Context context) {
View view = new View(context);
view.setBackgroundResource(R.drawable.top_round);
return view.getPaddingTop();
}
/** /**
* Get the WebContents of the Payment Handler for testing purpose. In other situations, * Get the WebContents of the Payment Handler for testing purpose. In other situations,
* WebContents should not be leaked outside the Payment Handler. * WebContents should not be leaked outside the Payment Handler.
......
...@@ -28,7 +28,7 @@ import org.chromium.ui.modelutil.PropertyModel; ...@@ -28,7 +28,7 @@ import org.chromium.ui.modelutil.PropertyModel;
/* package */ class PaymentHandlerMediator extends WebContentsObserver /* package */ class PaymentHandlerMediator extends WebContentsObserver
implements BottomSheetObserver, PaymentHandlerToolbarObserver, View.OnLayoutChangeListener { implements BottomSheetObserver, PaymentHandlerToolbarObserver, View.OnLayoutChangeListener {
// The value is picked in order to allow users to see the tab behind this UI. // The value is picked in order to allow users to see the tab behind this UI.
private static final float FULL_HEIGHT_RATIO = 0.9f; /* package */ static final float FULL_HEIGHT_RATIO = 0.9f;
private final PropertyModel mModel; private final PropertyModel mModel;
// Whenever invoked, invoked outside of the WebContentsObserver callbacks. // Whenever invoked, invoked outside of the WebContentsObserver callbacks.
...@@ -42,10 +42,9 @@ import org.chromium.ui.modelutil.PropertyModel; ...@@ -42,10 +42,9 @@ import org.chromium.ui.modelutil.PropertyModel;
// Used to postpone execution of a callback to avoid destroy objects (e.g., WebContents) in // Used to postpone execution of a callback to avoid destroy objects (e.g., WebContents) in
// their own methods. // their own methods.
private final Handler mHandler = new Handler(); private final Handler mHandler = new Handler();
private final int mShadowHeightPx;
private final View mToolbarView;
private final View mTabView; private final View mTabView;
private boolean mHasToolbarLaidOut; private final int mToolbarViewHeightPx;
private final int mContainerTopPaddingPx;
/** /**
* Build a new mediator that handle events from outside the payment handler component. * Build a new mediator that handle events from outside the payment handler component.
...@@ -56,22 +55,22 @@ import org.chromium.ui.modelutil.PropertyModel; ...@@ -56,22 +55,22 @@ import org.chromium.ui.modelutil.PropertyModel;
* @param webContents The web-contents that loads the payment app. * @param webContents The web-contents that loads the payment app.
* @param observer The {@link PaymentHandlerUiObserver} that observes this Payment Handler UI. * @param observer The {@link PaymentHandlerUiObserver} that observes this Payment Handler UI.
* @param tabView The view of the main tab. * @param tabView The view of the main tab.
* @param toolbarView The view of the PaymentHandler toolbar. * @param toolbarView The height of the PaymentHandler toolbar view.
* @param shadowHeightPx The height of the toolbar shadow. * @param containerTopPaddingPx The padding top of bottom_sheet_toolbar_container.
*/ */
/* package */ PaymentHandlerMediator(PropertyModel model, Runnable hider, /* package */ PaymentHandlerMediator(PropertyModel model, Runnable hider,
WebContents webContents, PaymentHandlerUiObserver observer, View tabView, WebContents webContents, PaymentHandlerUiObserver observer, View tabView,
View toolbarView, int shadowHeightPx) { int toolbarViewHeightPx, int containerTopPaddingPx) {
super(webContents); super(webContents);
assert webContents != null; assert webContents != null;
mHasToolbarLaidOut = false;
mTabView = tabView; mTabView = tabView;
mShadowHeightPx = shadowHeightPx;
mWebContentsRef = webContents; mWebContentsRef = webContents;
mToolbarView = toolbarView; mToolbarViewHeightPx = toolbarViewHeightPx;
mModel = model; mModel = model;
mHider = hider; mHider = hider;
mPaymentHandlerUiObserver = observer; mPaymentHandlerUiObserver = observer;
mContainerTopPaddingPx = containerTopPaddingPx;
mModel.set(PaymentHandlerProperties.CONTENT_VISIBLE_HEIGHT_PX, contentVisibleHeight());
} }
@Override @Override
...@@ -87,9 +86,6 @@ import org.chromium.ui.modelutil.PropertyModel; ...@@ -87,9 +86,6 @@ import org.chromium.ui.modelutil.PropertyModel;
@Override @Override
public void onLayoutChange(View v, int left, int top, int right, int bottom, int oldLeft, public void onLayoutChange(View v, int left, int top, int right, int bottom, int oldLeft,
int oldTop, int oldRight, int oldBottom) { int oldTop, int oldRight, int oldBottom) {
// Early return because this callback can be invoked before the toolbar has been laid out.
if (!mHasToolbarLaidOut) return;
mModel.set(PaymentHandlerProperties.CONTENT_VISIBLE_HEIGHT_PX, contentVisibleHeight()); mModel.set(PaymentHandlerProperties.CONTENT_VISIBLE_HEIGHT_PX, contentVisibleHeight());
} }
...@@ -112,8 +108,8 @@ import org.chromium.ui.modelutil.PropertyModel; ...@@ -112,8 +108,8 @@ import org.chromium.ui.modelutil.PropertyModel;
/** @return The height of visible area of the bottom sheet's content part. */ /** @return The height of visible area of the bottom sheet's content part. */
private int contentVisibleHeight() { private int contentVisibleHeight() {
return (int) (mTabView.getHeight() * FULL_HEIGHT_RATIO) - mToolbarView.getHeight() return (int) (mTabView.getHeight() * FULL_HEIGHT_RATIO) - mToolbarViewHeightPx
+ mShadowHeightPx; - mContainerTopPaddingPx;
} }
@Override @Override
...@@ -190,10 +186,4 @@ import org.chromium.ui.modelutil.PropertyModel; ...@@ -190,10 +186,4 @@ import org.chromium.ui.modelutil.PropertyModel;
ServiceWorkerPaymentAppBridge.onClosingPaymentAppWindow(mWebContentsRef); ServiceWorkerPaymentAppBridge.onClosingPaymentAppWindow(mWebContentsRef);
mHandler.post(mHider); mHandler.post(mHider);
} }
@Override
public void onToolbarLaidOut() {
mHasToolbarLaidOut = true;
mModel.set(PaymentHandlerProperties.CONTENT_VISIBLE_HEIGHT_PX, contentVisibleHeight());
}
} }
...@@ -80,7 +80,7 @@ import org.chromium.content_public.browser.WebContents; ...@@ -80,7 +80,7 @@ import org.chromium.content_public.browser.WebContents;
@Override @Override
public float getFullHeightRatio() { public float getFullHeightRatio() {
return BottomSheetContent.HeightMode.WRAP_CONTENT; return PaymentHandlerMediator.FULL_HEIGHT_RATIO;
} }
@Override @Override
......
...@@ -37,12 +37,6 @@ public class PaymentHandlerToolbarCoordinator { ...@@ -37,12 +37,6 @@ public class PaymentHandlerToolbarCoordinator {
/** Called when the close button is clicked. */ /** Called when the close button is clicked. */
void onToolbarCloseButtonClicked(); void onToolbarCloseButtonClicked();
/**
* Called when the toolbar has just laid out, called exactly once, because it's used only
* for calculating the height, which does not change in the toolbar after the first layout.
*/
void onToolbarLaidOut();
} }
/** /**
......
...@@ -8,7 +8,6 @@ import android.content.Context; ...@@ -8,7 +8,6 @@ import android.content.Context;
import android.view.LayoutInflater; import android.view.LayoutInflater;
import android.view.View; import android.view.View;
import android.view.View.OnClickListener; import android.view.View.OnClickListener;
import android.view.ViewTreeObserver.OnGlobalLayoutListener;
import android.widget.ImageView; import android.widget.ImageView;
import android.widget.ProgressBar; import android.widget.ProgressBar;
import android.widget.TextView; import android.widget.TextView;
...@@ -20,7 +19,7 @@ import org.chromium.components.browser_ui.widget.FadingShadow; ...@@ -20,7 +19,7 @@ import org.chromium.components.browser_ui.widget.FadingShadow;
import org.chromium.components.browser_ui.widget.FadingShadowView; import org.chromium.components.browser_ui.widget.FadingShadowView;
/** PaymentHandlerToolbar UI. */ /** PaymentHandlerToolbar UI. */
/* package */ class PaymentHandlerToolbarView implements OnGlobalLayoutListener { /* package */ class PaymentHandlerToolbarView {
private final int mToolbarHeightPx; private final int mToolbarHeightPx;
private final int mShadowHeightPx; private final int mShadowHeightPx;
...@@ -46,7 +45,6 @@ import org.chromium.components.browser_ui.widget.FadingShadowView; ...@@ -46,7 +45,6 @@ import org.chromium.components.browser_ui.widget.FadingShadowView;
context.getResources().getDimensionPixelSize(R.dimen.action_bar_shadow_height); context.getResources().getDimensionPixelSize(R.dimen.action_bar_shadow_height);
mToolbarView = LayoutInflater.from(context).inflate(R.layout.sheet_tab_toolbar, null); mToolbarView = LayoutInflater.from(context).inflate(R.layout.sheet_tab_toolbar, null);
mToolbarView.getViewTreeObserver().addOnGlobalLayoutListener(this);
mOriginView = mToolbarView.findViewById(R.id.origin); mOriginView = mToolbarView.findViewById(R.id.origin);
mTitleView = mToolbarView.findViewById(R.id.title); mTitleView = mToolbarView.findViewById(R.id.title);
...@@ -88,11 +86,4 @@ import org.chromium.components.browser_ui.widget.FadingShadowView; ...@@ -88,11 +86,4 @@ import org.chromium.components.browser_ui.widget.FadingShadowView;
/* package */ View getView() { /* package */ View getView() {
return mToolbarView; return mToolbarView;
} }
// OnGlobalLayoutListener:
@Override
public void onGlobalLayout() {
mObserver.onToolbarLaidOut();
mToolbarView.getViewTreeObserver().removeOnGlobalLayoutListener(this);
}
} }
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