Use WKDataTask for SystemPreview downloads #13171
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
grorg
added
the
WebKit Misc.
|
EWS run on previous version of this PR (hash 608e61e) |
grorg
requested review from
hortont424 and
achristensen07
and removed request for
rniwa and
cdumez
hortont424
approved these changes
Apr 25, 2023
| return; | ||
| } | ||
|
|
||
| [_data.get() appendData:data]; |
| @@ -59,6 +61,8 @@ + (void)launchPreviewApplicationWithURLs:(NSArray *)urls completion:(void (^)(NS | |||
|
|
|||
| static NSString * const _WKARQLWebsiteURLParameterKey = @"ARQLWebsiteURLParameterKey"; | |||
|
|
|||
| const int64_t SystemPreviewMaximumSize = 40 * 1024 * 1024; | |||
There was a problem hiding this comment.
This seems awfully tiny
There was a problem hiding this comment.
I will remove the limit.
| auto filePath = FileSystem::openTemporaryFile("SystemPreview"_s, fileHandle, ".usdz"_s); | ||
| ASSERT(FileSystem::isHandleValid(fileHandle)); | ||
|
|
||
| size_t byteCount = FileSystem::writeToFile(fileHandle, _data.get().bytes, _data.get().length); |
There was a problem hiding this comment.
I would [_data bytes] (etc.), but I know some people like this instead?
webkit-ews-buildbot
added
the
merging-blocked
pxlcoder
reviewed
Apr 25, 2023
| @interface _WKSystemPreviewDataTaskDelegate : NSObject <_WKDataTaskDelegate> { | ||
| WebKit::SystemPreviewController* _previewController; | ||
| long long _expectedContentLength; | ||
| long long _receivedLength; |
There was a problem hiding this comment.
I don't see this variable used anywhere?
grorg
force-pushed
the
eng/Use-WKDataTask-for-SystemPreview-downloads
branch
from
608e61e
to
f7acd1d
Compare
|
EWS run on previous version of this PR (hash f7acd1d) |
grorg
force-pushed
the
eng/Use-WKDataTask-for-SystemPreview-downloads
branch
from
f7acd1d
to
adb2bc1
Compare
|
EWS run on previous version of this PR (hash adb2bc1) |
grorg
force-pushed
the
eng/Use-WKDataTask-for-SystemPreview-downloads
branch
from
adb2bc1
to
65b5f69
Compare
grorg
added
unsafe-merge-queue
|
EWS run on current version of this PR (hash 65b5f69)
|
webkit-commit-queue
force-pushed
the
eng/Use-WKDataTask-for-SystemPreview-downloads
branch
from
65b5f69
to
369b7ba
Compare
https://bugs.webkit.org/show_bug.cgi?id=255954 rdar://108524222 Reviewed by Tim Horton. Migrate away from LegacyDownloadClient and instead handle System Previews directly. This will allow us to remove the horrible code in the download client, as well eventually allow 3rd-party engines to use ARQL. The fix is to intercept the HTMLAnchorElement click for a system preview, and send that directly to the SystemPreviewController in the UI Process. That object then creates a WKDataTask for the download and presents ARQL as normal. A followup patch will remove the old code. * Source/WebCore/html/HTMLAnchorElement.cpp: * Source/WebCore/page/ChromeClient.h: * Source/WebCore/page/Page.cpp: * Source/WebCore/page/Page.h: * Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivateForTesting.h: * Source/WebKit/UIProcess/API/Cocoa/WKWebViewTesting.mm: * Source/WebKit/UIProcess/Cocoa/SystemPreviewControllerCocoa.mm: * Source/WebKit/UIProcess/SystemPreviewController.h: * Source/WebKit/UIProcess/WebPageProxy.cpp: * Source/WebKit/UIProcess/WebPageProxy.h: * Source/WebKit/UIProcess/WebPageProxy.messages.in: * Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp: * Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.h: * Tools/TestWebKitAPI/Tests/WebKitCocoa/SystemPreview.mm: (-[TestSystemPreviewUIDelegate _presentingViewControllerForWebView:]): (TestWebKitAPI::TEST): * Tools/TestWebKitAPI/Tests/WebKitCocoa/UnitBox.usdz: Added. * Tools/TestWebKitAPI/Tests/WebKitCocoa/system-preview-trigger.html: * Tools/TestWebKitAPI/Tests/WebKitCocoa/system-preview.html: Added. Canonical link: https://commits.webkit.org/263393@main
webkit-commit-queue
force-pushed
the
eng/Use-WKDataTask-for-SystemPreview-downloads
branch
from
369b7ba
to
d55e3ca
Compare
|
Committed 263393@main (d55e3ca): https://commits.webkit.org/263393@main Reviewed commits have been landed. Closing PR #13171 and removing active labels. |
webkit-commit-queue
removed
the
unsafe-merge-queue
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
d55e3ca
65b5f69
🛠 ios🛠 wincairo🧪 bindings🛠 ios-sim🛠 mac-AS-debug🧪 wpe-wk2🧪 webkitperl🧪 ios-wk2🧪 api-mac🧪 ios-wk2-wpt🧪 mac-wk1🧪 gtk-wk2🧪 api-ios🧪 mac-wk2🧪 api-gtk🛠 tv🧪 mac-AS-debug-wk2🛠 tv-sim🧪 mac-wk2-stress🛠 watch🛠 watch-sim