Bug 165562 – Move FileHandle to WebCore FileHandle.h

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Keith Rollin]

Bug 164527 introduces a FileHandle class into WebKit2/NetworkProcess. This should be moved to WebCore. While doing this, we should consider what this class should actually be. Should is just be a C++ wrapper around the plain functionality in FileSystem.h? Or should it provide more?

[Keith Rollin]

Created attachment 296498 [details]
Patch

[Alex Christensen]

Comment on attachment 296498 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=296498&action=review

> Source/WebCore/platform/FileHandle.cpp:40
> +    , m_fileHandle(invalidPlatformFileHandle)

This isn't necessary because of the initializer list in the header.

> Source/WebCore/platform/FileHandle.cpp:47
> +    , m_fileHandle(invalidPlatformFileHandle)

I think this is confusing.  Maybe we should = delete this constructor.

> Source/WebCore/platform/FileHandle.cpp:69
> +        m_fileHandle = invalidPlatformFileHandle;

ditto

[Keith Rollin]

(In reply to comment #2)
> > Source/WebCore/platform/FileHandle.cpp:47
> > +    , m_fileHandle(invalidPlatformFileHandle)
> 
> I think this is confusing.  Maybe we should = delete this constructor.
> 
> > Source/WebCore/platform/FileHandle.cpp:69
> > +        m_fileHandle = invalidPlatformFileHandle;
> 
> ditto

Heh. I concur. I originally had them "= delete", but tried adding them into this version in the interests of completeness.

[Keith Rollin]

Created attachment 296547 [details]
Patch

[Keith Rollin]

Created attachment 296555 [details]
Patch

[Keith Rollin]

Created attachment 296562 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 296562 [details]
Patch

Clearing flags on attachment: 296562

Committed r209583: <http://trac.webkit.org/changeset/209583>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Ryan Haddad]

This change appears to have broken the Windows build:

https://build.webkit.org/builders/Apple%20Win%20Release%20%28Build%29/builds/82532

C:\cygwin\home\buildbot\slave\win-release\build\Source\WebCore\platform\FileHandle.cpp(107): error C3861: 'vasprintf': identifier not found [C:\cygwin\home\buildbot\slave\win-release\build\WebKitBuild\Release\Source\WebCore\WebCore.vcxproj]

[Keith Rollin]

OK, I'll fix it if you haven't already rolled it out.

[Keith Rollin]

Fixed in Bug 165642.