Title: [175093] branches/safari-600.1.4.12-branch/Source/WebKit2

Diff

Modified: branches/safari-600.1.4.12-branch/Source/WebKit2/ChangeLog (175092 => 175093)


--- branches/safari-600.1.4.12-branch/Source/WebKit2/ChangeLog	2014-10-23 07:06:12 UTC (rev 175092)
+++ branches/safari-600.1.4.12-branch/Source/WebKit2/ChangeLog	2014-10-23 07:08:26 UTC (rev 175093)
@@ -1,5 +1,56 @@
 2014-10-23  Babak Shafiei  <[email protected]>
 
+        Merge r174788.
+
+    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-23  Babak Shafiei  <[email protected]>
+
         Merge r174512.
 
     2014-10-09  Andy Estes  <[email protected]>

Modified: branches/safari-600.1.4.12-branch/Source/WebKit2/UIProcess/ios/ViewGestureControllerIOS.mm (175092 => 175093)


--- branches/safari-600.1.4.12-branch/Source/WebKit2/UIProcess/ios/ViewGestureControllerIOS.mm	2014-10-23 07:06:12 UTC (rev 175092)
+++ branches/safari-600.1.4.12-branch/Source/WebKit2/UIProcess/ios/ViewGestureControllerIOS.mm	2014-10-23 07:08:26 UTC (rev 175093)
@@ -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: branches/safari-600.1.4.12-branch/Source/WebKit2/UIProcess/mac/ViewGestureController.h (175092 => 175093)


--- branches/safari-600.1.4.12-branch/Source/WebKit2/UIProcess/mac/ViewGestureController.h	2014-10-23 07:06:12 UTC (rev 175092)
+++ branches/safari-600.1.4.12-branch/Source/WebKit2/UIProcess/mac/ViewGestureController.h	2014-10-23 07:08:26 UTC (rev 175093)
@@ -38,8 +38,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;
@@ -187,6 +188,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;
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to