Modified: trunk/Source/WebKit2/ChangeLog (174787 => 174788)
--- trunk/Source/WebKit2/ChangeLog 2014-10-16 19:45:31 UTC (rev 174787)
+++ trunk/Source/WebKit2/ChangeLog 2014-10-16 19:53:12 UTC (rev 174788)
@@ -1,3 +1,50 @@
+2014-10-16 Tim Horton <[email protected]>
+
+ Various crashes in ViewGestureControllerIOS when closing a tab while a swipe gesture is in progress
+ https://bugs.webkit.org/show_bug.cgi?id=137770
+ <rdar://problem/17916459>
+
+ Reviewed by Dan Bernstein.
+
+ When tearing down a WKWebView in the middle of a swipe gesture, a variety of
+ UI process crashes were observed. First, two uses of potentially deleted objects
+ (the WebBackForwardListItem and ViewGestureController), which were fixed by
+ extending the object's lifetime and checking for its liveness, respectively.
+ Second, a potential null-deref of DrawingArea if the timing of endSwipeGesture
+ vs. page teardown was such that DrawingArea was null but everything else was in line.
+ Lastly, another case of messaging a potentially deleted object (specifically,
+ the _UIViewControllerTransitionContext's animator) in a callback from CA, which
+ was fixed by nulling out the animator (and a few other properties) when tearing
+ down the ViewGestureController.
+
+ * UIProcess/ios/ViewGestureControllerIOS.mm:
+ (-[WKSwipeTransitionController invalidate]):
+ Clear the soon-to-be-invalid ViewGestureController pointer.
+
+ (WebKit::ViewGestureController::~ViewGestureController):
+ Call [WKSwipeTransitionController invalidate] upon destruction.
+ Clear our transition context's interactor and animator, and inform it that
+ the transition is not in flight. This avoids a crash when calling back
+ to the already-destroyed animator later.
+
+ (WebKit::ViewGestureController::beginSwipeGesture):
+ Keep a reference to the target WebBackForwardListItem; this avoids
+ it being deleted between here and the transition completion block firing.
+
+ Look up the ViewGestureController by pageID, just like we do in endSwipeGesture,
+ to avoid situations where the callback fires after the WKWebView/ViewGestureController
+ have gone away.
+
+ Hold on to our _UIViewControllerOneToOneTransitionContext, so that we can do the
+ aforementioned clearing upon deallocation.
+
+ (WebKit::ViewGestureController::endSwipeGesture):
+ Null check the DrawingArea. If it is null, instead of doing our normal delayed logic
+ for swipe snapshot teardown, just put things back together immediately.
+
+ (WebKit::ViewGestureController::removeSwipeSnapshot):
+ Clear m_swipeTransitionContext.
+
2014-10-16 Antti Koivisto <[email protected]>
REGRESSION (r173356): Downloading a disk image appends ".txt" to it
Modified: trunk/Source/WebKit2/UIProcess/ios/ViewGestureControllerIOS.mm (174787 => 174788)
--- trunk/Source/WebKit2/UIProcess/ios/ViewGestureControllerIOS.mm 2014-10-16 19:45:31 UTC (rev 174787)
+++ trunk/Source/WebKit2/UIProcess/ios/ViewGestureControllerIOS.mm 2014-10-16 19:53:12 UTC (rev 174788)
@@ -51,6 +51,7 @@
@interface WKSwipeTransitionController : NSObject <_UINavigationInteractiveTransitionBaseDelegate>
- (instancetype)initWithViewGestureController:(WebKit::ViewGestureController*)gestureController gestureRecognizerView:(UIView *)gestureRecognizerView;
+- (void)invalidate;
@end
@interface _UIViewControllerTransitionContext (WKDetails)
@@ -90,6 +91,11 @@
return self;
}
+- (void)invalidate
+{
+ _gestureController = nullptr;
+}
+
- (WebKit::ViewGestureController::SwipeDirection)directionForTransition:(_UINavigationInteractiveTransitionBase *)transition
{
return transition == _backTransitionController ? WebKit::ViewGestureController::SwipeDirection::Left : WebKit::ViewGestureController::SwipeDirection::Right;
@@ -145,6 +151,10 @@
ViewGestureController::~ViewGestureController()
{
+ [m_swipeTransitionContext _setTransitionIsInFlight:NO];
+ [m_swipeTransitionContext _setInteractor:nil];
+ [m_swipeTransitionContext _setAnimator:nil];
+ [m_swipeInteractiveTransitionDelegate invalidate];
viewGestureControllersForAllPages().remove(m_webPageProxy.pageID());
}
@@ -177,7 +187,7 @@
if (m_webPageProxyForBackForwardListForCurrentSwipe != &m_webPageProxy)
backForwardList.currentItem()->setSnapshot(m_webPageProxy.backForwardList().currentItem()->snapshot());
- WebBackForwardListItem* targetItem = direction == SwipeDirection::Left ? backForwardList.backItem() : backForwardList.forwardItem();
+ RefPtr<WebBackForwardListItem> targetItem = direction == SwipeDirection::Left ? backForwardList.backItem() : backForwardList.forwardItem();
CGRect liveSwipeViewFrame = [m_liveSwipeView frame];
@@ -215,26 +225,31 @@
UINavigationControllerOperation transitionOperation = direction == SwipeDirection::Left ? UINavigationControllerOperationPop : UINavigationControllerOperationPush;
RetainPtr<_UINavigationParallaxTransition> animationController = adoptNS([[_UINavigationParallaxTransition alloc] initWithCurrentOperation:transitionOperation]);
- RetainPtr<_UIViewControllerOneToOneTransitionContext> transitionContext = adoptNS([[_UIViewControllerOneToOneTransitionContext alloc] init]);
- [transitionContext _setFromViewController:targettedViewController.get()];
- [transitionContext _setToViewController:snapshotViewController.get()];
- [transitionContext _setContainerView:m_transitionContainerView.get()];
- [transitionContext _setFromStartFrame:liveSwipeViewFrame];
- [transitionContext _setToEndFrame:liveSwipeViewFrame];
- [transitionContext _setToStartFrame:CGRectZero];
- [transitionContext _setFromEndFrame:CGRectZero];
- [transitionContext _setAnimator:animationController.get()];
- [transitionContext _setInteractor:transition];
- [transitionContext _setTransitionIsInFlight:YES];
- [transitionContext _setInteractiveUpdateHandler:^(BOOL finish, CGFloat percent, BOOL transitionCompleted, _UIViewControllerTransitionContext *) {
+ m_swipeTransitionContext = adoptNS([[_UIViewControllerOneToOneTransitionContext alloc] init]);
+ [m_swipeTransitionContext _setFromViewController:targettedViewController.get()];
+ [m_swipeTransitionContext _setToViewController:snapshotViewController.get()];
+ [m_swipeTransitionContext _setContainerView:m_transitionContainerView.get()];
+ [m_swipeTransitionContext _setFromStartFrame:liveSwipeViewFrame];
+ [m_swipeTransitionContext _setToEndFrame:liveSwipeViewFrame];
+ [m_swipeTransitionContext _setToStartFrame:CGRectZero];
+ [m_swipeTransitionContext _setFromEndFrame:CGRectZero];
+ [m_swipeTransitionContext _setAnimator:animationController.get()];
+ [m_swipeTransitionContext _setInteractor:transition];
+ [m_swipeTransitionContext _setTransitionIsInFlight:YES];
+ [m_swipeTransitionContext _setInteractiveUpdateHandler:^(BOOL finish, CGFloat percent, BOOL transitionCompleted, _UIViewControllerTransitionContext *) {
if (finish)
m_webPageProxyForBackForwardListForCurrentSwipe->navigationGestureWillEnd(transitionCompleted, *targetItem);
}];
- [transitionContext _setCompletionHandler:^(_UIViewControllerTransitionContext *context, BOOL didComplete) { endSwipeGesture(targetItem, context, !didComplete); }];
- [transitionContext _setInteractiveUpdateHandler:^(BOOL, CGFloat, BOOL, _UIViewControllerTransitionContext *) { }];
+ uint64_t pageID = m_webPageProxy.pageID();
+ [m_swipeTransitionContext _setCompletionHandler:^(_UIViewControllerTransitionContext *context, BOOL didComplete) {
+ auto gestureControllerIter = viewGestureControllersForAllPages().find(pageID);
+ if (gestureControllerIter != viewGestureControllersForAllPages().end())
+ gestureControllerIter->value->endSwipeGesture(targetItem.get(), context, !didComplete);
+ }];
+ [m_swipeTransitionContext _setInteractiveUpdateHandler:^(BOOL, CGFloat, BOOL, _UIViewControllerTransitionContext *) { }];
[transition setAnimationController:animationController.get()];
- [transition startInteractiveTransition:transitionContext.get()];
+ [transition startInteractiveTransition:m_swipeTransitionContext.get()];
m_activeGestureType = ViewGestureType::Swipe;
}
@@ -275,12 +290,17 @@
m_webPageProxyForBackForwardListForCurrentSwipe->navigationGestureDidEnd(true, *targetItem);
m_webPageProxyForBackForwardListForCurrentSwipe->goToBackForwardItem(targetItem);
- uint64_t pageID = m_webPageProxy.pageID();
- m_webPageProxy.drawingArea()->dispatchAfterEnsuringDrawing([pageID] (CallbackBase::Error error) {
- auto gestureControllerIter = viewGestureControllersForAllPages().find(pageID);
- if (gestureControllerIter != viewGestureControllersForAllPages().end())
- gestureControllerIter->value->willCommitPostSwipeTransitionLayerTree(error == CallbackBase::Error::None);
- });
+ if (auto drawingArea = m_webPageProxy.drawingArea()) {
+ uint64_t pageID = m_webPageProxy.pageID();
+ drawingArea->dispatchAfterEnsuringDrawing([pageID] (CallbackBase::Error error) {
+ auto gestureControllerIter = viewGestureControllersForAllPages().find(pageID);
+ if (gestureControllerIter != viewGestureControllersForAllPages().end())
+ gestureControllerIter->value->willCommitPostSwipeTransitionLayerTree(error == CallbackBase::Error::None);
+ });
+ } else {
+ removeSwipeSnapshot();
+ return;
+ }
m_swipeWatchdogTimer.startOneShot(swipeSnapshotRemovalWatchdogDuration.count());
}
@@ -338,6 +358,8 @@
m_webPageProxyForBackForwardListForCurrentSwipe->navigationGestureSnapshotWasRemoved();
m_webPageProxyForBackForwardListForCurrentSwipe = nullptr;
+
+ m_swipeTransitionContext = nullptr;
}
} // namespace WebKit
Modified: trunk/Source/WebKit2/UIProcess/mac/ViewGestureController.h (174787 => 174788)
--- trunk/Source/WebKit2/UIProcess/mac/ViewGestureController.h 2014-10-16 19:45:31 UTC (rev 174787)
+++ trunk/Source/WebKit2/UIProcess/mac/ViewGestureController.h 2014-10-16 19:53:12 UTC (rev 174788)
@@ -39,8 +39,9 @@
OBJC_CLASS UIView;
OBJC_CLASS WKSwipeTransitionController;
OBJC_CLASS WKWebView;
+OBJC_CLASS _UINavigationInteractiveTransitionBase;
+OBJC_CLASS _UIViewControllerOneToOneTransitionContext;
OBJC_CLASS _UIViewControllerTransitionContext;
-OBJC_CLASS _UINavigationInteractiveTransitionBase;
#else
OBJC_CLASS NSEvent;
OBJC_CLASS NSView;
@@ -207,6 +208,7 @@
RetainPtr<UIView> m_snapshotView;
RetainPtr<UIView> m_transitionContainerView;
RetainPtr<WKSwipeTransitionController> m_swipeInteractiveTransitionDelegate;
+ RetainPtr<_UIViewControllerOneToOneTransitionContext> m_swipeTransitionContext;
uint64_t m_snapshotRemovalTargetRenderTreeSize;
bool m_shouldRemoveSnapshotWhenTargetRenderTreeSizeHit;
WeakObjCPtr<WKWebView> m_alternateBackForwardListSourceView;
