diff --git a/app/src/main/java/com/httrack/android/HTTrackActivity.java b/app/src/main/java/com/httrack/android/HTTrackActivity.java index c92d610..057e1af 100755 --- a/app/src/main/java/com/httrack/android/HTTrackActivity.java +++ b/app/src/main/java/com/httrack/android/HTTrackActivity.java @@ -226,6 +226,20 @@ protected static class VERSION_CODES { // Handler to execute code in UI thread private final Handler handlerUI = new Handler(); + // The engine refreshes faster than the main thread can draw, so only the newest frame is queued + private final ProgressCoalescer progressLines = new ProgressCoalescer(); + + private final Runnable progressLinesTask = new Runnable() { + @Override + public void run() { + final String[] lines = progressLines.take(); + // Defensive: only a disarm after the post could empty the coalescer, and nothing live does. + if (lines != null) { + setProgressLinesInternal(lines); + } + } + }; + // Interrupt was requested protected boolean interruptRequested; @@ -2380,12 +2394,10 @@ private void removeLinesFromLayout(final LinearLayout layout, * Set the "progress" layout lines. To be run in any thread. */ protected void setProgressLines(final String[] lines) { - handlerUI.post(new Runnable() { - @Override - public void run() { - setProgressLinesInternal(lines); - } - }); + if (progressLines.offerNeedsPost(lines) && !handlerUI.post(progressLinesTask)) { + // Unreachable short of process death, since post() only refuses once the Looper quits. + progressLines.disarm(); + } } /* diff --git a/app/src/main/java/com/httrack/android/ProgressCoalescer.java b/app/src/main/java/com/httrack/android/ProgressCoalescer.java new file mode 100644 index 0000000..6f43b60 --- /dev/null +++ b/app/src/main/java/com/httrack/android/ProgressCoalescer.java @@ -0,0 +1,44 @@ +package com.httrack.android; + +/** + * Holds at most one pending update, always the newest. One posted task per engine refresh grows + * the main thread's queue until the app stops answering input. The frames dropped in between cost + * nothing, because each one replaces the whole progress pane. + */ +final class ProgressCoalescer { + /** Holds the frame to draw next, and is null exactly when no task has been posted. */ + private T pending; + + /** + * Keep this payload as the one to draw next, and say whether a task must now be posted. + * + * @param payload the newest progress, never null, since a null would read as nothing pending + * @return true when the caller must schedule the drawing task, false when a pending one will + * carry this payload instead + */ + synchronized boolean offerNeedsPost(final T payload) { + if (payload == null) { + throw new NullPointerException("payload"); + } + final boolean needsPost = pending == null; + pending = payload; + return needsPost; + } + + /** + * Take the payload to draw, and let the next offer schedule again. Clear before the drawing, not + * after, so a refresh landing mid-draw gets a task of its own. + * + * @return the newest payload, or null when nothing is pending + */ + synchronized T take() { + final T payload = pending; + disarm(); + return payload; + } + + /** Drop whatever is pending and let the next offer schedule again. */ + synchronized void disarm() { + pending = null; + } +} diff --git a/app/src/test/java/com/httrack/android/ProgressCoalescerTest.java b/app/src/test/java/com/httrack/android/ProgressCoalescerTest.java new file mode 100644 index 0000000..79120cc --- /dev/null +++ b/app/src/test/java/com/httrack/android/ProgressCoalescerTest.java @@ -0,0 +1,172 @@ +package com.httrack.android; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.concurrent.atomic.AtomicInteger; +import org.junit.Test; + +/** + * Verifies the queue discipline setProgressLines follows, with a counter standing in for the + * untestable looper. + */ +public class ProgressCoalescerTest { + /** Every drain pattern over the first six frames of a run. */ + private static final int SCHEDULES = 64; + private static final int FRAMES_PER_SCHEDULE = 20; + private static final int CONCURRENT_FRAMES = 5000; + + private static String frame(final int index) { + return "frame " + index; + } + + /** Posting, and drawing split in two, so a refresh can be timed to land mid-draw. */ + private static final class FakeUiThread { + final ProgressCoalescer coalescer = new ProgressCoalescer(); + + /** What the drawing task has put on screen, oldest first. */ + final List drawn = new ArrayList(); + + /** Tasks posted and not yet run, counted atomically because the crawl thread offers too. */ + private final AtomicInteger postedTasks = new AtomicInteger(); + + void refresh(final String payload) { + if (coalescer.offerNeedsPost(payload)) { + postedTasks.incrementAndGet(); + } + } + + int postedTasks() { + return postedTasks.get(); + } + + String startDraw() { + postedTasks.decrementAndGet(); + return coalescer.take(); + } + + void endDraw(final String payload) { + if (payload != null) { + drawn.add(payload); + } + } + + void runOneTask() { + endDraw(startDraw()); + } + + void drain() { + while (postedTasks() > 0) { + runOneTask(); + } + } + + String lastDrawn() { + return drawn.get(drawn.size() - 1); + } + } + + @Test + public void aBurstOfRefreshesPostsOneTaskCarryingTheLastOne() { + final FakeUiThread ui = new FakeUiThread(); + for (int i = 0; i < 5; i++) { + ui.refresh(frame(i)); + } + assertEquals(1, ui.postedTasks()); + ui.drain(); + assertEquals(Arrays.asList(frame(4)), ui.drawn); + } + + @Test + public void aTaskThatHasRunLetsTheNextRefreshPostAgain() { + final FakeUiThread ui = new FakeUiThread(); + final List expected = new ArrayList(); + for (int i = 0; i < 100; i++) { + ui.refresh(frame(i)); + assertEquals(1, ui.postedTasks()); + ui.drain(); + expected.add(frame(i)); + } + assertEquals(expected, ui.drawn); + } + + @Test + public void aRefreshArrivingMidDrawGetsATaskOfItsOwn() { + final FakeUiThread ui = new FakeUiThread(); + ui.refresh("first"); + final String drawing = ui.startDraw(); + ui.refresh("last"); + assertEquals(1, ui.postedTasks()); + ui.endDraw(drawing); + ui.drain(); + assertEquals(Arrays.asList("first", "last"), ui.drawn); + } + + /** Whatever the engine and the looper do, the frame nothing follows must reach the screen. */ + @Test + public void theLastFrameIsDrawnWhateverTheInterleaving() { + for (int schedule = 0; schedule < SCHEDULES; schedule++) { + final FakeUiThread ui = new FakeUiThread(); + int drainAfter = schedule; + for (int i = 0; i < FRAMES_PER_SCHEDULE; i++) { + ui.refresh(frame(i)); + if (drainAfter % 2 == 0) { + ui.runOneTask(); + } + drainAfter /= 2; + } + ui.drain(); + assertEquals("schedule " + schedule, frame(FRAMES_PER_SCHEDULE - 1), ui.lastDrawn()); + } + } + + @Test + public void aTaskWhoseFrameWasAlreadyDrawnDrawsNothing() { + final ProgressCoalescer coalescer = new ProgressCoalescer(); + assertNull(coalescer.take()); + coalescer.offerNeedsPost("frame"); + assertEquals("frame", coalescer.take()); + assertNull(coalescer.take()); + } + + /** What the failed-post branch of setProgressLines rests on. */ + @Test + public void disarmingDropsThePendingFrameAndLetsTheNextOnePost() { + final ProgressCoalescer coalescer = new ProgressCoalescer(); + assertTrue(coalescer.offerNeedsPost("stranded")); + coalescer.disarm(); + assertNull(coalescer.take()); + assertTrue(coalescer.offerNeedsPost("next")); + assertEquals("next", coalescer.take()); + } + + /** A null frame would read as no task pending, and post a second one over the first. */ + @Test(expected = NullPointerException.class) + public void aNullFrameIsRefused() { + new ProgressCoalescer().offerNeedsPost(null); + } + + @Test + public void theCrawlThreadAndTheUiThreadAgreeOnTheLastFrame() throws InterruptedException { + final FakeUiThread ui = new FakeUiThread(); + final Thread crawl = new Thread(new Runnable() { + @Override + public void run() { + for (int i = 0; i < CONCURRENT_FRAMES; i++) { + ui.refresh(frame(i)); + } + } + }); + crawl.start(); + while (crawl.isAlive()) { + ui.drain(); + } + crawl.join(); + ui.drain(); + assertEquals(frame(CONCURRENT_FRAMES - 1), ui.lastDrawn()); + } +}