diff --git a/app/src/main/java/com/httrack/android/DirScanner.java b/app/src/main/java/com/httrack/android/DirScanner.java new file mode 100644 index 0000000..764d9d3 --- /dev/null +++ b/app/src/main/java/com/httrack/android/DirScanner.java @@ -0,0 +1,108 @@ +package com.httrack.android; + +import java.io.File; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.Executor; +import java.util.concurrent.atomic.AtomicInteger; + +/** + * Lists directories for the file chooser off the UI thread. A stat() per entry blows past the ANR + * budget on a large or slow directory. Only the newest scan is drawn, because a slower one started + * earlier would otherwise draw over it. + */ +final class DirScanner { + private static final File ROOT = new File("/"); + + /** Holds what one scan found. */ + static final class Listing { + /** Where the "up one level" entry leads, the filesystem root when there is nothing above. */ + final File parent; + + /** The visible sub-directories, in the order the filesystem listed them. */ + final List subdirectories; + + Listing(final File parent, final List subdirectories) { + this.parent = parent; + this.subdirectories = subdirectories; + } + } + + /** Takes the listing of the newest scan, on the UI executor. */ + interface Sink { + void onListing(Listing listing); + } + + private final Executor worker; + private final Executor ui; + + /** Counts the scans asked for, so a scan can tell whether it is still the newest. */ + private final AtomicInteger newest = new AtomicInteger(); + + DirScanner(final Executor worker, final Executor ui) { + this.worker = worker; + this.ui = ui; + } + + /** + * Scan a directory and hand the result to a sink. + * + * @param dir the directory to list, as the chooser currently has it, never null + * @param fallback listed instead when dir is gone, never null + * @param sink called on the UI executor, unless a later scan has superseded this one + */ + void scan(final File dir, final File fallback, final Sink sink) { + final int ticket = newest.incrementAndGet(); + worker.execute(new Runnable() { + @Override + public void run() { + if (ticket != newest.get()) { + return; + } + final Listing listing = list(dir, fallback); + ui.execute(new Runnable() { + @Override + public void run() { + if (ticket == newest.get()) { + sink.onListing(listing); + } + } + }); + } + }); + } + + /** + * Read the parent and the visible sub-directories of a directory. Private, so that the UI thread + * can reach a listing only through scan(). + * + * @param dir the directory to list, which may be gone + * @param fallback listed instead when dir is gone, which leaves the parent at the root, because + * a path that no longer exists says nothing about where it sat + * @return what the chooser must show + */ + private static Listing list(final File dir, final File fallback) { + final boolean usable = dir.exists(); + return new Listing(usable ? parentOf(dir) : ROOT, subdirectoriesOf(usable ? dir : fallback)); + } + + /** Returns the directory above dir, or the filesystem root when there is nothing above it. */ + private static File parentOf(final File dir) { + final File parent = dir.getParentFile(); + return parent != null ? parent : ROOT; + } + + /** Returns the sub-directories of dir the chooser offers, which excludes the hidden ones. */ + private static List subdirectoriesOf(final File dir) { + final List dirs = new ArrayList(); + final File[] list = dir.listFiles(); + if (list != null) { + for (final File file : list) { + if (file.isDirectory() && !file.isHidden()) { + dirs.add(file); + } + } + } + return dirs; + } +} diff --git a/app/src/main/java/com/httrack/android/FileChooserActivity.java b/app/src/main/java/com/httrack/android/FileChooserActivity.java index 2583289..861b90a 100755 --- a/app/src/main/java/com/httrack/android/FileChooserActivity.java +++ b/app/src/main/java/com/httrack/android/FileChooserActivity.java @@ -3,6 +3,7 @@ import java.io.File; import java.util.ArrayList; import java.util.List; +import java.util.concurrent.Executor; import android.os.Bundle; import android.os.Handler; @@ -33,6 +34,19 @@ public class FileChooserActivity extends Activity implements // Handler to execute code in UI thread private final Handler handlerUI = new Handler(); + /** Lists the directories to offer on a worker thread, then draws them through handlerUI. */ + private final DirScanner scanner = new DirScanner(new Executor() { + @Override + public void execute(final Runnable command) { + new Thread(command).start(); + } + }, new Executor() { + @Override + public void execute(final Runnable command) { + handlerUI.post(command); + } + }); + /* * Convert dp (Density-independent Pixels) to px (pixel) */ @@ -51,84 +65,80 @@ private void setFile(final File file) { } /* - * Create all tabs for main options menu. + * Ask for the current directory's listing. showDir draws it once it arrives. */ private void refreshDir() { - // Clear list - files.clear(); + scanner.scan(projectRootFile, defaultRootFile, new DirScanner.Sink() { + @Override + public void onListing(final DirScanner.Listing listing) { + showDir(listing); + } + }); + } - // Find parent - final File parent = projectRootFile != null && projectRootFile.exists() ? projectRootFile.getParentFile() : null; - if (parent != null && projectRootFile != null && !parent.getAbsolutePath().equals(projectRootFile.getAbsolutePath())) { - files.add(new Pair(getString(R.string.ellipsis), parent)); - Log.d("httrack", "found parent of " + projectRootFile.getAbsolutePath() + ": " + parent.getAbsolutePath()); - } else { - files.add(new Pair(getString(R.string.ellipsis), new File("/"))); - Log.d("httrack", "could not find found parent of " + projectRootFile.getAbsolutePath() + ", using /"); + /** Draws the listing as one button per entry. */ + private void showDir(final DirScanner.Listing listing) { + if (isDestroyed()) { + return; } - // List directory - final File[] list = projectRootFile != null && projectRootFile.exists() ? projectRootFile - .listFiles() : defaultRootFile.listFiles(); - if (list != null) { - for (final File file : list) { - if (file.isDirectory() && !file.isHidden() /* && file.canWrite() */) { - files.add(new Pair(file.getName(), file)); - } - } + // Clear list + files.clear(); + // Up one level, then the sub-directories + files.add(new Pair(getString(R.string.ellipsis), listing.parent)); + for (final File dir : listing.subdirectories) { + files.add(new Pair(dir.getName(), dir)); } - if (files != null) { - // Create tabs - final LinearLayout scroll = LinearLayout.class - .cast(findViewById(R.id.layout)); - scroll.removeAllViews(); - - // Add all buttons linking to options - for (int i = 0; i < files.size(); i++) { - // Add separator - if (borderless && i != 0) { - final View line = new View(this, null, R.style.DividerLineHorizontal); - - // FIXME TODO: why in hell isn't my shiny style NOT working ? - final LinearLayout.MarginLayoutParams layout = new LinearLayout.MarginLayoutParams( - LayoutParams.FILL_PARENT, dpToPx(1)); - layout.bottomMargin = dpToPx(8); - layout.topMargin = dpToPx(8); - line.setLayoutParams(layout); - line.setBackgroundColor(getResources().getColor(R.color.black)); - - // Add line - scroll.addView(line); - } + // Create tabs + final LinearLayout scroll = LinearLayout.class + .cast(findViewById(R.id.layout)); + scroll.removeAllViews(); + + // Add all buttons linking to options + for (int i = 0; i < files.size(); i++) { + // Add separator + if (borderless && i != 0) { + final View line = new View(this, null, R.style.DividerLineHorizontal); + + // FIXME TODO: why in hell isn't my shiny style NOT working ? + final LinearLayout.MarginLayoutParams layout = new LinearLayout.MarginLayoutParams( + LayoutParams.FILL_PARENT, dpToPx(1)); + layout.bottomMargin = dpToPx(8); + layout.topMargin = dpToPx(8); + line.setLayoutParams(layout); + line.setBackgroundColor(getResources().getColor(R.color.black)); + + // Add line + scroll.addView(line); + } - // Next tab class - final Pair entry = files.get(i); - final String title = entry.first; - - // Create a flat button ; see - // - // Added in API level 11 - final Button button = borderless ? new Button(this, null, - android.R.attr.borderlessButtonStyle) : new Button(this); - if (borderless) { - // Left-aligned text - button.setGravity(Gravity.LEFT | Gravity.CENTER_VERTICAL); - } + // Next tab class + final Pair entry = files.get(i); + final String title = entry.first; + + // Create a flat button ; see + // + // Added in API level 11 + final Button button = borderless ? new Button(this, null, + android.R.attr.borderlessButtonStyle) : new Button(this); + if (borderless) { + // Left-aligned text + button.setGravity(Gravity.LEFT | Gravity.CENTER_VERTICAL); + } - // Title - button.setText(title); + // Title + button.setText(title); - // Set listener - button.setOnClickListener(this); + // Set listener + button.setOnClickListener(this); - // Set tag to our index - button.setTag(i); + // Set tag to our index + button.setTag(i); - // And finally add button - scroll.addView(button); - } + // And finally add button + scroll.addView(button); } } diff --git a/app/src/test/java/com/httrack/android/DirScannerTest.java b/app/src/test/java/com/httrack/android/DirScannerTest.java new file mode 100644 index 0000000..dddbffd --- /dev/null +++ b/app/src/test/java/com/httrack/android/DirScannerTest.java @@ -0,0 +1,225 @@ +package com.httrack.android; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import java.io.File; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.concurrent.Executor; +import java.util.concurrent.atomic.AtomicInteger; + +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; + +/** Exercises what the file chooser offers, and the rule that only the newest scan is drawn. */ +public class DirScannerTest { + @Rule + public final TemporaryFolder tmp = new TemporaryFolder(); + + private static final Executor DIRECT = new Executor() { + @Override + public void execute(final Runnable command) { + command.run(); + } + }; + + /** Queues what it is handed, standing in for the worker thread and for the looper. */ + private static final class Deferred implements Executor { + private final List queued = new ArrayList(); + + @Override + public void execute(final Runnable command) { + queued.add(command); + } + + void runAll() { + final List batch = new ArrayList(queued); + queued.clear(); + for (final Runnable command : batch) { + command.run(); + } + } + } + + /** A directory whose listing the test controls: it counts the calls and can fix what they give. */ + private static final class CountedDir extends File { + private static final long serialVersionUID = 1L; + + final AtomicInteger listings = new AtomicInteger(); + + private final File[] contents; + + /** Lists the real directory, and counts. */ + CountedDir(final File dir) { + this(dir, null); + } + + /** Lists exactly these, in this order. */ + CountedDir(final File dir, final File[] contents) { + super(dir.getPath()); + this.contents = contents; + } + + @Override + public File[] listFiles() { + listings.incrementAndGet(); + return contents != null ? contents : super.listFiles(); + } + } + + private static final class Recorder implements DirScanner.Sink { + final List listings = new ArrayList(); + + @Override + public void onListing(final DirScanner.Listing listing) { + listings.add(listing); + } + } + + /** Scans with no thread and no looper, and returns what the sink was handed. */ + private static DirScanner.Listing listed(final File dir, final File fallback) { + final Recorder recorder = new Recorder(); + new DirScanner(DIRECT, DIRECT).scan(dir, fallback, recorder); + return recorder.listings.get(0); + } + + private static List names(final List files) { + final List names = new ArrayList(); + for (final File file : files) { + names.add(file.getName()); + } + return names; + } + + private static List sortedNames(final List files) { + final List names = names(files); + Collections.sort(names); + return names; + } + + private File folder(final String name, final String... children) throws Exception { + final File dir = tmp.newFolder(name); + for (final String child : children) { + assertTrue(new File(dir, child).mkdir()); + } + return dir; + } + + @Test + public void offersTheVisibleSubdirectoriesOnly() throws Exception { + final File dir = folder("base", "sub", ".hidden"); + assertTrue(new File(dir, "page.html").createNewFile()); + + final DirScanner.Listing listing = listed(dir, tmp.getRoot()); + assertEquals(Arrays.asList("sub"), sortedNames(listing.subdirectories)); + assertEquals(dir.getParentFile().getAbsolutePath(), listing.parent.getAbsolutePath()); + } + + /** listFiles() answers in no particular order, and the chooser offers that order unchanged. */ + @Test + public void keepsTheOrderTheFilesystemGave() throws Exception { + final File dir = folder("base", "a", "b", "c"); + final File[] reversed = { new File(dir, "c"), new File(dir, "b"), new File(dir, "a") }; + + final DirScanner.Listing listing = listed(new CountedDir(dir, reversed), tmp.getRoot()); + assertEquals(Arrays.asList("c", "b", "a"), names(listing.subdirectories)); + } + + /** Nothing above the directory means the root, both at "/" and for a directory that is gone. */ + @Test + public void theParentEntryFallsBackToTheRoot() throws Exception { + final File gone = tmp.newFolder("gone"); + assertTrue(gone.delete()); + + assertEquals("/", listed(new File("/"), tmp.getRoot()).parent.getAbsolutePath()); + assertEquals("/", listed(gone, tmp.getRoot()).parent.getAbsolutePath()); + } + + @Test + public void aDirectoryThatIsGoneIsListedFromTheFallback() throws Exception { + final File fallback = folder("default", "websites"); + final File gone = tmp.newFolder("gone"); + assertTrue(gone.delete()); + + assertEquals(Arrays.asList("websites"), sortedNames(listed(gone, fallback).subdirectories)); + } + + /** listFiles() answers null on anything but a directory, and that is not an empty listing. */ + @Test + public void aPlainFileOffersNothing() throws Exception { + final File file = tmp.newFile("note.txt"); + assertTrue(listed(file, tmp.getRoot()).subdirectories.isEmpty()); + } + + /** The listing reads a directory created after scan() returned, so it cannot have run inline. */ + @Test + public void scanReadsTheDirectoryAfterItReturns() throws Exception { + final Deferred worker = new Deferred(); + final Deferred ui = new Deferred(); + final Recorder recorder = new Recorder(); + final File dir = folder("base"); + + new DirScanner(worker, ui).scan(dir, dir, recorder); + assertTrue(recorder.listings.isEmpty()); + assertTrue(new File(dir, "late").mkdir()); + worker.runAll(); + ui.runAll(); + + assertEquals(Arrays.asList("late"), sortedNames(recorder.listings.get(0).subdirectories)); + } + + /** A listing finished before the next tap must not draw over the directory that tap opened. */ + @Test + public void aFinishedScanIsDroppedOnceAnotherIsAsked() throws Exception { + final Deferred worker = new Deferred(); + final Deferred ui = new Deferred(); + final Recorder recorder = new Recorder(); + final DirScanner scanner = new DirScanner(worker, ui); + final File first = folder("first", "in-first"); + final File second = folder("second", "in-second"); + + scanner.scan(first, first, recorder); + worker.runAll(); + scanner.scan(second, second, recorder); + worker.runAll(); + ui.runAll(); + + assertEquals(1, recorder.listings.size()); + assertEquals(Arrays.asList("in-second"), sortedNames(recorder.listings.get(0).subdirectories)); + } + + /** A scan still queued when the next tap arrives must not read the filesystem at all. */ + @Test + public void aQueuedScanIsNeverRunOnceAnotherIsAsked() throws Exception { + final Deferred worker = new Deferred(); + final Deferred ui = new Deferred(); + final Recorder recorder = new Recorder(); + final DirScanner scanner = new DirScanner(worker, ui); + final CountedDir first = new CountedDir(folder("first", "in-first")); + final File second = folder("second", "in-second"); + + scanner.scan(first, first, recorder); + scanner.scan(second, second, recorder); + worker.runAll(); + ui.runAll(); + + assertEquals(0, first.listings.get()); + assertEquals(Arrays.asList("in-second"), sortedNames(recorder.listings.get(0).subdirectories)); + } + + /** The activity owns no listing of its own, and never draws one into a destroyed window. */ + @Test + public void theChooserOnlyDrawsWhatTheScannerHandsIt() throws Exception { + final String source = TestSources.javaSource("FileChooserActivity"); + assertFalse("listing belongs to DirScanner, which runs off the UI thread", + source.contains("listFiles(") || source.contains(".list()")); + assertTrue("a listing can land after onDestroy, and the views are gone by then", + TestSources.between(source, "private void showDir", "files.clear()") + .contains("isDestroyed()")); + } +}