WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
47535
DOMFileSystem's reference should be kept while there're any active Entries/callbacks
https://bugs.webkit.org/show_bug.cgi?id=47535
Summary
DOMFileSystem's reference should be kept while there're any active Entries/ca...
Kinuko Yasuda
Reported
2010-10-11 21:42:52 PDT
DOMFileSystem's reference should be kept while there're any pending Entries/callbacks. Right now the reference needs to be kept in the script -- otherwise it crashes.
Attachments
Patch
(24.93 KB, patch)
2010-10-21 10:32 PDT
,
Kinuko Yasuda
no flags
Details
Formatted Diff
Diff
Patch
(26.72 KB, patch)
2010-10-21 17:14 PDT
,
Kinuko Yasuda
no flags
Details
Formatted Diff
Diff
Patch
(26.71 KB, patch)
2010-10-26 16:51 PDT
,
Kinuko Yasuda
dumi
: review+
dumi
: commit-queue-
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Kinuko Yasuda
Comment 1
2010-10-11 21:46:31 PDT
Or Entries and callbacks should hold a reference to the DOMFileSystem in a proper way.
Michael Nordman
Comment 2
2010-10-13 15:22:47 PDT
FYI: There is an easy workaround to the crashes, hold a script reference to the resulting fileSystem object returned as a result of the requestFileSystem() call.
Kinuko Yasuda
Comment 3
2010-10-21 10:32:00 PDT
Created
attachment 71455
[details]
Patch
Eric U.
Comment 4
2010-10-21 16:26:24 PDT
Comment on
attachment 71455
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=71455&action=review
> WebCore/fileapi/FileSystemCallbacks.cpp:141 > +PassOwnPtr<EntriesCallbacks> EntriesCallbacks::create(PassRefPtr<EntriesCallback> successCallback, PassRefPtr<ErrorCallback> errorCallback, PassRefPtr<DirectoryReaderBase> directoryReader, const String& basePath)
What about EntryCallbacks?
Kinuko Yasuda
Comment 5
2010-10-21 17:14:35 PDT
Created
attachment 71512
[details]
Patch
Kinuko Yasuda
Comment 6
2010-10-21 17:16:16 PDT
(In reply to
comment #4
)
> (From update of
attachment 71455
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=71455&action=review
> > > WebCore/fileapi/FileSystemCallbacks.cpp:141 > > +PassOwnPtr<EntriesCallbacks> EntriesCallbacks::create(PassRefPtr<EntriesCallback> successCallback, PassRefPtr<ErrorCallback> errorCallback, PassRefPtr<DirectoryReaderBase> directoryReader, const String& basePath) > > What about EntryCallbacks?
Forgotten... fixed.
Eric U.
Comment 7
2010-10-21 17:37:19 PDT
Comment on
attachment 71512
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=71512&action=review
LGTM
> LayoutTests/fast/filesystem/script-tests/filesystem-reference.js:8 > + testFailed("Error occured: " + error.code);
typo: occurred
Kinuko Yasuda
Comment 8
2010-10-26 16:51:35 PDT
Created
attachment 71966
[details]
Patch Fixed the typo.
Kinuko Yasuda
Comment 9
2010-10-26 16:55:18 PDT
Adding 'real' reviewers... could someone take a look at it?
Dumitru Daniliuc
Comment 10
2010-10-27 15:49:23 PDT
Comment on
attachment 71966
[details]
Patch 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.
Kinuko Yasuda
Comment 11
2010-10-27 20:18:31 PDT
Committed
r70741
: <
http://trac.webkit.org/changeset/70741
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug