[Webkit-unassigned] [Bug 58443] [GTK] Support the file system API

[bugzilla-daemon at webkit.org]

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





--- Comment #15 from Gustavo Noronha (kov) <gns at gnome.org>  2012-04-05 18:39:12 PST ---
(From update of attachment 135795)
View in context: https://bugs.webkit.org/attachment.cgi?id=135795&action=review

OK, I'm starting to grasp this code a bit more. I still need to do some more reading of the spec and understand the interaction with ScriptController, I'll probably be able to do it over the weekend, I have a few comments for now:

> Source/WebCore/Modules/filesystem/LocalFileSystem.cpp:55
> +#if PLATFORM(GTK)
> +#include "AsyncFileSystemCallbacksGtk.h"
> +#endif
> +

We're essentially replacing the whole file for GTK+, I don't see any reason to use #if PLATFORM(GTK) in this file at all, better do like Chromium: http://trac.webkit.org/browser/trunk/Source/WebCore/Modules/filesystem/LocalFileSystem.cpp#L34

> Source/WebCore/platform/gtk/AsyncFileSystemCallbacksGtk.cpp:3
> + * Copyright (C) 2012 ChangSeok Oh <shivamidow at gmail.com>

This file seems to be essentially unchanged from Zan's patch, except for some code shuffling, so I think he should be the only copyright holder.

> Source/WebCore/platform/gtk/AsyncFileSystemCallbacksGtk.h:3
> + * Copyright (C) 2012 ChangSeok Oh <shivamidow at gmail.com>

Ditto. Please check for more such cases =)

> Source/WebCore/platform/gtk/AsyncFileSystemGtk.cpp:34
> +#include "LocalFileSystem.h"
>  #include "NotImplemented.h"
>  

Empty line, don't need NotImplemented.h anymore, too.

> LayoutTests/platform/gtk/Skipped:304
> +# Following tests are passed, if FileSystem API feature is enabled.

I think when we land this we should enable filesystem by default in build-webkit, so that it gets tested in the bots.

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