Handle has_more correctly

Source/modules/filesystem/DirectoryReader.cpp

Index: Source/modules/filesystem/DirectoryReader.cpp
diff --git a/Source/modules/filesystem/DirectoryReader.cpp b/Source/modules/filesystem/DirectoryReader.cpp
index 338d1081b54aca89ef60d13bd3309c1358fb308a..eacf6cef5ce34aedbe81c08d5ce3765a48520bfa 100644
--- a/Source/modules/filesystem/DirectoryReader.cpp
+++ b/Source/modules/filesystem/DirectoryReader.cpp
@@ -38,19 +38,88 @@
 
 namespace WebCore {
 
+class DirectoryReader::EntriesCallbackHelper : public EntriesCallback {
+public:
+    EntriesCallbackHelper(PassRefPtr<DirectoryReader> reader)
+        : m_reader(reader)
+    {
+    }
+
+    virtual void handleEvent(const Vector<RefPtr<Entry> >& entries) OVERRIDE
+    {
+        m_reader->addEntries(entries);
+    }
+
+private:
+    RefPtr<DirectoryReader> m_reader;
+};
+
+class DirectoryReader::ErrorCallbackHelper : public ErrorCallback {
+public:
+    ErrorCallbackHelper(PassRefPtr<DirectoryReader> reader)
+        : m_reader(reader)
+    {
+    }
+
+    virtual void handleEvent(FileError* error) OVERRIDE
+    {
+        m_reader->onError(error);
+    }
+
+private:
+    RefPtr<DirectoryReader> m_reader;

[kinuko, 2014/03/06 13:13:38]

So in the current impl this callback (and DirectoryReader and DOMFileSystem)
will be kept alive as long as browser side's returning entries with
hasMore=true, whether it's desirable or not, is that right?  The new behavior is
at least much better than the current one, but it's probably worth having some
FIXME (and file a bug) to make it cancellable?

[hashimoto, 2014/03/07 05:21:26]

Done.

Please note that EntriesCallbacks (beware of 's', not EntriesCallback inherited
above) also keeps RefPtr<DirectoryReaderBase>.

+};
+
 DirectoryReader::DirectoryReader(PassRefPtr<DOMFileSystemBase> fileSystem, const String& fullPath)
     : DirectoryReaderBase(fileSystem, fullPath)
+    , m_isReading(false)
 {
     ScriptWrappable::init(this);
 }
 
 void DirectoryReader::readEntries(PassOwnPtr<EntriesCallback> entriesCallback, PassOwnPtr<ErrorCallback> errorCallback)
 {
-    if (!m_hasMoreEntries) {
-        filesystem()->scheduleCallback(entriesCallback, EntryVector());
+    if (!m_isReading) {
+        m_isReading = true;
+        filesystem()->readDirectory(this, m_fullPath, adoptPtr(new EntriesCallbackHelper(this)), adoptPtr(new ErrorCallbackHelper(this)));
+    }
+
+    if (m_error) {
+        filesystem()->scheduleCallback(errorCallback, m_error.get());
+        return;
+    }
+
+    if (m_entriesCallback) {

[kinuko, 2014/03/06 13:13:38]

nit: can you add a comment that non-null m_entriesCallback indicates multiple
concurrent readEntries() calls and we disallow it?

[hashimoto, 2014/03/07 05:21:26]

Done.

+        filesystem()->scheduleCallback(errorCallback, FileError::create(FileError::INVALID_STATE_ERR));
         return;
     }
-    filesystem()->readDirectory(this, m_fullPath, entriesCallback, errorCallback);
+
+    if (!m_hasMoreEntries || !m_entries.isEmpty()) {
+        filesystem()->scheduleCallback(entriesCallback, m_entries);
+        m_entries.clear();
+        return;
+    }
+
+    m_entriesCallback = entriesCallback;
+    m_errorCallback = errorCallback;
+}
+
+void DirectoryReader::addEntries(const Vector<RefPtr<Entry> >& entries)
+{
+    m_entries.appendVector(entries);
+    if (m_entriesCallback) {
+        OwnPtr<EntriesCallback> entriesCallback = m_entriesCallback.release();
+        Vector<RefPtr<Entry> > entries;
+        entries.swap(m_entries);
+        entriesCallback->handleEvent(entries);
+    }
+}
+
+void DirectoryReader::onError(FileError* error)
+{
+    m_error = error;
+    if (m_errorCallback)
+        m_errorCallback->handleEvent(error);
 }
 
 }