Bug 196289 - [WinCairo] storage/indexeddb tests are timing out
Summary: [WinCairo] storage/indexeddb tests are timing out
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Fujii Hironori
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-03-26 22:26 PDT by Fujii Hironori
Modified: 2019-05-15 20:41 PDT (History)
15 users (show)

See Also:


Attachments
WIP patch (15.63 KB, patch)
2019-04-05 03:29 PDT, Fujii Hironori
no flags Details | Formatted Diff | Diff
WIP patch (15.76 KB, patch)
2019-04-08 22:01 PDT, Fujii Hironori
no flags Details | Formatted Diff | Diff
Patch (19.12 KB, patch)
2019-04-22 00:33 PDT, Fujii Hironori
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews126 for ios-simulator-wk2 (17.34 MB, application/zip)
2019-04-22 14:58 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews107 for mac-highsierra-wk2 (2.96 MB, application/zip)
2019-04-22 19:22 PDT, Build Bot
no flags Details
Patch (25.37 KB, patch)
2019-05-06 22:58 PDT, Fujii Hironori
no flags Details | Formatted Diff | Diff
Patch (24.10 KB, patch)
2019-05-06 23:35 PDT, Fujii Hironori
no flags Details | Formatted Diff | Diff
Patch (25.32 KB, patch)
2019-05-07 00:05 PDT, Fujii Hironori
no flags Details | Formatted Diff | Diff
Patch (25.31 KB, patch)
2019-05-07 00:13 PDT, Fujii Hironori
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews106 for mac-highsierra-wk2 (2.91 MB, application/zip)
2019-05-07 01:41 PDT, Build Bot
no flags Details
Patch (25.31 KB, patch)
2019-05-07 02:19 PDT, Fujii Hironori
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews105 for mac-highsierra-wk2 (2.82 MB, application/zip)
2019-05-07 03:42 PDT, Build Bot
no flags Details
Patch (25.31 KB, patch)
2019-05-07 03:52 PDT, Fujii Hironori
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews105 for mac-highsierra-wk2 (2.78 MB, application/zip)
2019-05-07 05:14 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews212 for win-future (13.49 MB, application/zip)
2019-05-07 06:25 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews125 for ios-simulator-wk2 (2.68 MB, application/zip)
2019-05-07 08:57 PDT, Build Bot
no flags Details
Patch (25.77 KB, patch)
2019-05-07 22:20 PDT, Fujii Hironori
no flags Details | Formatted Diff | Diff
Patch (27.13 KB, patch)
2019-05-08 20:30 PDT, Fujii Hironori
no flags Details | Formatted Diff | Diff
Patch (27.05 KB, patch)
2019-05-09 19:19 PDT, Fujii Hironori
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Fujii Hironori 2019-03-26 22:26:12 PDT
[curl] storage/indexeddb tests are timing out

Even though WK1 DumpRenderTree can pass the test cases, WK2 WKTR is timing out.

> python ./Tools/Scripts/run-webkit-tests --debug  --wincairo --no-new-test-results storage/indexeddb
Comment 1 Fujii Hironori 2019-03-26 22:27:49 PDT
CacheStorageEngine doesn't work because following files are unimplemented yet.

Source/WebKit/NetworkProcess/cache/NetworkCacheDataCurl.cpp
Source/WebKit/NetworkProcess/cache/NetworkCacheIOChannelCurl.cpp
Comment 2 Fujii Hironori 2019-04-05 03:29:05 PDT
Created attachment 366805 [details]
WIP patch
Comment 3 Fujii Hironori 2019-04-08 22:01:07 PDT
Created attachment 367027 [details]
WIP patch
Comment 4 Don Olmstead 2019-04-09 09:13:57 PDT
Is it possible to use more of FileSystem and file mapping to make the network cache platform agnostic? It feels like some of your patch is going this direction so I figured I'd just ask your opinion on it.
Comment 5 Fujii Hironori 2019-04-21 23:27:54 PDT
Yeah, I have the same question in my mind.
Comment 6 Fujii Hironori 2019-04-22 00:33:59 PDT
Created attachment 367926 [details]
Patch
Comment 7 Build Bot 2019-04-22 14:58:54 PDT
Comment on attachment 367926 [details]
Patch

Attachment 367926 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/11964268

New failing tests:
imported/w3c/web-platform-tests/service-workers/service-worker/fetch-event-within-sw.https.html
http/tests/cache-storage/cache-origins.https.html
imported/w3c/web-platform-tests/service-workers/cache-storage/serviceworker/cache-keys-attributes-for-service-worker.https.html
http/tests/cache-storage/cache-persistency.https.html
Comment 8 Build Bot 2019-04-22 14:58:56 PDT
Created attachment 367983 [details]
Archive of layout-test-results from ews126 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6
Comment 9 Build Bot 2019-04-22 19:22:40 PDT
Comment on attachment 367926 [details]
Patch

Attachment 367926 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/11967880

