Bug 197684

Summary: [Win] Implement NetworkCache::Data by using FileSystem::MappedFileData
Product: WebKit Reporter: Fujii Hironori <Hironori.Fujii>
Component: WebKit2Assignee: Christopher Reid <chris.reid>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, annulen, aperez, Basuke.Suzuki, benjamin, bfulgham, cdumez, cgarcia, chris.reid, cmarcelo, commit-queue, cturner, dbates, don.olmstead, ews-watchlist, Hironori.Fujii, koivisto, ross.kirsling, stephan.szabo, webkit-bug-importer, youennf, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 207807    
Bug Blocks:    
Attachments:
Description Flags
patch
none
patch
none
patch
none
patch
none
patch
none
patch
none
patch
achristensen: review-
patch
none
patch
none
patch
none
patch
none
patch
koivisto: review-
Patch
none
patch
none
Patch
none
patch
none
Patch for landing
none
Patch for landing
none
Patch for landing
none
Patch for landing none

Description Fujii Hironori 2019-05-07 21:07:48 PDT
[Win] Implement NetworkCache::Data by using FileSystem::MappedFileData

We need to implement FileSystem::MappedFileData for Windows first.
Comment 1 Fujii Hironori 2019-05-07 21:10:25 PDT
See also Bug 196289 Comment 28.
Comment 2 Fujii Hironori 2019-05-26 22:11:18 PDT
(In reply to Fujii Hironori from comment #0)
> We need to implement FileSystem::MappedFileData for Windows first.

Filed this in Bug 198269.
Comment 3 Christopher Reid 2020-01-30 15:35:34 PST
Created attachment 389293 [details]
patch

WinCairo now passes 30/31 disk-cache tests with these changes.
Comment 4 Christopher Reid 2020-01-30 16:24:03 PST
Created attachment 389299 [details]
patch

Fixing style and GTK/Mac build issues
Comment 5 Christopher Reid 2020-01-31 13:56:08 PST
Created attachment 389410 [details]
patch

more build fixes and fixing GTK assert
Comment 6 Christopher Reid 2020-01-31 13:59:41 PST
Created attachment 389411 [details]
patch

style fixes
Comment 7 Christopher Reid 2020-01-31 16:11:06 PST
Created attachment 389437 [details]
patch

Adding flag to FileSystem::openFile can open with 0600 directly instead of 0666 then fchmod to 0600.
Comment 8 Christopher Reid 2020-01-31 16:20:43 PST
Created attachment 389438 [details]
patch

fixing mac builds
Comment 9 Fujii Hironori 2020-02-02 20:13:30 PST
Comment on attachment 389438 [details]
patch

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

> LayoutTests/platform/wincairo/TestExpectations:856
> +

Sort alphabetically. Put http/tests/cache/* after http/tests/blink/sendbeacon before http/tests/cache-storage .

> Source/WTF/wtf/win/FileSystemWin.cpp:439
> +        creationDisposition = CREATE_ALWAYS;

CREATE_ALWAYS always truncates the file. Why is this OK?

> Source/WTF/wtf/win/PathWalker.cpp:35
> +    String path = makeString(directory, "\\"_s, pattern);

'\\' is slightly better than "\\"_s.

> Source/WebKit/NetworkProcess/cache/NetworkCacheDataCurl.cpp:47
> +    return Data(WTF::nullopt);

This is null Data. Data::empty() returns the empty Data in other ports. Is this OK?
Comment 10 Christopher Reid 2020-02-03 18:50:41 PST
(In reply to Fujii Hironori from comment #9)
> Comment on attachment 389438 [details]
> patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=389438&action=review
> 
> > LayoutTests/platform/wincairo/TestExpectations:856
> > +
> 
> Sort alphabetically. Put http/tests/cache/* after
> http/tests/blink/sendbeacon before http/tests/cache-storage .
> 
will fix this.

> > Source/WTF/wtf/win/FileSystemWin.cpp:439
> > +        creationDisposition = CREATE_ALWAYS;
> 
> CREATE_ALWAYS always truncates the file. Why is this OK?
> 
It seems like this was only working for this case because if the file exists it gets deleted before this call so it's not called with a non-empty file. https://github.com/WebKit/webkit/blob/master/Source/WebKit/NetworkProcess/cache/NetworkCacheBlobStorage.cpp#L111.
O_RDWR doesn't seem needed for this case then.

> > Source/WTF/wtf/win/PathWalker.cpp:35
> > +    String path = makeString(directory, "\\"_s, pattern);
> 
> '\\' is slightly better than "\\"_s.
> 
will fix this.

> > Source/WebKit/NetworkProcess/cache/NetworkCacheDataCurl.cpp:47
> > +    return Data(WTF::nullopt);
> 
> This is null Data. Data::empty() returns the empty Data in other ports. Is
> this OK?
Right, this should use an empty vector instead of null.
Comment 11 Christopher Reid 2020-02-04 15:10:23 PST
Created attachment 389719 [details]
patch

updated patch
Comment 12 Brent Fulgham 2020-02-05 11:26:49 PST
Comment on attachment 389719 [details]
patch

I think this patch looks good from the Windows code standpoint, but we need Antti or another cache person to double-check the cache logic.
Comment 13 Alex Christensen 2020-02-05 15:32:45 PST
Comment on attachment 389719 [details]
patch

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

> Source/WTF/wtf/FileSystem.cpp:336
> +    default:
> +        ASSERT_NOT_REACHED();

Could we remove the default and put EventsOnly?

> Source/WTF/wtf/FileSystem.h:87
> +enum class FileAccessPermission {

: bool

> Source/WTF/wtf/posix/FileSystemPOSIX.cpp:96
>          platformFlag |= O_EVTONLY;

Please also change this to a switch.

> Source/WTF/wtf/win/FileSystemWin.cpp:638
> +    default:
> +        ASSERT_NOT_REACHED();

Again, please remove the default.

> Source/WebKit/NetworkProcess/cache/NetworkCacheDataCurl.cpp:96
> +    memcpy(buffer.data(), map, size);

Do we really want this memcpy here?  Doesn't that defeat the purpose of having mapped memory?
Comment 14 Adrian Perez 2020-02-05 16:25:32 PST
Comment on attachment 389719 [details]
patch

Overall it's super nice to see common code being cleaned up and operating
system specific guards being removed \o/

Adding to Alex's comments, the changes to the GLib/Soup/Unix bits look good
to me, with the caveat that the API test /webkit/WebKitWebsiteData/databases
is not supposed to be flaky so I would say we need it to keep it passing
before we can land this—please drop me a line (or anyone else) on IRC if
you need a hand figuring out why it might be failing :-]


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

> Source/WTF/wtf/posix/FileSystemPOSIX.cpp:103
> +        permissionFlag |= 0666;

I would probably use (S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH|S_IWOTH)
for the sake of consistency, even though 0666 is a common idiom.

(Note: with _BSD_SOURCE there is a DEFFILEMODE macro which expands to
the expression above, but it won't be available in all systems so I'd
rather avoid using this macro.)
Comment 15 Christopher Reid 2020-02-11 13:59:25 PST
Created attachment 390418 [details]
patch

Updated patch addressing comments from Alex and Adrian.

I updated NetworkCacheDataCurl to use a ref counted Variant<Vector<uint8_t>, FileSystem::MappedFileData> to act like the ref counted buffers for mmap/allocated containers in Soup and Mac ports so the mapped file can be kept around.

I'll be digging in to that GTK failure today.
Comment 16 Fujii Hironori 2020-02-12 01:49:51 PST
Comment on attachment 390418 [details]
patch

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

> Source/WTF/wtf/win/FileSystemWin.cpp:-438
> -        ASSERT_NOT_REACHED();

Why did you remove this ASSERT_NOT_REACHED? IIRC, MSVC reports compilation warnings.

> Source/WTF/wtf/win/FileSystemWin.cpp:551
> +    if (!GetDiskFreeSpaceExW(path.wideCharacters().data(), &freeBytesAvailableToCaller, &totalNumberOfBytes, &totalNumberOfFreeBytes))

GetDiskFreeSpaceExW can take NULL. Replace totalNumberOfBytes, totalNumberOfFreeBytes with nullptr.

> Source/WTF/wtf/win/FileSystemWin.cpp:554
> +    if (freeBytesAvailableToCaller.QuadPart > static_cast<ULONGLONG>(std::numeric_limits<uint64_t>::max()))

ULONGLONG and uint64_t are same in x64 Windows. Is this code only for 32bit Windows?
If so, how about enclosing it with #if defined(_M_IX86).
https://docs.microsoft.com/en-us/windows/win32/winprog/windows-data-types

> Source/WTF/wtf/win/FileSystemWin.cpp:555
> +        return false;

If you have plenty of storage, it always return false.
What do you think returning std::numeric_limits<uint64_t>::max() in this case?

> Source/WebKit/NetworkProcess/cache/NetworkCacheData.h:102
> +    Box<Variant<Vector<uint8_t>, FileSystem::MappedFileData>> m_buffer;

Box<T> manages T as ThreadSafeRefCounted. Do you need to manage T by thread-safe and ref-counted?
What about using Optional<T> or unique_ptr<T> ?
Variant is already marked by WTF_MAKE_FAST_ALLOCATED.
Comment 17 Konstantin Tokarev 2020-02-12 04:09:55 PST
(In reply to Fujii Hironori from comment #16)
> Comment on attachment 390418 [details]
> patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=390418&action=review
> 
> > Source/WTF/wtf/win/FileSystemWin.cpp:-438
> > -        ASSERT_NOT_REACHED();
> 
> Why did you remove this ASSERT_NOT_REACHED? IIRC, MSVC reports compilation
> warnings.

If assert is really needed to avoid warning, then release build should have this warning because it's not a RELEASE_ASSERT_NOT_REACHED()
Comment 18 Christopher Reid 2020-02-12 14:03:39 PST
Comment on attachment 390418 [details]
patch

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

>>> Source/WTF/wtf/win/FileSystemWin.cpp:-438
>>> -        ASSERT_NOT_REACHED();
>> 
>> Why did you remove this ASSERT_NOT_REACHED? IIRC, MSVC reports compilation warnings.
> 
> If assert is really needed to avoid warning, then release build should have this warning because it's not a RELEASE_ASSERT_NOT_REACHED()

Right should be a warning in MSVC if there was an unhandled enum class value. All the FileOpenMode cases for windows are handled here so we shouldn't get a warning.

>> Source/WTF/wtf/win/FileSystemWin.cpp:551
>> +    if (!GetDiskFreeSpaceExW(path.wideCharacters().data(), &freeBytesAvailableToCaller, &totalNumberOfBytes, &totalNumberOfFreeBytes))
> 
> GetDiskFreeSpaceExW can take NULL. Replace totalNumberOfBytes, totalNumberOfFreeBytes with nullptr.

That's good to know, I'll take them out.

>> Source/WTF/wtf/win/FileSystemWin.cpp:554
>> +    if (freeBytesAvailableToCaller.QuadPart > static_cast<ULONGLONG>(std::numeric_limits<uint64_t>::max()))
> 
> ULONGLONG and uint64_t are same in x64 Windows. Is this code only for 32bit Windows?
> If so, how about enclosing it with #if defined(_M_IX86).
> https://docs.microsoft.com/en-us/windows/win32/winprog/windows-data-types

Oh yeah this seems pretty redundant even on 32-bit windows.
On 32-bit ULONGLONG and uint64_t should still both have at least 64-bits of storage.
I'll remove this check.

>> Source/WebKit/NetworkProcess/cache/NetworkCacheData.h:102
>> +    Box<Variant<Vector<uint8_t>, FileSystem::MappedFileData>> m_buffer;
> 
> Box<T> manages T as ThreadSafeRefCounted. Do you need to manage T by thread-safe and ref-counted?
> What about using Optional<T> or unique_ptr<T> ?
> Variant is already marked by WTF_MAKE_FAST_ALLOCATED.

I used this ref-counted container here because there's currently some cases where this class is copied and needs to manage multiple instances of the data.
The case I remember seeing was https://github.com/WebKit/webkit/blob/master/Source/WebKit/NetworkProcess/cache/NetworkCacheStorage.cpp#L99
Comment 19 Christopher Reid 2020-02-13 15:32:10 PST
Created attachment 390697 [details]
patch

Updated patch
Comment 20 Christopher Reid 2020-02-13 15:38:39 PST
Created attachment 390698 [details]
patch

Style fix
Comment 21 Christopher Reid 2020-02-13 15:48:23 PST
This should also fix the GTK failure.

It seemed to be a pre-existing problem where the CacheStorage directory wasn't being created for ports without SandboxExtensions.

Before trying to open CacheStorage/salt was failing silently.
Now with these FileSystem changes GTK's writeToFile fails an assert if the handle isn't valid.

That directory should now be created properly for all ports.
Comment 22 Fujii Hironori 2020-02-13 18:09:51 PST
Comment on attachment 390698 [details]
patch

LGTM
Comment 23 Adrian Perez 2020-02-14 02:04:25 PST
(In reply to Christopher Reid from comment #21)
> This should also fix the GTK failure.
> 
> It seemed to be a pre-existing problem where the CacheStorage directory
> wasn't being created for ports without SandboxExtensions.
> 
> Before trying to open CacheStorage/salt was failing silently.
> Now with these FileSystem changes GTK's writeToFile fails an assert if the
> handle isn't valid.
> 
> That directory should now be created properly for all ports.

Nicer find! đź’Ş
Comment 24 WebKit Commit Bot 2020-02-14 12:05:51 PST
The commit-queue encountered the following flaky tests while processing attachment 390698 [details]:

editing/spelling/spellcheck-async-remove-frame.html bug 158401 (authors: morrita@google.com, rniwa@webkit.org, and tony@chromium.org)
The commit-queue is continuing to process your patch.
Comment 25 WebKit Commit Bot 2020-02-14 12:06:33 PST
Comment on attachment 390698 [details]
patch

Clearing flags on attachment: 390698

Committed r256633: <https://trac.webkit.org/changeset/256633>
Comment 26 WebKit Commit Bot 2020-02-14 12:06:35 PST
All reviewed patches have been landed.  Closing bug.
Comment 27 Radar WebKit Bug Importer 2020-02-14 12:07:14 PST
<rdar://problem/59467397>
Comment 28 WebKit Commit Bot 2020-02-14 22:46:47 PST
Re-opened since this is blocked by bug 207807
Comment 29 Christopher Reid 2020-02-18 18:48:15 PST
Created attachment 391124 [details]
patch

I ended up re-adding FileOpenMode::ReadWrite since mmap was failing on FileSystemPOSIX when the file was created with O_WRONLY. That caused the memory regression in Mac.

I also wanted to check if O_EXCL is needed on this NetworkCacheData mapped file before re-landing. I didn't carry it over to FileSystem::openFile implementations since I don't think most openFile calls using ReadWrite will want to always create a file. Also the parent directory for the caches are created with S_IRWXU access.

If O_EXCL is needed it might make sense then to have something like a FileSystem::createFile call that fails if the file already exists.
Comment 30 Christopher Reid 2020-02-18 18:51:57 PST
Created attachment 391126 [details]
patch

fixing gtk build
Comment 31 Yusuke Suzuki 2020-02-19 11:02:46 PST
Note that I already run the patch offered by Chris and ensured that new patch does not cause Membuster regression on macOS.
Comment 32 Antti Koivisto 2020-02-20 09:09:42 PST
Comment on attachment 391126 [details]
patch

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

> Source/WebKit/NetworkProcess/cache/NetworkCacheData.cpp:45
> -    int fd = open(FileSystem::fileSystemRepresentation(path).data(), O_CREAT | O_EXCL | O_RDWR , S_IRUSR | S_IWUSR);
> -    if (fd < 0)
> -        return { };
> -
> -    if (ftruncate(fd, m_size) < 0) {
> -        close(fd);
> +    auto handle = FileSystem::openFile(path, FileSystem::FileOpenMode::ReadWrite, FileSystem::FileAccessPermission::User);
> +    if (!FileSystem::truncateFile(handle, m_size)) {
> +        FileSystem::closeFile(handle);

Please don't lose O_EXCL (or any other flags). Otherwise in error situation we'l truncate a file backing an existing mapping. This will crash hard.
Comment 33 Christopher Reid 2020-02-20 17:35:43 PST
Created attachment 391361 [details]
Patch

Added FileSystem::createFile to keep the O_EXCL flag. It will return an invalid handle if the file already exists.
Comment 34 Christopher Reid 2020-02-20 17:39:27 PST
Created attachment 391362 [details]
patch

mac build fix
Comment 35 Antti Koivisto 2020-02-20 23:47:47 PST
Comment on attachment 391362 [details]
patch

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

> Source/WTF/wtf/posix/FileSystemPOSIX.cpp:116
> +PlatformFileHandle createFile(const String& path, FileOpenMode mode, FileAccessPermission permission)

Can't exclusivness just be another parameter for openFile? Seems unnecessary to copy the whole function. Also your openFile can still create files and name createFile doesn't really communicate the difference.
Comment 36 Christopher Reid 2020-02-25 00:49:03 PST
Created attachment 391632 [details]
Patch

Added onlyCreateFile parameter to openFile instead of using a new function.
Comment 37 Christopher Reid 2020-02-25 14:18:06 PST
Created attachment 391685 [details]
patch

Ensure the salt file is deleted if it exists but doesn't match the needed salt size.
Comment 38 Yusuke Suzuki 2020-02-25 17:25:59 PST
Comment on attachment 391685 [details]
patch

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

r=me

> Source/WTF/ChangeLog:16
> +        Added onlyCreateFile flag to openFile which causes openFile to fail if the file already exists.

We should name this flag as `failIfFileExists`

> Source/WebKit/NetworkProcess/cache/NetworkCacheData.cpp:43
> +    auto handle = FileSystem::openFile(path, FileSystem::FileOpenMode::ReadWrite, FileSystem::FileAccessPermission::User, true);

Let's not pass boolean directly, it is hard to read what this bool means.

constexpr bool failIfFileExists = true;
auto handle = FileSystem::openFile(path, ..., failIfFileExists);

is better.
Comment 39 Christopher Reid 2020-02-26 10:52:43 PST
Created attachment 391756 [details]
Patch for landing
Comment 40 Christopher Reid 2020-02-26 10:57:59 PST
Created attachment 391757 [details]
Patch for landing
Comment 41 WebKit Commit Bot 2020-02-26 11:28:19 PST
Comment on attachment 391757 [details]
Patch for landing

Rejecting attachment 391757 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-02', 'validate-changelog', '--check-oops', '--non-interactive', 391757, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

ChangeLog entry in LayoutTests/ChangeLog contains OOPS!.

Full output: https://webkit-queues.webkit.org/results/13329377
Comment 42 Christopher Reid 2020-02-26 13:05:15 PST
Created attachment 391772 [details]
Patch for landing
Comment 43 WebKit Commit Bot 2020-02-26 13:40:01 PST
Comment on attachment 391772 [details]
Patch for landing

Rejecting attachment 391772 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 391772, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Logging in as commit-queue@webkit.org...
Fetching: https://bugs.webkit.org/attachment.cgi?id=391772&action=edit
Fetching: https://bugs.webkit.org/show_bug.cgi?id=197684&ctype=xml&excludefield=attachmentdata
Processing 1 patch from 1 bug.
Updating working directory
Processing patch 391772 from bug 197684.
Fetching: https://bugs.webkit.org/attachment.cgi?id=391772
Failed to run "['git', 'svn', 'dcommit', '--rmdir']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	LayoutTests/ChangeLog
	M	LayoutTests/platform/wincairo/TestExpectations
	M	Source/WTF/ChangeLog
	M	Source/WTF/wtf/FileSystem.cpp
	M	Source/WTF/wtf/FileSystem.h
	M	Source/WTF/wtf/glib/FileSystemGlib.cpp
	M	Source/WTF/wtf/posix/FileSystemPOSIX.cpp
	M	Source/WTF/wtf/win/FileSystemWin.cpp
	M	Source/WTF/wtf/win/PathWalker.cpp
	M	Source/WebKit/ChangeLog
	M	Source/WebKit/NetworkProcess/NetworkProcess.cpp
	M	Source/WebKit/NetworkProcess/cache/NetworkCacheData.cpp
	M	Source/WebKit/NetworkProcess/cache/NetworkCacheData.h
	M	Source/WebKit/NetworkProcess/cache/NetworkCacheDataCocoa.mm
	M	Source/WebKit/NetworkProcess/cache/NetworkCacheDataCurl.cpp
	M	Source/WebKit/NetworkProcess/cache/NetworkCacheDataSoup.cpp
	M	Source/WebKit/NetworkProcess/cache/NetworkCacheFileSystem.cpp
	M	Tools/ChangeLog
	M	Tools/TestWebKitAPI/Tests/WTF/FileSystem.cpp

ERROR from SVN:
Merge conflict during commit: Conflict at '/trunk/LayoutTests/ChangeLog'
W: c1bfafc7aabfe735fbd0fdaeed79acbf8a3a31be and refs/remotes/origin/master differ, using rebase:
:040000 040000 3715dea03ca78607882c1c315dd41b063fb3cfd7 148ef925334bff92d196d535b0dac66bb1037bda M	LayoutTests
:040000 040000 82adc380b74dda0797ee354f6e8bade45a6ddd51 8eee6c7b770d71aabbb3cc5bab7b21d43e8dbbeb M	Source
:040000 040000 57674841b68c9058d5d4bbfa659f2fbd7367d9b4 adfcec4e4cae19a6159741b7cffdddced27e95ec M	Tools
Current branch master is up to date.
ERROR: Not all changes have been committed into SVN, however the committed
ones (if any) seem to be successfully integrated into the working tree.
Please see the above messages for details.


Failed to run "['git', 'svn', 'dcommit', '--rmdir']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	LayoutTests/ChangeLog
	M	LayoutTests/platform/wincairo/TestExpectations
	M	Source/WTF/ChangeLog
	M	Source/WTF/wtf/FileSystem.cpp
	M	Source/WTF/wtf/FileSystem.h
	M	Source/WTF/wtf/glib/FileSystemGlib.cpp
	M	Source/WTF/wtf/posix/FileSystemPOSIX.cpp
	M	Source/WTF/wtf/win/FileSystemWin.cpp
	M	Source/WTF/wtf/win/PathWalker.cpp
	M	Source/WebKit/ChangeLog
	M	Source/WebKit/NetworkProcess/NetworkProcess.cpp
	M	Source/WebKit/NetworkProcess/cache/NetworkCacheData.cpp
	M	Source/WebKit/NetworkProcess/cache/NetworkCacheData.h
	M	Source/WebKit/NetworkProcess/cache/NetworkCacheDataCocoa.mm
	M	Source/WebKit/NetworkProcess/cache/NetworkCacheDataCurl.cpp
	M	Source/WebKit/NetworkProcess/cache/NetworkCacheDataSoup.cpp
	M	Source/WebKit/NetworkProcess/cache/NetworkCacheFileSystem.cpp
	M	Tools/ChangeLog
	M	Tools/TestWebKitAPI/Tests/WTF/FileSystem.cpp

ERROR from SVN:
Merge conflict during commit: Conflict at '/trunk/LayoutTests/ChangeLog'
W: c1bfafc7aabfe735fbd0fdaeed79acbf8a3a31be and refs/remotes/origin/master differ, using rebase:
:040000 040000 3715dea03ca78607882c1c315dd41b063fb3cfd7 148ef925334bff92d196d535b0dac66bb1037bda M	LayoutTests
:040000 040000 82adc380b74dda0797ee354f6e8bade45a6ddd51 8eee6c7b770d71aabbb3cc5bab7b21d43e8dbbeb M	Source
:040000 040000 57674841b68c9058d5d4bbfa659f2fbd7367d9b4 adfcec4e4cae19a6159741b7cffdddced27e95ec M	Tools
Current branch master is up to date.
ERROR: Not all changes have been committed into SVN, however the committed
ones (if any) seem to be successfully integrated into the working tree.
Please see the above messages for details.


Failed to run "['git', 'svn', 'dcommit', '--rmdir']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit
Updating OpenSource
From https://git.webkit.org/git/WebKit
   3e82a3cc5f3..6e9734c7a4b  master     -> origin/master
Partial-rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc ...
Currently at 257504 = 3e82a3cc5f3052c3efd94f1fcd7e5a415cff6081
r257505 = a637395bea617d4b228aee28c27901a985d367d9
r257506 = 527e95c4d5abfe1debc954754e724863585b06b0
r257507 = c79f943807fc8c0263328fe23f54573a3f130024
r257508 = 6e9734c7a4b54261fe14c29ef4c46c599eb2f656
Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc
First, rewinding head to replay your work on top of it...
Fast-forwarded master to refs/remotes/origin/master.
Total errors found: 0 in 1 files

Full output: https://webkit-queues.webkit.org/results/13329457
Comment 44 Christopher Reid 2020-02-26 13:57:59 PST
Created attachment 391776 [details]
Patch for landing
Comment 45 WebKit Commit Bot 2020-02-26 14:41:45 PST
Comment on attachment 391776 [details]
Patch for landing

Clearing flags on attachment: 391776

Committed r257518: <https://trac.webkit.org/changeset/257518>
Comment 46 WebKit Commit Bot 2020-02-26 14:41:48 PST
All reviewed patches have been landed.  Closing bug.