[Webkit-unassigned] [Bug 47535] DOMFileSystem's reference should be kept while there're any active Entries/callbacks

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=47535


Dumitru Daniliuc <dumi at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #71966|review?                     |review+, commit-queue-
               Flag|                            |




--- Comment #10 from Dumitru Daniliuc <dumi at chromium.org>  2010-10-27 15:49:23 PST ---
(From update of attachment 71966)
View in context: https://bugs.webkit.org/attachment.cgi?id=71966&action=review

r=me, but please address my comments before submitting.

> LayoutTests/fast/filesystem/script-tests/filesystem-reference.js:44
> +function runTest1(root)

nit/optional: i'd make runTest2() come after runTest1(), and runTest3() come after runTest2(). this way, it's easier (i think) to read the test from top to bottom.

> WebCore/fileapi/DOMFileSystemBase.cpp:38
> +#include "DirectoryReaderBase.h"

why do we need this #include? Changing DirectoryReaderBase* to PassRefPtr<DirectoryReaderBase> shouldn't require any additional #includes (and the list of #includes in DOMFileSystemBase.h didn't change either).

> WebCore/fileapi/DirectoryEntry.cpp:51
> +    return DirectoryReader::create(filesystem(), m_fullPath);

i think we should be consistent in how we use the fields declared in base classes (EntryBase): either keep them protected, and use m_fileSystem.get() and m_fullPath here, or we should make them private and add public/protected getters and use filesystem() and fullPath() here.

> WebCore/fileapi/DirectoryEntrySync.cpp:51
> +    return DirectoryReaderSync::create(filesystem(), m_fullPath);

same comment: let's make this consistent; either use the members directly, or provide getters for all of them.

> WebCore/fileapi/EntrySync.cpp:50
> +        return adoptRef(new FileEntrySync(entry->filesystem(), entry->m_fullPath));

ditto.

> WebCore/fileapi/EntrySync.cpp:102
> +    return DirectoryEntrySync::create(filesystem(), parentPath);

ditto.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.