New failing tests:
imported/w3c/web-platform-tests/service-workers/service-worker/fetch-event-within-sw.https.html
imported/w3c/web-platform-tests/service-workers/cache-storage/serviceworker/cache-keys-attributes-for-service-worker.https.html
http/tests/cache-storage/cache-origins.https.html
http/tests/cache-storage/cache-records-persistency.https.html
http/tests/cache-storage/cache-persistency.https.html
Comment 10 Build Bot 2019-04-22 19:22:42 PDT
Created attachment 368006 [details]
Archive of layout-test-results from ews107 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6
Comment 11 Fujii Hironori 2019-05-06 22:58:15 PDT
Created attachment 369239 [details]
Patch
Comment 12 Fujii Hironori 2019-05-06 23:35:46 PDT
Created attachment 369244 [details]
Patch
Comment 13 Fujii Hironori 2019-05-07 00:05:18 PDT
Created attachment 369248 [details]
Patch
Comment 14 Fujii Hironori 2019-05-07 00:13:10 PDT
Created attachment 369250 [details]
Patch
Comment 15 Build Bot 2019-05-07 01:41:50 PDT
Comment on attachment 369250 [details]
Patch

Attachment 369250 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/12120984

New failing tests:
http/tests/cache-storage/cache-origins.https.html
imported/w3c/web-platform-tests/service-workers/cache-storage/serviceworker/cache-match.https.html
imported/w3c/web-platform-tests/service-workers/cache-storage/window/cache-match.https.html
http/tests/cache-storage/cache-records-persistency.https.html
http/tests/cache-storage/cache-persistency.https.html
imported/w3c/web-platform-tests/service-workers/service-worker/fetch-event-within-sw.https.html
imported/w3c/web-platform-tests/service-workers/cache-storage/worker/cache-match.https.html
imported/w3c/web-platform-tests/service-workers/cache-storage/serviceworker/cache-keys-attributes-for-service-worker.https.html
Comment 16 Build Bot 2019-05-07 01:41:52 PDT
Created attachment 369256 [details]
Archive of layout-test-results from ews106 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6
Comment 17 Fujii Hironori 2019-05-07 02:19:20 PDT
Created attachment 369260 [details]
Patch
Comment 18 Build Bot 2019-05-07 03:42:51 PDT
Comment on attachment 369260 [details]
Patch

Attachment 369260 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/12121879

New failing tests:
imported/w3c/web-platform-tests/service-workers/cache-storage/window/cache-match.https.html
imported/w3c/web-platform-tests/service-workers/cache-storage/worker/cache-match.https.html
imported/w3c/web-platform-tests/service-workers/cache-storage/serviceworker/cache-match.https.html
Comment 19 Build Bot 2019-05-07 03:42:53 PDT
Created attachment 369271 [details]
Archive of layout-test-results from ews105 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6
Comment 20 Fujii Hironori 2019-05-07 03:52:40 PDT
Created attachment 369273 [details]
Patch
Comment 21 Build Bot 2019-05-07 05:14:41 PDT
Comment on attachment 369273 [details]
Patch

Attachment 369273 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/12122573

New failing tests:
imported/w3c/web-platform-tests/service-workers/cache-storage/window/cache-match.https.html
imported/w3c/web-platform-tests/service-workers/cache-storage/worker/cache-match.https.html
imported/w3c/web-platform-tests/service-workers/cache-storage/serviceworker/cache-match.https.html
Comment 22 Build Bot 2019-05-07 05:14:43 PDT
Created attachment 369276 [details]
Archive of layout-test-results from ews105 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6
Comment 23 Build Bot 2019-05-07 06:25:54 PDT
Comment on attachment 369273 [details]
Patch

Attachment 369273 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/12122866

New failing tests:
security/contentSecurityPolicy/video-with-file-url-allowed-by-media-src-star-with-AllowContentSecurityPolicySourceStarToMatchAnyProtocol-enabled.html
Comment 24 Build Bot 2019-05-07 06:25:56 PDT
Created attachment 369278 [details]
Archive of layout-test-results from ews212 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews212  Port: win-future  Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit
Comment 25 Alex Christensen 2019-05-07 08:52:54 PDT
Comment on attachment 369273 [details]
Patch

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

