Bug 186875 - [Cocoa] Use the isDirectory: variants of NSURL methods more to eliminate unnecessary file system activity
Summary: [Cocoa] Use the isDirectory: variants of NSURL methods more to eliminate unne...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Darin Adler
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-06-20 20:16 PDT by Darin Adler
Modified: 2018-06-22 08:07 PDT (History)
8 users (show)

See Also:

Attachments
Patch (21.10 KB, patch)
2018-06-20 20:28 PDT, Darin Adler 		andersca: review+
ews-watchlist: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews202 for win-future (12.50 MB, application/zip)
2018-06-21 02:55 PDT, EWS Watchlist 		no flags Details
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Darin Adler 2018-06-20 20:16:37 PDT 
[Cocoa] Use the isDirectory: variants of NSURL methods more to eliminate unnecessary file system activity
Comment 1 Darin Adler 2018-06-20 20:28:24 PDT 
Created attachment 343208 [details]
Patch
Comment 2 Conrad Shultz 2018-06-20 22:40:09 PDT 
Comment on attachment 343208 [details]
Patch

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

Looks good to me, though I can't r+.

> Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm:3962
> +    if (![wrapper writeToURL:[NSURL fileURLWithPath:path isDirectory:NO] options:NSFileWrapperWritingWithNameUpdating originalContentsURL:nil error:nullptr])

A few lines earlier, we use `m_promisedURL` when initializing the file wrapper, but use the plain `+URLWithString:`. It seems like `m_promisedURL` must also be a file URL, so should that be changed?

Since this is apparently used only for saving images, this change seems correct, but I'm a little wary since file wrappers can refer to directories.
Comment 3 EWS Watchlist 2018-06-21 02:55:23 PDT 
Comment on attachment 343208 [details]
Patch

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

New failing tests:
http/tests/security/canvas-remote-read-remote-video-blocked-no-crossorigin.html
Comment 4 EWS Watchlist 2018-06-21 02:55:34 PDT 
Created attachment 343228 [details]
Archive of layout-test-results from ews202 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews202  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Comment 5 Darin Adler 2018-06-22 08:04:02 PDT 
Committed r233079: <https://trac.webkit.org/changeset/233079>
Comment 6 Radar WebKit Bug Importer 2018-06-22 08:07:26 PDT 
<rdar://problem/41368124>