Fix a bug causing an endless layout loop.

I've gotten the freeze three times total on my phone, and this time I was near a computer to do some debugging. This is what I found.

Clover uses the ViewTreeObserver in some places to wait for the layout phase to be done and then use the calculated views to do some second calculations. This is not always necessary and I've removed/changed code that used it needlessly. I used this mostly out of laziness to create a new view to implement onMeasure, and not having a good understanding of the measure system. This is bad for performance because a second measure pass is not needed most of the time.

I've misinterpreted the meaning of the return value of onPreDrawListener. I thought false meant continue with the rendering, true to request a new layout phase. This was the other way around and has been switched on all pieces of code that used it wrong.

A perfect example was the use of waitForLayout in the DrawerController. It would wait for a layout pass to get the total width of the containing view and change the width of the drawer. It also used the wrong return value so the layout was not triggered again when needed and triggeded when not needed. This is now fixed and an issue where the drawer width was wrong is now also fixed.

Secondly (and this has been an issue before), the listener would not get removed from the correct ViewTreeObserver in some cases, causing the listener to be triggered on every draw call. This is because Android creates a temporary ViewTreeObserver when a view is not attached to the window, and the callbacks are copied to a real ViewTreeObserver when the view gets attached. I'm still unsure why that does not work sometimes.

The final trigger of the bug was the measure callback that was placed on the width of the menu width on the toolbar (the things that switches the boards). This callback always returned false (thinking that was do not cancel) and canceled the draw every time. This is not harmful normally but when the listener is not removed because the ViewTreeObserver is temporary it would get stuck in an endless loop.

The weird thing is that touch handling still works correctly when Android gets stuck in such an endless loop. This was why you could still press the reply button and the keyboard would show up. It just doesn't get to the draw phase and then appears to look stuck.

I've placed some debug logs in the callbacks when the listener doesn't get removed, and it raises an IllegalArgumentException when the ViewTreeObserver is temporarily.
multisite
Floens 10 years ago
parent 69bfe55b04
commit 262f34dfc1
  1. 7
      Clover/app/src/main/java/org/floens/chan/controller/transition/PushControllerTransition.java
  2. 31
      Clover/app/src/main/java/org/floens/chan/ui/controller/DrawerController.java
  3. 7
      Clover/app/src/main/java/org/floens/chan/ui/controller/ImageViewerController.java
  4. 10
      Clover/app/src/main/java/org/floens/chan/ui/controller/PassSettingsController.java
  5. 41
      Clover/app/src/main/java/org/floens/chan/ui/layout/DrawerWidthAdjustingLayout.java
  6. 2
      Clover/app/src/main/java/org/floens/chan/ui/settings/SettingsController.java
  7. 13
      Clover/app/src/main/java/org/floens/chan/ui/toolbar/Toolbar.java
  8. 2
      Clover/app/src/main/java/org/floens/chan/ui/view/FloatingMenu.java
  9. 2
      Clover/app/src/main/java/org/floens/chan/ui/view/MultiImageView.java
  10. 65
      Clover/app/src/main/java/org/floens/chan/utils/AndroidUtils.java
  11. 6
      Clover/app/src/main/res/layout/controller_navigation_drawer.xml

