Handle has_more correctly

Source/modules/filesystem/DirectoryReader.cpp

 /*
  * Copyright (C) 2010 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 20 matching lines @@
 #include "config.h"
 #include "modules/filesystem/DirectoryReader.h"
 
 #include "core/fileapi/FileError.h"
 #include "modules/filesystem/EntriesCallback.h"
 #include "modules/filesystem/Entry.h"
 #include "modules/filesystem/ErrorCallback.h"
 
 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;
     }
-    filesystem()->readDirectory(this, m_fullPath, entriesCallback, errorCallback);
+
+    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;
+    }
+
+    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);
 }
 
 }