Move animated resize into the layer tree transaction, and make it asynchronous
Created attachment 341649 [details] Patch
Not sure this is 100% done (and it definitely needs a changelog), but it’s getting there.
Created attachment 341717 [details] Patch
Very cool! Given how complex this logic is, do you have any ideas about how we could add automated tests for this?
Pretty sure we already have API tests for animated resize. I’m not sure it’s worth it to write an API test for “animated resize doesn’t synchronously block the UI process”, though.
It is possible we could test some of the smaller changes, though.
Comment on attachment 341717 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=341717&action=review > Source/WebKit/Shared/RemoteLayerTree/RemoteLayerTreeTransaction.h:274 > + std::optional<uint64_t> dynamicViewportSizeUpdateID() const { return m_dynamicViewportSizeUpdateID; } Why do we not have typedefs for uint64_t things. > Source/WebKit/Shared/RemoteLayerTree/RemoteLayerTreeTransaction.h:312 > + std::optional<uint64_t> m_dynamicViewportSizeUpdateID { std::nullopt }; No need to initialize to nullopt. Same with the one above. > Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:352 > + BOOL _waitingForEndAnimatedResize; > + BOOL _waitingForCommitAfterAnimatedResize; Do these get cleared in "the web process did crash" code path? > Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:1758 > +- (void)_didCommitLayerTreeDuringAnimatedResize:(const WebKit::RemoteLayerTreeTransaction&)layerTreeTransaction Things kinda sounds like it should return a BOOL, but I can't think of a better name. > Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:2917 > + if (!_lastSentViewLayoutSize || newViewLayoutSize != _lastSentViewLayoutSize.value()) > + [self _dispatchSetViewLayoutSize:newViewLayoutSize]; > + if (!_lastSentMaximumUnobscuredSize || newMaximumUnobscuredSize != _lastSentMaximumUnobscuredSize.value()) > + [self _dispatchSetMaximumUnobscuredSize:WebCore::FloatSize(newMaximumUnobscuredSize)]; > + if (!_lastSentDeviceOrientation || newOrientation != _lastSentDeviceOrientation.value()) > + [self _dispatchSetDeviceOrientation:newOrientation]; Some blank lines between these would be nice. > Source/WebKit/UIProcess/ios/WebPageProxyIOS.mm:316 > + m_process->send(Messages::WebPage::DynamicViewportSizeUpdate(viewLayoutSize, maximumUnobscuredSize, targetExposedContentRect, targetUnobscuredRect, targetUnobscuredRectInScrollViewCoordinates, unobscuredSafeAreaInsets, targetScale, deviceOrientation, dynamicViewportSizeUpdateID), m_pageID); Maybe unwrap this. > Source/WebKit/WebProcess/WebPage/WebPage.h:1658 > uint64_t m_currentAssistedNodeIdentifier { 0 }; Oh look a uint64_t that's something totally different.
Created attachment 342020 [details] Patch
Still have to make the crash case work right. There’s a bunch of preexisting problems, I’m just adding some more :)
Created attachment 342036 [details] Patch
Created attachment 342037 [details] Patch
(In reply to Tim Horton from comment #9) > Still have to make the crash case work right. There’s a bunch of preexisting > problems, I’m just adding some more :) Not true! It’s actually fine. But we should clean the existing mechanism up, because it duplicates a lot of code.
<rdar://problem/38477288>
Comment on attachment 342037 [details] Patch Attachment 342037 [details] did not pass win-ews (win): Output: http://webkit-queues.webkit.org/results/8027639 New failing tests: storage/domstorage/storage-close-database-on-idle.html
Created attachment 342041 [details] Archive of layout-test-results from ews200 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews200 Port: win-future Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Comment on attachment 342037 [details] Patch Clearing flags on attachment: 342037 Committed r232544: <https://trac.webkit.org/changeset/232544>
All reviewed patches have been landed. Closing bug.