@ -22,7 +22,6 @@ import android.animation.AnimatorListenerAdapter;
import android.animation.AnimatorSet;
import android.animation.ObjectAnimator;
import android.view.View;
import android.view.animation.AccelerateDecelerateInterpolator;
import android.view.animation.DecelerateInterpolator;
import org.floens.chan.controller.ControllerTransition;
@ -34,9 +33,9 @@ public class PushControllerTransition extends ControllerTransition {
AndroidUtils.waitForMeasure(to.view, new AndroidUtils.OnMeasuredCallback() {
@Override
public boolean onMeasured(View view) {
Animator fromAlpha = ObjectAnimator.ofFloat(from.view, View.ALPHA, 1f, 0.7f);
/*Animator fromAlpha = ObjectAnimator.ofFloat(from.view, View.ALPHA, 1f, 0.7f);
fromAlpha.setDuration(217);
fromAlpha.setInterpolator(new AccelerateDecelerateInterpolator()); // new PathInterpolator(0.4f, 0f, 0.2f, 1f)
fromAlpha.setInterpolator(new AccelerateDecelerateInterpolator()); // new PathInterpolator(0.4f, 0f, 0.2f, 1f)*/
Animator toAlpha = ObjectAnimator.ofFloat(to.view, View.ALPHA, 0f, 1f);
toAlpha.setDuration(200);
@ -56,7 +55,7 @@ public class PushControllerTransition extends ControllerTransition {
AnimatorSet set = new AnimatorSet();
set.playTogether(/*fromAlpha, */toAlpha, toY);
set.start();
return false;
return true;
}
});
}

@ -19,7 +19,6 @@ package org.floens.chan.ui.controller;
import android.content.Context;
import android.content.DialogInterface;
import android.content.res.Configuration;
import android.support.design.widget.Snackbar;
import android.support.v4.widget.DrawerLayout;
import android.support.v7.app.AlertDialog;
@ -97,13 +96,6 @@ public class DrawerController extends Controller implements PinAdapter.Callback,
itemTouchHelper.attachToRecyclerView(recyclerView);
updateBadge();
AndroidUtils.waitForMeasure(drawer, new AndroidUtils.OnMeasuredCallback() {
@Override
public boolean onMeasured(View view) {
return setDrawerWidth();
}
});
}
@Override
@ -119,18 +111,6 @@ public class DrawerController extends Controller implements PinAdapter.Callback,
childController.onShow();
}
@Override
public void onConfigurationChanged(Configuration newConfig) {
super.onConfigurationChanged(newConfig);
AndroidUtils.waitForLayout(drawer, new AndroidUtils.OnMeasuredCallback() {
@Override
public boolean onMeasured(View view) {
return setDrawerWidth();
}
});
}
@Override
public void onClick(View v) {
if (v == settings) {
@ -274,17 +254,6 @@ public class DrawerController extends Controller implements PinAdapter.Callback,
}
}
private boolean setDrawerWidth() {
int width = Math.min(view.getWidth() - dp(56), dp(56) * 6);
if (drawer.getWidth() != width) {
drawer.getLayoutParams().width = width;
drawer.requestLayout();
return true;
} else {
return false;
}
}
private void openController(Controller controller) {
Controller top = getTop();
if (top instanceof NavigationController) {

@ -130,7 +130,12 @@ public class ImageViewerController extends Controller implements ImageViewerPres
pager.addOnPageChangeListener(presenter);
loadingBar = (LoadingBar) view.findViewById(R.id.loading_bar);
AndroidUtils.waitForMeasure(view, new AndroidUtils.OnMeasuredCallback() {
// Sanity check
if (parentController.view.getWindowToken() == null) {
throw new IllegalArgumentException("parentController.view not attached");
}
AndroidUtils.waitForLayout(parentController.view.getViewTreeObserver(), view, new AndroidUtils.OnMeasuredCallback() {
@Override
public boolean onMeasured(View view) {
presenter.onViewMeasured();

@ -80,13 +80,19 @@ public class PassSettingsController extends Controller implements View.OnClickLi
inputToken.setText(ChanSettings.passToken.get());
inputPin.setText(ChanSettings.passPin.get());
AndroidUtils.waitForLayout(view, new AndroidUtils.OnMeasuredCallback() {
// Sanity check
if (parentController.view.getWindowToken() == null) {
throw new IllegalArgumentException("parentController.view not attached");
}
// TODO: remove
AndroidUtils.waitForLayout(parentController.view.getViewTreeObserver(), view, new AndroidUtils.OnMeasuredCallback() {
@Override
public boolean onMeasured(View view) {
crossfadeView.getLayoutParams().height = crossfadeView.getHeight();
crossfadeView.requestLayout();
crossfadeView.toggle(!loggedIn, false);
return true;
return false;
}
});
}

@ -0,0 +1,41 @@
package org.floens.chan.ui.layout;
import android.content.Context;
import android.support.v4.widget.DrawerLayout;
import android.util.AttributeSet;
import android.view.View;
import org.floens.chan.R;
import static org.floens.chan.utils.AndroidUtils.dp;
public class DrawerWidthAdjustingLayout extends DrawerLayout {
public DrawerWidthAdjustingLayout(Context context) {
super(context);
}
public DrawerWidthAdjustingLayout(Context context, AttributeSet attrs) {
super(context, attrs);
}
public DrawerWidthAdjustingLayout(Context context, AttributeSet attrs, int defStyle) {
super(context, attrs, defStyle);
}
@Override
protected void onMeasure(int widthMeasureSpec, int heightMeasureSpec) {
// int widthMode = MeasureSpec.getMode(widthMeasureSpec);
// int heightMode = MeasureSpec.getMode(heightMeasureSpec);
int widthSize = MeasureSpec.getSize(widthMeasureSpec);
// int heightSize = MeasureSpec.getSize(heightMeasureSpec);
View drawer = findViewById(R.id.drawer);
int width = Math.min(widthSize - dp(56), dp(56) * 6);
if (drawer.getLayoutParams().width != width) {
drawer.getLayoutParams().width = width;
}
super.onMeasure(widthMeasureSpec, heightMeasureSpec);
}
}

@ -61,7 +61,7 @@ public class SettingsController extends Controller implements AndroidUtils.OnMea
@Override
public boolean onMeasured(View view) {
setMargins();
return true;
return false;
}
public void onPreferenceChange(SettingView item) {

@ -42,6 +42,7 @@ import org.floens.chan.R;
import org.floens.chan.ui.drawable.ArrowMenuDrawable;
import org.floens.chan.ui.drawable.DropdownArrowDrawable;
import org.floens.chan.ui.layout.SearchLayout;
import org.floens.chan.ui.view.FloatingMenu;
import org.floens.chan.ui.view.LoadView;
import org.floens.chan.utils.AndroidUtils;
@ -453,15 +454,9 @@ public class Toolbar extends LinearLayout implements View.OnClickListener {
menu.addView(item.menu, new LayoutParams(LayoutParams.WRAP_CONTENT, LayoutParams.MATCH_PARENT));
}
AndroidUtils.waitForMeasure(titleView, new AndroidUtils.OnMeasuredCallback() {
@Override
public boolean onMeasured(View view) {
if (item.middleMenu != null) {
item.middleMenu.setPopupWidth(Math.max(dp(200), titleView.getWidth()));
}
return false;
}
});
if (item.middleMenu != null) {
item.middleMenu.setPopupWidth(FloatingMenu.POPUP_WIDTH_ANCHOR);
}
return menu;
}

@ -111,7 +111,7 @@ public class FloatingMenu {
popupWindow.setVerticalOffset(-anchor.getHeight() + anchorOffsetY);
popupWindow.setHorizontalOffset(anchorOffsetX);
if (popupWidth == POPUP_WIDTH_ANCHOR) {
popupWindow.setContentWidth(Math.min(dp(4 * 56), anchor.getWidth()));
popupWindow.setContentWidth(Math.min(dp(8 * 56), Math.max(dp(4 * 56), anchor.getWidth())));
} else if (popupWidth == POPUP_WIDTH_AUTO) {
popupWindow.setContentWidth(dp(3 * 56));
} else {

@ -130,7 +130,7 @@ public class MultiImageView extends FrameLayout implements View.OnClickListener
setVideo(postImage.imageUrl);
break;
}
return false;
return true;
}
});
}

@ -40,7 +40,6 @@ import android.os.Handler;
import android.os.Looper;
import android.preference.PreferenceManager;
import android.support.design.widget.Snackbar;
import android.util.Log;
import android.view.View;
import android.view.ViewGroup;
import android.view.ViewTreeObserver;
@ -58,6 +57,8 @@ import java.util.List;
import java.util.Locale;
public class AndroidUtils {
private static final String TAG = "AndroidUtils";
private static HashMap<String, Typeface> typefaceCache = new HashMap<>();
public static Typeface ROBOTO_MEDIUM;
@ -254,45 +255,87 @@ public class AndroidUtils {
}
public interface OnMeasuredCallback {
/**
* Called when the layout is done.
*
* @param view same view as the argument.
* @return true to continue with rendering, false to cancel and redo the layout.
*/
boolean onMeasured(View view);
}
/**
* Waits for a measure. Calls callback immediately if the view width and height are more than 0.
* Otherwise it registers an onpredrawlistener and rechedules a layout.
* Warning: the view you give must be attached to the view root!!!
* Otherwise it registers an onpredrawlistener.
* <b>Warning: the view you give must be attached to the view root!</b>
*/
public static void waitForMeasure(final View view, final OnMeasuredCallback callback) {
waitForMeasure(true, view, callback);
if (view.getWindowToken() == null) {
// If you call getViewTreeObserver on a view when it's not attached to a window will result in the creation of a temporarily viewtreeobserver.
// This is almost always not what you want.
throw new IllegalArgumentException("The view given to waitForMeasure is not attached to the window and does not have a ViewTreeObserver.");
}
waitForLayoutInternal(true, view.getViewTreeObserver(), view, callback);
}
/**
* Always registers an onpredrawlistener.
* <b>Warning: the view you give must be attached to the view root!</b>
*/
public static void waitForLayout(final View view, final OnMeasuredCallback callback) {
waitForMeasure(false, view, callback);
if (view.getWindowToken() == null) {
// See comment above
throw new IllegalArgumentException("The view given to waitForLayout is not attached to the window and does not have a ViewTreeObserver.");
}
waitForLayoutInternal(false, view.getViewTreeObserver(), view, callback);
}
/**
* Always registers an onpredrawlistener. The given ViewTreeObserver will be used.
*/
public static void waitForLayout(final ViewTreeObserver viewTreeObserver, final View view, final OnMeasuredCallback callback) {
waitForLayoutInternal(false, viewTreeObserver, view, callback);
}
private static void waitForMeasure(boolean returnIfNotZero, final View view, final OnMeasuredCallback callback) {
private static void waitForLayoutInternal(boolean returnIfNotZero, final ViewTreeObserver viewTreeObserver, final View view, final OnMeasuredCallback callback) {
int width = view.getWidth();
int height = view.getHeight();
if (returnIfNotZero && width > 0 && height > 0) {
callback.onMeasured(view);
} else {
view.getViewTreeObserver().addOnPreDrawListener(new ViewTreeObserver.OnPreDrawListener() {
Logger.d(TAG, "Adding OnPreDrawListener to ViewTreeObserver");
viewTreeObserver.addOnPreDrawListener(new ViewTreeObserver.OnPreDrawListener() {
@Override
public boolean onPreDraw() {
final ViewTreeObserver observer = view.getViewTreeObserver();
if (observer.isAlive()) {
observer.removeOnPreDrawListener(this);
Logger.d(TAG, "OnPreDraw callback");
ViewTreeObserver usingViewTreeObserver = viewTreeObserver;
if (viewTreeObserver != view.getViewTreeObserver()) {
Logger.e(TAG, "view.getViewTreeObserver() is another viewtreeobserver! replacing with the new one");
usingViewTreeObserver = view.getViewTreeObserver();
}
if (usingViewTreeObserver.isAlive()) {
usingViewTreeObserver.removeOnPreDrawListener(this);
} else {
Logger.w(TAG, "ViewTreeObserver not alive, could not remove onPreDrawListener! This will probably not end well");
}
boolean ret;
try {
ret = callback.onMeasured(view);
} catch (Exception e) {
Log.i("AndroidUtils", "Exception in onMeasured", e);
Logger.i(TAG, "Exception in onMeasured", e);
throw e;
}
if (!ret) {
Logger.w(TAG, "waitForLayout requested a re-layout by returning false");
}
return ret;
}
});

@ -15,7 +15,7 @@ GNU General Public License for more details.
You should have received a copy of the GNU General Public License
along with this program. If not, see <http://www.gnu.org/licenses/>.
-->
<android.support.v4.widget.DrawerLayout xmlns:android="http://schemas.android.com/apk/res/android"
<org.floens.chan.ui.layout.DrawerWidthAdjustingLayout xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:tools="http://schemas.android.com/tools"
android:id="@+id/drawer_layout"
android:layout_width="match_parent"
@ -28,7 +28,7 @@ along with this program. If not, see <http://www.gnu.org/licenses/>.
<org.floens.chan.ui.view.TouchBlockingLinearLayout
android:id="@+id/drawer"
android:layout_width="200dp"
android:layout_width="0dp"
android:layout_height="match_parent"
android:layout_gravity="left"
android:background="?backcolor"
@ -89,4 +89,4 @@ along with this program. If not, see <http://www.gnu.org/licenses/>.
</org.floens.chan.ui.view.TouchBlockingLinearLayout>
</android.support.v4.widget.DrawerLayout>
</org.floens.chan.ui.layout.DrawerWidthAdjustingLayout>

Loading…
Cancel
Save