> Source/WTF/wtf/win/FileSystemWin.cpp:506
> +bool hardLink(const String& source, const String& destination)
> +{
> +    return CreateHardLink(destination.wideCharacters().data(), source.wideCharacters().data(), nullptr);
> +}
> +
>  bool hardLinkOrCopyFile(const String& source, const String& destination)
>  {
>      return !!::CopyFile(source.wideCharacters().data(), destination.wideCharacters().data(), TRUE);

Why not just implement hardLinkOrCopyFile like other platforms?
Comment 26 Build Bot 2019-05-07 08:57:33 PDT
Comment on attachment 369273 [details]
Patch

Attachment 369273 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/12123492

New failing tests:
imported/w3c/web-platform-tests/service-workers/cache-storage/window/cache-match.https.html
imported/w3c/web-platform-tests/service-workers/cache-storage/worker/cache-match.https.html
imported/w3c/web-platform-tests/service-workers/cache-storage/serviceworker/cache-match.https.html
Comment 27 Build Bot 2019-05-07 08:57:35 PDT
Created attachment 369288 [details]
Archive of layout-test-results from ews125 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews125  Port: ios-simulator-wk2  Platform: Mac OS X 10.14.4
Comment 28 Don Olmstead 2019-05-07 09:29:59 PDT
There's some similar code to what's happening in NetworkCacheData.cpp in MemoryMappedFile within FileSystem. There isn't a Windows implementation at all but would it make sense to maybe expand on the MemoryMappedFile code and then just use that in data? This is definitely better but there's still a lot of Windows specific code happening.
Comment 29 Fujii Hironori 2019-05-07 19:25:11 PDT
Comment on attachment 369273 [details]
Patch

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

>> Source/WTF/wtf/win/FileSystemWin.cpp:506
>>      return !!::CopyFile(source.wideCharacters().data(), destination.wideCharacters().data(), TRUE);
> 
> Why not just implement hardLinkOrCopyFile like other platforms?

They are used for different usages.

hardLinkOrCopyFile is used only in
SQLiteIDBTransaction::moveBlobFilesIfNecessary. (Bug 156321)

https://github.com/WebKit/webkit/blob/89c28d471fae35f1788a0f857067896a10af8974/Source/WebCore/Modules/indexeddb/server/SQLiteIDBTransaction.cpp#L94

It is no problem for moveBlobFilesIfNecessary to use copying
becuase it will remove the source file soon.

On the other hand, BlobStorage::add should use hard linking
because BlobStorage unifies files which has the identical
content.

I don't know why moveBlobFilesIfNecessary needs the fallback of
using copy if hard linking fails. Is it possible to use only hard
linking even for moveBlobFilesIfNecessary case?
Comment 30 Fujii Hironori 2019-05-07 21:09:05 PDT
(In reply to Don Olmstead from comment #28)

Sounds a good idea. Filed in Bug 197684.
Comment 31 Fujii Hironori 2019-05-07 22:20:26 PDT
Created attachment 369358 [details]
Patch
Comment 32 Alex Christensen 2019-05-08 09:08:33 PDT
(In reply to Fujii Hironori from comment #29)
> I don't know why moveBlobFilesIfNecessary needs the fallback of
> using copy if hard linking fails. Is it possible to use only hard
> linking even for moveBlobFilesIfNecessary case?
hardLinkOrCopyFile uses hard linking because it's faster then falls back to copying if hard linking fails, such as if the source and destination are on different drives.  It is a performance optimization to hard link.
Comment 33 Alex Christensen 2019-05-08 09:18:14 PDT
Comment on attachment 369358 [details]
Patch

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

> Source/WTF/wtf/glib/FileSystemGlib.cpp:432
> +bool hardLink(const String& source, const String& destination)

hardLinkOrCopyFile could call this to reduce duplicate code.

> Source/WTF/wtf/posix/FileSystemPOSIX.cpp:447
> +bool hardLink(const String& source, const String& destination)

ditto

> Source/WebKit/NetworkProcess/cache/NetworkCacheData.h:78
> +#if !OS(WINDOWS)

I think we should not have so many macros in this header.  Having non-inline functions in the header would move these macros to the implementation files and make this file much tidier
Comment 34 Fujii Hironori 2019-05-08 20:30:07 PDT
Created attachment 369457 [details]
Patch

* Addressed review feedbacks.
Comment 35 Alex Christensen 2019-05-09 10:31:40 PDT
Comment on attachment 369457 [details]
Patch

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

> Source/WTF/wtf/glib/FileSystemGlib.cpp:456
> +    return !!::CopyFile(source.charactersWithNullTermination().data(), destination.charactersWithNullTermination().data(), TRUE);

Why do we use charactersWithNullTermination here...

> Source/WTF/wtf/win/FileSystemWin.cpp:510
>      return !!::CopyFile(source.wideCharacters().data(), destination.wideCharacters().data(), TRUE);

...but wideCharacters here?
Comment 36 Fujii Hironori 2019-05-09 19:04:13 PDT
Comment on attachment 369457 [details]
Patch

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

>> Source/WTF/wtf/glib/FileSystemGlib.cpp:456
>> +    return !!::CopyFile(source.charactersWithNullTermination().data(), destination.charactersWithNullTermination().data(), TRUE);
> 
> Why do we use charactersWithNullTermination here...

Good catch. It should be wideCharacters. We should use wideCharacters for Win32 API after ICU 59.
Comment 37 Fujii Hironori 2019-05-09 19:19:26 PDT
Created attachment 369539 [details]
Patch

* Addressed review feedbacks.
Comment 38 WebKit Commit Bot 2019-05-10 12:02:03 PDT
Comment on attachment 369539 [details]
Patch

Clearing flags on attachment: 369539

Committed r245186: <https://trac.webkit.org/changeset/245186>
Comment 39 WebKit Commit Bot 2019-05-10 12:02:05 PDT
All reviewed patches have been landed.  Closing bug.
Comment 40 Radar WebKit Bug Importer 2019-05-10 12:03:38 PDT
<rdar://problem/50671610>