Skip to content

Commit

Permalink
[Jank] Add experiment to optimize SwipeRefreshHandler.
Browse files Browse the repository at this point in the history
Currently the SwipeRefreshHandler sets z-order while pulling and
animating. However, this causes a re-layout of the UI. Moreover, when
completing the pull-refresh gesture, it ends up enqueing 5x layouts
while animating away and removing control. Add an experiment flag to
measure impact of doing a single re-layout at the start.

Bug: 1335416
Change-Id: I191a64e23dbdc0ed855cbb89d20b97ca0339006c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3703643
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Reviewed-by: Calder Kitagawa <ckitagawa@chromium.org>
Commit-Queue: Yaron Friedman <yfriedman@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1014996}
  • Loading branch information
yfriedman authored and Chromium LUCI CQ committed Jun 16, 2022
1 parent 332e17e commit 9e5c3d7
Show file tree
Hide file tree
Showing 9 changed files with 30 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import org.chromium.base.metrics.RecordUserAction;
import org.chromium.base.task.PostTask;
import org.chromium.chrome.R;
import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.gesturenav.HistoryNavigationCoordinator;
import org.chromium.chrome.browser.tab.EmptyTabObserver;
import org.chromium.chrome.browser.tab.Tab;
Expand Down Expand Up @@ -195,7 +196,8 @@ public boolean start(
if (type == OverscrollAction.PULL_TO_REFRESH) {
if (mSwipeRefreshLayout == null) initSwipeRefreshLayout(mTab.getContext());
attachSwipeRefreshLayoutIfNecessary();
return mSwipeRefreshLayout.start();
return mSwipeRefreshLayout.start(ChromeFeatureList.isEnabled(
ChromeFeatureList.OPTIMIZE_LAYOUTS_FOR_PULL_REFRESH));
} else if (type == OverscrollAction.HISTORY_NAVIGATION) {
if (mNavigationCoordinator != null) {
mNavigationCoordinator.startGesture();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,8 @@ public boolean onInterceptTouchEvent(MotionEvent event) {
final float yDiff = y - mLastMotionY;
if (yDiff > mTouchSlop && !mIsBeingDragged) {
mIsBeingDragged = true;
start();
// TODO(1335416): Update this to |true| if experiment is successful
start(false);
}
break;
}
Expand Down
4 changes: 4 additions & 0 deletions chrome/browser/flags/android/chrome_feature_list.cc
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,7 @@ const base::Feature* const kFeaturesExposedToJava[] = {
&kBookmarksImprovedSaveFlow,
&kBookmarksRefresh,
&kBackGestureRefactorAndroid,
&kOptimizeLayoutsForPullRefresh,
&kPostTaskFocusTab,
&kProbabilisticCryptidRenderer,
&kReachedCodeProfiler,
Expand Down Expand Up @@ -714,6 +715,9 @@ const base::Feature kBookmarksRefresh{"BookmarksRefresh",
const base::Feature kBackGestureRefactorAndroid{
"BackGestureRefactorAndroid", base::FEATURE_DISABLED_BY_DEFAULT};

const base::Feature kOptimizeLayoutsForPullRefresh{
"OptimizeLayoutsForPullRefresh", base::FEATURE_DISABLED_BY_DEFAULT};

const base::Feature kPostTaskFocusTab{"PostTaskFocusTab",
base::FEATURE_DISABLED_BY_DEFAULT};

Expand Down
1 change: 1 addition & 0 deletions chrome/browser/flags/android/chrome_feature_list.h
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ extern const base::Feature kPageAnnotationsService;
extern const base::Feature kBookmarksImprovedSaveFlow;
extern const base::Feature kBookmarksRefresh;
extern const base::Feature kBackGestureRefactorAndroid;
extern const base::Feature kOptimizeLayoutsForPullRefresh;
extern const base::Feature kPostTaskFocusTab;
extern const base::Feature kProbabilisticCryptidRenderer;
extern const base::Feature kReachedCodeProfiler;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -430,6 +430,7 @@ public static boolean getFieldTrialParamByFeatureAsBoolean(
"OmniboxUpdatedConnectionSecurityIndicators";
public static final String OPTIMIZATION_GUIDE_PUSH_NOTIFICATIONS =
"OptimizationGuidePushNotifications";
public static final String OPTIMIZE_LAYOUTS_FOR_PULL_REFRESH = "OptimizeLayoutsForPullRefresh";
public static final String OVERLAY_NEW_LAYOUT = "OverlayNewLayout";
public static final String PAGE_ANNOTATIONS_SERVICE = "PageAnnotationsService";
public static final String PAGE_INFO_ABOUT_THIS_SITE_EN = "PageInfoAboutThisSiteEn";
Expand Down
1 change: 1 addition & 0 deletions components/paint_preview/player/android/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ android_library("java") {
"//base:base_java",
"//base:jni_java",
"//build/android:build_java",
"//chrome/browser/flags:java",
"//components/browser_ui/styles/android:java",
"//components/paint_preview/browser/android:java",
"//content/public/android:content_java",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,8 @@ public View getView() {

@Override
public boolean start() {
return mSwipeRefreshLayout.start();
// TODO(1335416): Update this to |true| if experiment is successful
return mSwipeRefreshLayout.start(false);
}

@Override
Expand Down
2 changes: 2 additions & 0 deletions third_party/android_swipe_refresh/README.chromium
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,5 @@ SwipeRefreshLayout
* All ViewCompat and MotionEventCompat dependencies removed.
* Added OnResetListener interface to notify SwipeRefreshHandler to detach
this view.
* Add a flag to minimze the number of z-order changes, thereby minimizing
relayouts of the UI.
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,12 @@ public class SwipeRefreshLayout extends ViewGroup {

private boolean mNotify;

/**
* Flag used during duration of pull-refresh animation to reduce the number of calls to
* |bringToFront|, and therefore requested layouts. crbug/1335416
*/
private boolean mOptimizeLayouts;

private int mCircleWidth;

private int mCircleHeight;
Expand Down Expand Up @@ -564,10 +570,11 @@ public void onMeasure(int widthMeasureSpec, int heightMeasureSpec) {
* is currently active, the request will be ignored.
* @return whether a new pull sequence has started.
*/
public boolean start() {
public boolean start(boolean optimizeLayouts) {
if (!isEnabled()) return false;
if (mRefreshing) return false;
mCircleView.clearAnimation();
mOptimizeLayouts = optimizeLayouts;
mProgress.stop();
// See ACTION_DOWN handling in {@link #onTouchEvent(...)}.
setTargetOffsetTopAndBottom(mOriginalOffsetTop - mCircleView.getTop(), true);
Expand Down Expand Up @@ -636,6 +643,12 @@ public void pull(float delta) {
true /* requires update */);
}

@Override
public void bringChildToFront(View child) {
if (mOptimizeLayouts && indexOfChild(child) == getChildCount() - 1) return;
super.bringChildToFront(child);
}

/**
* Release the active pull. If no pull has started, the release will be
* ignored. If the pull was sufficiently large, the refresh sequence will
Expand Down

0 comments on commit 9e5c3d7

Please sign in to comment.