Summary: | Add FileSystemSync implementation for Worker | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Kinuko Yasuda <kinuko> | ||||||||||||
Component: | WebCore Misc. | Assignee: | Nobody <webkit-unassigned> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | ericu, levin, michaeln | ||||||||||||
Priority: | P2 | ||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||
Hardware: | PC | ||||||||||||||
OS: | OS X 10.5 | ||||||||||||||
Bug Depends on: | 46405 | ||||||||||||||
Bug Blocks: | |||||||||||||||
Attachments: |
|
Description
Kinuko Yasuda
2010-10-01 21:59:06 PDT
Created attachment 69568 [details]
Patch
Created attachment 69570 [details]
Patch
Created attachment 69761 [details]
Patch
Fixed nits.
Comment on attachment 69761 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=69761&action=review Just the comments from my phase 1 pass: nits. > WebCore/fileapi/DOMFileSystemBase.h:48 > +class EntriesCallback; out of order. > WebCore/fileapi/DOMFileSystemBase.h:74 > + bool readDirectory(DirectoryReaderBase* reader, const String& path, PassRefPtr<EntriesCallback>, PassRefPtr<ErrorCallback>); s/src/source/ Remove param names that add no information (entry, base, reader). > WebCore/fileapi/Entry.h:45 > +class EntrySync; out of order. > WebCore/fileapi/EntryArraySync.h:51 > + static PassRefPtr<EntryArraySync> create(EntryArray* entries); remove param name. > WebCore/fileapi/EntryBase.h:28 > + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ Mistaken change? > WebCore/fileapi/EntrySync.h:50 > + static PassRefPtr<EntrySync> create(EntryBase* entry); param name. Comment on attachment 69761 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=69761&action=review Just a few things to address. > WebCore/fileapi/DirectoryEntry.cpp:54 > + filesystem()->scheduleCallback(errorCallback, FileError::create(error)); Please get rid of the macro. > WebCore/fileapi/Entry.cpp:51 > + filesystem()->scheduleCallback(errorCallback, FileError::create(error)); Please get rid of the macro. >> WebCore/fileapi/EntrySync.h:50 >> + static PassRefPtr<EntrySync> create(EntryBase* entry); > > param name. (remove the param name) > WebCore/fileapi/FileEntry.h:-42 > -class DOMFileSystem; Why didn't this become DOMFileSystemBase? > WebCore/platform/AsyncFileSystem.h:63 > + // This should return false if there're no pending operations. s/there're/there are/ > WebCore/workers/WorkerContext.h:51 > + class DOMFileSystemSync; out of order. > WebKit/chromium/src/WorkerAsyncFileSystemChromium.cpp:77 > + return false; It seems like m_bridgeForLastOperation should be cleared at the end of this. If that is true, then consider RefPtr<> bridge = m_bridge.release(); and use bridge. Now bridge will get deref'ed when this function is exited. > WebKit/chromium/src/WorkerAsyncFileSystemChromium.h:62 > + virtual bool waitCompletion(); s/waitCompletion/waitForOperationToComplete/ > WebKit/chromium/src/WorkerAsyncFileSystemChromium.h:82 > + RefPtr<WebKit::WorkerFileSystemCallbacksBridge> m_bridgeForLastOperation; m_bridgeForCurrentOperation? Created attachment 69899 [details]
Patch
Created attachment 69901 [details]
Patch
Comment on attachment 69901 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=69899&action=review Just a few nits to address. > WebCore/fileapi/DOMFileSystemBase.h:72 > + bool getFile(const EntryBase* base, const String& path, PassRefPtr<Flags>, PassRefPtr<EntryCallback>, PassRefPtr<ErrorCallback>); remove "base" > WebCore/fileapi/DOMFileSystemBase.h:73 > + bool getDirectory(const EntryBase* base, const String& path, PassRefPtr<Flags>, PassRefPtr<EntryCallback>, PassRefPtr<ErrorCallback>); remove "base" > WebCore/fileapi/FileEntry.h:-42 > -class DOMFileSystem; Please consider adding this back but as Base. Committed r69249: <http://trac.webkit.org/changeset/69249> |