Bug 192042 – [Win] listDirectory in FileSystemWin.cpp should not skip all directories

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Christopher Reid]

ASSERT(diskUsage >= databaseFileSize); is failing in SQLiteIDBBackingStore.cpp with mozilla's indexdb demo https://mdn.mozillademos.org/en-US/docs/Web/API/IndexedDB_API/Using_IndexedDB$samples/Full_IndexedDB_example?revision=1436833

SQLiteIDBBackingStore.cpp:
>    uint64_t diskUsage = 0;
>    for (auto& directory : FileSystem::listDirectory(m_absoluteDatabaseDirectory, "*")) {
>        for (auto& file : FileSystem::listDirectory(directory, "*.sqlite3"_s))
>            diskUsage += SQLiteFileSystem::getDatabaseFileSize(file);
>    }
>    ASSERT(diskUsage >= databaseFileSize);

FileSystem::listDirectory(m_absoluteDatabaseDirectory, "*") doesn't return anything because windows' listDirectory implementation never returns any child directories.
So diskUsage is always 0 for this case.

[Christopher Reid]

Created attachment 355804 [details]
Patch

[Don Olmstead]

Comment on attachment 355804 [details]
Patch

LGTM

[Brent Fulgham]

Comment on attachment 355804 [details]
Patch

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

> Source/WebCore/ChangeLog:8
> +        Covered by existing tests. 

Does this allow us to unskip a test on Windows?

[Fujii Hironori]

Comment on attachment 355804 [details]
Patch

good catch.

[Christopher Reid]

(In reply to Brent Fulgham from comment #3)
> Comment on attachment 355804 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=355804&action=review
> 
> > Source/WebCore/ChangeLog:8
> > +        Covered by existing tests. 
> 
> Does this allow us to unskip a test on Windows?

It appears so, all storage tests are currently skipped on wincairo and all indexeddb tests seem to fail that assert. I believe most indexeddb tests should be passing now, I'll run through storage tests and update the expectations.

[Christopher Reid]

Created attachment 356021 [details]
patch

[Don Olmstead]

The WinCairo bot seems to just be in bad shape. Guessing you compiled on wincairo so we can assume all is well.

[Christopher Reid]

(In reply to Don Olmstead from comment #7)
> The WinCairo bot seems to just be in bad shape. Guessing you compiled on
> wincairo so we can assume all is well.

> "bin64\WebKitTestRunner.exe". The process cannot access the file because it is being used by another process
Yeah, I'm not sure why WebKitTestRunner.exe would be in use on wincairo EWS

[WebKit Commit Bot]

Comment on attachment 356021 [details]
patch

Clearing flags on attachment: 356021

Committed r238723: <https://trac.webkit.org/changeset/238723>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/46361645>