Bug 74164 - Add the FileSystem functions of iOS
Summary: Add the FileSystem functions of iOS
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Benjamin Poulain
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-12-08 21:47 PST by Benjamin Poulain
Modified: 2011-12-09 21:34 PST (History)
2 users (show)

See Also:


Attachments
Patch (14.85 KB, patch)
2011-12-08 21:54 PST, Benjamin Poulain
no flags Details | Formatted Diff | Diff
Patch (11.51 KB, patch)
2011-12-09 17:26 PST, Benjamin Poulain
ddkilzer: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Benjamin Poulain 2011-12-08 21:47:33 PST
Add the FileSystem of iOS
Comment 1 Benjamin Poulain 2011-12-08 21:54:25 PST
Created attachment 118529 [details]
Patch
Comment 2 Alexey Proskuryakov 2011-12-09 13:52:39 PST
Comment on attachment 118529 [details]
Patch

Were the added files published in iOS code drops before? In that case, they may need to retain existing copyright dates.
Comment 3 Benjamin Poulain 2011-12-09 14:12:06 PST
Comment on attachment 118529 [details]
Patch

(In reply to comment #2)
> (From update of attachment 118529 [details])
> Were the added files published in iOS code drops before? In that case, they may need to retain existing copyright dates.

I think one of them was. I will check
Comment 4 Benjamin Poulain 2011-12-09 14:21:49 PST
Comment on attachment 118529 [details]
Patch

> I think one of them was. I will check

Nope, actually that is fine.
Comment 5 David Kilzer (:ddkilzer) 2011-12-09 17:07:04 PST
Comment on attachment 118529 [details]
Patch

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

r- to address a strategy for Source/WebKit code sharing and platform structure.

It may be easier to land the WebCore pieces first.

> Source/WebKit/mac/Misc/WebNSFileManagerExtras.h:39
>  @interface NSFileManager (WebNSFileManagerExtras)
> +#if !PLATFORM(IOS)
>  - (void)_webkit_setMetadataURL:(NSString *)URLString referrer:(NSString *)referrer atPath:(NSString *)path;
>  - (NSString *)_webkit_startupVolumeName;
> +#endif // !PLATFORM(IOS)
> +
> +#if PLATFORM(IOS)
> +- (NSString *)_webkit_createTemporaryDirectoryWithTemplatePrefix:(NSString *)prefix;
> +#endif // PLATFORM(IOS)

We need to come up with a better strategy for sharing this code rather than mixing Mac-only, iOS-only and shared methods into a single header and implementation file.

Also, adding PLATFORM(IOS) macros won't work for any public headers unless we start post-processing the headers to strip out these macros, and it could impact any Apple projects using private headers as SPI (since these macros won't be defined when they build their projects).
Comment 6 Benjamin Poulain 2011-12-09 17:26:42 PST
Created attachment 118670 [details]
Patch
Comment 7 Benjamin Poulain 2011-12-09 17:27:58 PST
> We need to come up with a better strategy for sharing this code rather than mixing Mac-only, iOS-only and shared methods into a single header and implementation file.

Good point. 

I believed we could do that afterwards but we can't since that would pollute the public headers.
Comment 8 David Kilzer (:ddkilzer) 2011-12-09 20:46:07 PST
Comment on attachment 118670 [details]
Patch

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

r=me

> Source/WebCore/platform/ios/FileSystemIOS.mm:31
> +#if PLATFORM(IOS)

Is this needed with the EXCLUDED_SOURCE_FILE_NAMES variable in Source/WebCore/Configurations/WebCore.xcconfig?
Comment 9 Benjamin Poulain 2011-12-09 20:49:02 PST
> > Source/WebCore/platform/ios/FileSystemIOS.mm:31
> > +#if PLATFORM(IOS)
> 
> Is this needed with the EXCLUDED_SOURCE_FILE_NAMES variable in Source/WebCore/Configurations/WebCore.xcconfig?

Nope, that shouldn't be necessary. I'll update that before landing.

Thanks for the review.
Comment 10 Benjamin Poulain 2011-12-09 21:34:15 PST
Committed r102517: <http://trac.webkit.org/changeset/102517>