diff --git a/epoxy-adapter/src/main/java/com/airbnb/epoxy/EpoxyController.java b/epoxy-adapter/src/main/java/com/airbnb/epoxy/EpoxyController.java index 874d5e65c0..c1b55d50e0 100644 --- a/epoxy-adapter/src/main/java/com/airbnb/epoxy/EpoxyController.java +++ b/epoxy-adapter/src/main/java/com/airbnb/epoxy/EpoxyController.java @@ -3,10 +3,13 @@ import android.os.Bundle; import android.os.Handler; import android.os.Looper; +import android.support.annotation.IntDef; import android.support.annotation.Nullable; import android.support.v7.widget.GridLayoutManager.SpanSizeLookup; import android.support.v7.widget.RecyclerView; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; import java.util.ArrayList; import java.util.HashSet; import java.util.List; @@ -57,16 +60,36 @@ public abstract class EpoxyController { private Timer timer = NO_OP_TIMER; private EpoxyDiffLogger debugObserver; private boolean hasBuiltModelsEver; - private boolean hasPendingModelBuildRequest; private List modelInterceptorCallbacks; private int recyclerViewAttachCount = 0; private EpoxyModel stagedModel; + /** + * Posting and canceling runnables is a bit expensive - it is synchronizes and iterates the the + * list of runnables. We want clients to be able to request model builds as often as they want and + * have it act as a no-op if one is already requested, without being a performance hit. To do that + * we track whether we have a call to build models posted already so we can avoid canceling a + * current call and posting it again. + */ + @RequestedModelBuildType private int requestedModelBuildType = RequestedModelBuildType.NONE; + + @Retention(RetentionPolicy.SOURCE) + @IntDef({RequestedModelBuildType.NONE, + RequestedModelBuildType.NEXT_FRAME, + RequestedModelBuildType.DELAYED}) + private @interface RequestedModelBuildType { + int NONE = 0; + /** A request has been made to build models immediately. It is posted. */ + int NEXT_FRAME = 1; + /** A request has been made to build models after a delay. It is post delayed. */ + int DELAYED = 2; + } + /** * Call this to request a model update. The controller will schedule a call to {@link * #buildModels()} so that models can be rebuilt for the current data. Once a build is requested - * all subsequent requests are ignored until the model build run. Therefore, the calling code need - * not worry about calling this multiple times in a row. + * all subsequent requests are ignored until the model build runs. Therefore, the calling code + * need not worry about calling this multiple times in a row. *

* The exception is that the first time this is called on a new instance of {@link * EpoxyController} it is run synchronously. This allows state to be restored and the initial view @@ -85,8 +108,7 @@ public void requestModelBuild() { if (hasBuiltModelsEver) { requestDelayedModelBuild(0); } else { - cancelPendingModelBuild(); - dispatchModelBuild(); + buildModelsRunnable.run(); } } @@ -100,14 +122,14 @@ public void requestModelBuild() { * delaying the model build too long is that models will not be in sync with the data or view, and * scrolling the view offscreen and back onscreen will cause the model to bind old data. *

- * If a model build was previously requested it will run as originally scheduled, and this new - * request will be dropped. + * If a previous request is still pending it will be removed in favor of this new delay + *

+ * Any call to {@link #requestModelBuild()} will override a delayed request. *

* In most cases you should use {@link #requestModelBuild()} instead of this. * * @param delayMs The time in milliseconds to delay the model build by. Should be greater than or - * equal to 0. Even if a delay of 0 is given the model build will be posted to the - * next frame. + * equal to 0. A value of 0 is equivalent to calling {@link #requestModelBuild()} */ public void requestDelayedModelBuild(int delayMs) { if (isBuildingModels()) { @@ -115,10 +137,16 @@ public void requestDelayedModelBuild(int delayMs) { "Cannot call `requestDelayedModelBuild` from inside `buildModels`"); } - if (!hasPendingModelBuildRequest) { - hasPendingModelBuildRequest = true; - handler.postDelayed(buildModelsRunnable, delayMs); + if (requestedModelBuildType == RequestedModelBuildType.DELAYED) { + cancelPendingModelBuild(); + } else if (requestedModelBuildType == RequestedModelBuildType.NEXT_FRAME) { + return; } + + requestedModelBuildType = + delayMs == 0 ? RequestedModelBuildType.NEXT_FRAME : RequestedModelBuildType.DELAYED; + + handler.postDelayed(buildModelsRunnable, delayMs); } /** @@ -126,39 +154,37 @@ public void requestDelayedModelBuild(int delayMs) { * #requestModelBuild()}. */ public void cancelPendingModelBuild() { - hasPendingModelBuildRequest = false; - handler.removeCallbacks(buildModelsRunnable); + if (requestedModelBuildType != RequestedModelBuildType.NONE) { + requestedModelBuildType = RequestedModelBuildType.NONE; + handler.removeCallbacks(buildModelsRunnable); + } } private final Runnable buildModelsRunnable = new Runnable() { @Override public void run() { - dispatchModelBuild(); - } - }; - - private void dispatchModelBuild() { - hasPendingModelBuildRequest = false; - helper.resetAutoModels(); + cancelPendingModelBuild(); + helper.resetAutoModels(); - modelsBeingBuilt = new ControllerModelList(getExpectedModelCount()); + modelsBeingBuilt = new ControllerModelList(getExpectedModelCount()); - timer.start(); - buildModels(); - addCurrentlyStagedModelIfExists(); - timer.stop("Models built"); + timer.start(); + buildModels(); + addCurrentlyStagedModelIfExists(); + timer.stop("Models built"); - runInterceptors(); - filterDuplicatesIfNeeded(modelsBeingBuilt); - modelsBeingBuilt.freeze(); + runInterceptors(); + filterDuplicatesIfNeeded(modelsBeingBuilt); + modelsBeingBuilt.freeze(); - timer.start(); - adapter.setModels(modelsBeingBuilt); - timer.stop("Models diffed"); + timer.start(); + adapter.setModels(modelsBeingBuilt); + timer.stop("Models diffed"); - modelsBeingBuilt = null; - hasBuiltModelsEver = true; - } + modelsBeingBuilt = null; + hasBuiltModelsEver = true; + } + }; /** An estimate for how many models will be built in the next {@link #buildModels()} phase. */ private int getExpectedModelCount() { diff --git a/epoxy-adapter/src/test/java/com/airbnb/epoxy/TypedEpoxyControllerTest.java b/epoxy-adapter/src/test/java/com/airbnb/epoxy/TypedEpoxyControllerTest.java index 0e57ca2210..75f58c5502 100644 --- a/epoxy-adapter/src/test/java/com/airbnb/epoxy/TypedEpoxyControllerTest.java +++ b/epoxy-adapter/src/test/java/com/airbnb/epoxy/TypedEpoxyControllerTest.java @@ -4,24 +4,34 @@ import org.junit.runner.RunWith; import org.robolectric.annotation.Config; -import static org.mockito.Mockito.spy; -import static org.mockito.Mockito.verify; +import static junit.framework.Assert.assertEquals; @Config(sdk = 21, manifest = TestRunner.MANIFEST_PATH) @RunWith(TestRunner.class) public class TypedEpoxyControllerTest { + static class TestTypedController extends TypedEpoxyController { + int numTimesBuiltModels = 0; + + @Override + protected void buildModels(String data) { + assertEquals("data", data); + numTimesBuiltModels++; + } + } + @Test public void setData() { - TypedEpoxyController controller = spy(new TypedEpoxyController() { - @Override - protected void buildModels(String data) { + TestTypedController controller = new TestTypedController(); - } - }); + controller.setData("data"); + controller.setData("data"); + + controller.cancelPendingModelBuild(); + controller.setData("data"); controller.setData("data"); - verify(controller).buildModels("data"); + assertEquals(4, controller.numTimesBuiltModels); } } \ No newline at end of file