Bug 185773

Summary: File's structured serialization should serialize lastModified attribute
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: Tools / TestsAssignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: alecflett, beidson, commit-queue, ews-watchlist, ggaren, jsbell, lforschler, mark.lam, rniwa, sam, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
URL: https://w3c.github.io/FileAPI/#file-section
Bug Depends on: 185850    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from ews102 for mac-sierra
none
Archive of layout-test-results from ews107 for mac-sierra-wk2
none
Patch
none
Archive of layout-test-results from ews206 for win-future
none
Patch
none
Patch
none
Patch
none
Patch none

Description Chris Dumez 2018-05-18 10:49:59 PDT
Re-sync & unskip web-platform-tests/workers/semantics/structured-clone/ after https://github.com/w3c/web-platform-tests/issues/10799.
Comment 1 Chris Dumez 2018-05-18 13:59:18 PDT
Created attachment 340735 [details]
Patch
Comment 2 EWS Watchlist 2018-05-18 15:05:13 PDT
Comment on attachment 340735 [details]
Patch

Attachment 340735 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/7725778

New failing tests:
fast/storage/serialized-script-value.html
imported/w3c/web-platform-tests/IndexedDB/keypath-special-identifiers.htm
Comment 3 EWS Watchlist 2018-05-18 15:05:14 PDT
Created attachment 340741 [details]
Archive of layout-test-results from ews102 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 4 EWS Watchlist 2018-05-18 15:13:20 PDT
Comment on attachment 340735 [details]
Patch

Attachment 340735 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/7725852

New failing tests:
fast/storage/serialized-script-value.html
imported/w3c/web-platform-tests/IndexedDB/keypath-special-identifiers.htm
Comment 5 EWS Watchlist 2018-05-18 15:13:22 PDT
Created attachment 340744 [details]
Archive of layout-test-results from ews107 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6
Comment 6 Chris Dumez 2018-05-18 15:26:22 PDT
Created attachment 340747 [details]
Patch
Comment 7 EWS Watchlist 2018-05-18 21:02:57 PDT
Comment on attachment 340747 [details]
Patch

Attachment 340747 [details] did not pass win-ews (win):
Output: http://webkit-queues.webkit.org/results/7728784

New failing tests:
http/tests/security/contentSecurityPolicy/userAgentShadowDOM/allow-video.html
Comment 8 EWS Watchlist 2018-05-18 21:03:08 PDT
Created attachment 340770 [details]
Archive of layout-test-results from ews206 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews206  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Comment 9 Chris Dumez 2018-05-21 13:04:34 PDT
ping review?
Comment 10 youenn fablet 2018-05-21 13:20:03 PDT
Comment on attachment 340747 [details]
Patch

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

> Source/WebCore/fileapi/File.h:87
> +    const std::optional<double>& lastModifiedOverride() const { return m_overrideLastModifiedDate; }

Is there a reason to  pass a std::optional<double> in deserialize but here return a const& ?

> Source/WebCore/fileapi/File.h:115
> +    std::optional<double> m_overrideLastModifiedDate;

I guess this is mandated by File IDL?

> LayoutTests/imported/w3c/web-platform-tests/workers/semantics/structured-clone/shared-expected.txt:1
> +CONSOLE MESSAGE: line 19: ReferenceError: Can't find variable: SharedWorker

Might want to skip this one.
Comment 11 Chris Dumez 2018-05-21 13:25:19 PDT
Comment on attachment 340747 [details]
Patch

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

>> Source/WebCore/fileapi/File.h:115
>> +    std::optional<double> m_overrideLastModifiedDate;
> 
> I guess this is mandated by File IDL?

Actually, the IDL is using "long long" which maps to int64_t. I'll update my patch to use this consistently internally.
Comment 12 Chris Dumez 2018-05-21 13:27:33 PDT
Comment on attachment 340747 [details]
Patch

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

>> Source/WebCore/fileapi/File.h:87
>> +    const std::optional<double>& lastModifiedOverride() const { return m_overrideLastModifiedDate; }
> 
> Is there a reason to  pass a std::optional<double> in deserialize but here return a const& ?

I'll use const & consistently.
Comment 13 Chris Dumez 2018-05-21 14:09:10 PDT
Created attachment 340886 [details]
Patch
Comment 14 WebKit Commit Bot 2018-05-21 14:11:45 PDT
Comment on attachment 340886 [details]
Patch

Rejecting attachment 340886 [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-03', 'validate-changelog', '--check-oops', '--non-interactive', 340886, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

ChangeLog entry in LayoutTests/ChangeLog contains OOPS!.

Full output: http://webkit-queues.webkit.org/results/7756691
Comment 15 Chris Dumez 2018-05-21 14:19:04 PDT
Created attachment 340889 [details]
Patch
Comment 16 WebKit Commit Bot 2018-05-21 14:23:37 PDT
Comment on attachment 340889 [details]
Patch

Rejecting attachment 340889 [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', 340889, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Youen Fablet found in /Volumes/Data/EWS/WebKit/LayoutTests/imported/w3c/ChangeLog does not appear to be a valid reviewer according to contributors.json.
/Volumes/Data/EWS/WebKit/LayoutTests/imported/w3c/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

Full output: http://webkit-queues.webkit.org/results/7757009
Comment 17 Chris Dumez 2018-05-21 14:25:21 PDT
Created attachment 340892 [details]
Patch
Comment 18 WebKit Commit Bot 2018-05-21 14:53:53 PDT
Comment on attachment 340892 [details]
Patch

Clearing flags on attachment: 340892

Committed r232030: <https://trac.webkit.org/changeset/232030>
Comment 19 WebKit Commit Bot 2018-05-21 14:53:55 PDT
All reviewed patches have been landed.  Closing bug.
Comment 20 Radar WebKit Bug Importer 2018-05-21 14:55:22 PDT
<rdar://problem/40430827>
Comment 21 WebKit Commit Bot 2018-05-21 16:39:44 PDT
Re-opened since this is blocked by bug 185850
Comment 22 Chris Dumez 2018-05-21 18:42:31 PDT
Created attachment 340939 [details]
Patch
Comment 23 WebKit Commit Bot 2018-05-21 19:24:24 PDT
Comment on attachment 340939 [details]
Patch

Clearing flags on attachment: 340939

Committed r232043: <https://trac.webkit.org/changeset/232043>
Comment 24 WebKit Commit Bot 2018-05-21 19:24:26 PDT
All reviewed patches have been landed.  Closing bug.