[Webkit-unassigned] [Bug 44734] Add LocalFileSystem.requestFileSystem interface to DOMWindow

[bugzilla-daemon at webkit.org]

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





--- Comment #13 from Kinuko Yasuda <kinuko at chromium.org>  2010-08-31 17:05:16 PST ---
(In reply to comment #10)
> (From update of attachment 66121 [details])
> > WebCore/bindings/generic/RuntimeEnabledFeatures.h:141
> > +    static bool requestFileSystemEnabled() { return isFileSystemEnabled; }
> I still don't understand your comment. requestFileSystemEnabled() seems to be identical to fileSystemEnabled(). By looking into the name, I do not know what requestFileSystemEnabled is used for.

(I think Michael's comment is explaining things better...)

requestFileSystemEnabled() is there to dynamically enable requestFileSystem() interface defined in DOMWindow.idl annotated with [EnabledAtRuntime].   fileSystemEnabled() and isFileSystemEnabled() is supposed to be a flag for the entire feature and requestFileSystem is one of the feature's methods that needs to be exposed if the feature is enabled.

If we have more methods to be enabled at runtime for the feature (actually the FileSystem API defines one more DOMWindow method that hasn't been added yet) we'll have similar xxxEnabled() methods that will look identical to fileSystemEnabled(). 
(And again, I don't have strong opinion here and basically am trying to follow other code.)

> > WebCore/page/DOMWindow.cpp:742
> > +    if (!document) {
> Can this check be moved to the beginning of this method? That is, can we still call m_localFileSystem->requestFileSystem if document is gone?

Do you mean we shouldn't call requestFileSystem if the document is gone?
That's true I moved the check to the beginning.

> > WebCore/page/DOMWindow.cpp:759
> > +    m_localFileSystem->requestFileSystem(document, static_cast<AsyncFileSystem::Type>(type), size, successCallback, errorCallback);
> This could be merged with the first call of "m_localFileSystem->requestFileSystem", like:

Fixed.

> > WebCore/page/DOMWindow.h:86
> > +    class LocalFileSystem;
> Please move all these forward declarations to the right place.

Fixed.

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