[iOS] Occasional crash under -[UIPreviewTarget initWithContainer:center:transform...
authorwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 25 Jun 2019 18:48:25 +0000 (18:48 +0000)
committerwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 25 Jun 2019 18:48:25 +0000 (18:48 +0000)
https://bugs.webkit.org/show_bug.cgi?id=199192
<rdar://problem/51554509>

Reviewed by Tim Horton.

Source/WebKit:

Tweak our preview generation code (for both the context menu and dragging) to be robust in the case where the
content view's unscaled view is nil; this may happen in the case after the web content process is terminated
and -cleanupInteraction is called, but before -setupInteraction is subsequently called.

Additionally, make our logic for creating targeted previews robust in the case where the view is removed from
the view hierarchy right before the platform asks for a targeted preview.

Test:   DragAndDropTests.WebProcessTerminationDuringDrag
        DragAndDropTests.WebViewRemovedFromViewHierarchyDuringDrag

* UIProcess/ios/WKContentViewInteraction.mm:
(-[WKContentView containerViewForTargetedPreviews]):
(-[WKContentView _deliverDelayedDropPreviewIfPossible:]):
(-[WKContentView dragInteraction:previewForLiftingItem:session:]):
(-[WKContentView _ensureTargetedPreview]):

Tools:

Tweak the drag and drop simulator to ask for drag cancellation previews, and use this to write a couple tests to
verify that we gracefully handle web process termination and web view unparenting mid-drag.

* TestWebKitAPI/Tests/ios/DragAndDropTestsIOS.mm:
(TestWebKitAPI::TEST):
* TestWebKitAPI/cocoa/DragAndDropSimulator.h:
* TestWebKitAPI/ios/DragAndDropSimulatorIOS.mm:
(-[DragAndDropSimulator _resetSimulatedState]):
(-[DragAndDropSimulator _concludeDropAndPerformOperationIfNecessary]):
(-[DragAndDropSimulator _advanceProgress]):
(-[DragAndDropSimulator liftPreviews]):
(-[DragAndDropSimulator cancellationPreviews]):
(-[DragAndDropSimulator setSessionWillBeginBlock:]):
(-[DragAndDropSimulator sessionWillBeginBlock]):
(-[DragAndDropSimulator _webView:dataInteraction:sessionWillBegin:]):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@246800 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebKit/ChangeLog
Source/WebKit/UIProcess/ios/DragDropInteractionState.mm
Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/ios/DragAndDropTestsIOS.mm
Tools/TestWebKitAPI/cocoa/DragAndDropSimulator.h
Tools/TestWebKitAPI/ios/DragAndDropSimulatorIOS.mm

index 9dec21a..7851cdd 100644 (file)
@@ -1,3 +1,27 @@
+2019-06-25  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [iOS] Occasional crash under -[UIPreviewTarget initWithContainer:center:transform:] when generating a drag preview
+        https://bugs.webkit.org/show_bug.cgi?id=199192
+        <rdar://problem/51554509>
+
+        Reviewed by Tim Horton.
+
+        Tweak our preview generation code (for both the context menu and dragging) to be robust in the case where the
+        content view's unscaled view is nil; this may happen in the case after the web content process is terminated
+        and -cleanupInteraction is called, but before -setupInteraction is subsequently called.
+
+        Additionally, make our logic for creating targeted previews robust in the case where the view is removed from
+        the view hierarchy right before the platform asks for a targeted preview.
+
+        Test:   DragAndDropTests.WebProcessTerminationDuringDrag
+                DragAndDropTests.WebViewRemovedFromViewHierarchyDuringDrag
+
+        * UIProcess/ios/WKContentViewInteraction.mm:
+        (-[WKContentView containerViewForTargetedPreviews]):
+        (-[WKContentView _deliverDelayedDropPreviewIfPossible:]):
+        (-[WKContentView dragInteraction:previewForLiftingItem:session:]):
+        (-[WKContentView _ensureTargetedPreview]):
+
 2019-06-25  Youenn Fablet  <youenn@apple.com>
 
         Close sockets with too high file descriptor
index 8f2807b..af276a3 100644 (file)
@@ -46,7 +46,7 @@ static UIDragItem *dragItemMatchingIdentifier(id <UIDragSession> session, NSInte
 
 static RetainPtr<UITargetedDragPreview> createTargetedDragPreview(UIImage *image, UIView *rootView, UIView *previewContainer, const FloatRect& frameInRootViewCoordinates, const Vector<FloatRect>& clippingRectsInFrameCoordinates, UIColor *backgroundColor, UIBezierPath *visiblePath)
 {
-    if (frameInRootViewCoordinates.isEmpty() || !image)
+    if (frameInRootViewCoordinates.isEmpty() || !image || !previewContainer.window)
         return nullptr;
 
     NSMutableArray *clippingRectValuesInFrameCoordinates = [NSMutableArray arrayWithCapacity:clippingRectsInFrameCoordinates.size()];
index 53b0be9..4d42f0b 100644 (file)
@@ -6214,6 +6214,11 @@ ALLOW_DEPRECATED_DECLARATIONS_END
     return NO;
 }
 
+- (UIView *)containerViewForTargetedPreviews
+{
+    return self.unscaledView ?: self;
+}
+
 #if ENABLE(DRAG_SUPPORT)
 
 static BOOL shouldEnableDragInteractionForPolicy(_WKDragInteractionPolicy policy)
@@ -6446,7 +6451,7 @@ static NSArray<NSItemProvider *> *extractItemProvidersFromDropSession(id <UIDrop
     [_unselectedContentSnapshot setFrame:data->contentImageWithoutSelectionRectInRootViewCoordinates];
 
     [self insertSubview:_unselectedContentSnapshot.get() belowSubview:_visibleContentViewSnapshot.get()];
-    _dragDropInteractionState.deliverDelayedDropPreview(self, self.unscaledView, data.value());
+    _dragDropInteractionState.deliverDelayedDropPreview(self, self.containerViewForTargetedPreviews, data.value());
 }
 
 - (void)_didPerformDragOperation:(BOOL)handled
@@ -6792,7 +6797,7 @@ static WebKit::DocumentEditingContextRequest toWebRequest(UIWKDocumentRequest *r
         if (overriddenPreview)
             return overriddenPreview;
     }
-    return _dragDropInteractionState.previewForDragItem(item, self, self.unscaledView);
+    return _dragDropInteractionState.previewForDragItem(item, self, self.containerViewForTargetedPreviews);
 }
 
 - (void)dragInteraction:(UIDragInteraction *)interaction willAnimateLiftWithAnimator:(id <UIDragAnimating>)animator session:(id <UIDragSession>)session
@@ -7880,7 +7885,7 @@ static RetainPtr<UIImage> uiImageForImage(WebCore::Image* image)
 // FIXME: This should be merged with createTargetedDragPreview in DragDropInteractionState.
 static RetainPtr<UITargetedPreview> createTargetedPreview(UIImage *image, UIView *rootView, UIView *previewContainer, const WebCore::FloatRect& frameInRootViewCoordinates, const Vector<WebCore::FloatRect>& clippingRectsInFrameCoordinates, UIColor *backgroundColor)
 {
-    if (frameInRootViewCoordinates.isEmpty() || !image)
+    if (frameInRootViewCoordinates.isEmpty() || !image || !previewContainer.window)
         return nil;
 
     WebCore::FloatRect frameInContainerCoordinates = [rootView convertRect:frameInRootViewCoordinates toView:previewContainer];
@@ -7936,15 +7941,15 @@ static RetainPtr<UITargetedPreview> createFallbackTargetedPreview(UIView *rootVi
     if (_positionInformation.isLink && _positionInformation.linkIndicator.contentImage) {
         auto indicator = _positionInformation.linkIndicator;
         auto textIndicatorImage = uiImageForImage(indicator.contentImage.get());
-        targetedPreview = createTargetedPreview(textIndicatorImage.get(), self, self.unscaledView, indicator.textBoundingRectInRootViewCoordinates, indicator.textRectsInBoundingRectCoordinates, [UIColor colorWithCGColor:cachedCGColor(indicator.estimatedBackgroundColor)]);
+        targetedPreview = createTargetedPreview(textIndicatorImage.get(), self, self.containerViewForTargetedPreviews, indicator.textBoundingRectInRootViewCoordinates, indicator.textRectsInBoundingRectCoordinates, [UIColor colorWithCGColor:cachedCGColor(indicator.estimatedBackgroundColor)]);
     } else if ((_positionInformation.isAttachment || _positionInformation.isImage) && _positionInformation.image) {
         auto cgImage = _positionInformation.image->makeCGImageCopy();
         auto image = adoptNS([[UIImage alloc] initWithCGImage:cgImage.get()]);
-        targetedPreview = createTargetedPreview(image.get(), self, self.unscaledView, _positionInformation.bounds, { }, nil);
+        targetedPreview = createTargetedPreview(image.get(), self, self.containerViewForTargetedPreviews, _positionInformation.bounds, { }, nil);
     }
 
     if (!targetedPreview)
-        targetedPreview = createFallbackTargetedPreview(self, self.unscaledView, _positionInformation.bounds);
+        targetedPreview = createFallbackTargetedPreview(self, self.containerViewForTargetedPreviews, _positionInformation.bounds);
 
     _contextMenuInteractionTargetedPreview = WTFMove(targetedPreview);
     return _contextMenuInteractionTargetedPreview.get();
index 8dd1f5f..8f56538 100644 (file)
@@ -1,3 +1,27 @@
+2019-06-25  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [iOS] Occasional crash under -[UIPreviewTarget initWithContainer:center:transform:] when generating a drag preview
+        https://bugs.webkit.org/show_bug.cgi?id=199192
+        <rdar://problem/51554509>
+
+        Reviewed by Tim Horton.
+
+        Tweak the drag and drop simulator to ask for drag cancellation previews, and use this to write a couple tests to
+        verify that we gracefully handle web process termination and web view unparenting mid-drag.
+
+        * TestWebKitAPI/Tests/ios/DragAndDropTestsIOS.mm:
+        (TestWebKitAPI::TEST):
+        * TestWebKitAPI/cocoa/DragAndDropSimulator.h:
+        * TestWebKitAPI/ios/DragAndDropSimulatorIOS.mm:
+        (-[DragAndDropSimulator _resetSimulatedState]):
+        (-[DragAndDropSimulator _concludeDropAndPerformOperationIfNecessary]):
+        (-[DragAndDropSimulator _advanceProgress]):
+        (-[DragAndDropSimulator liftPreviews]):
+        (-[DragAndDropSimulator cancellationPreviews]):
+        (-[DragAndDropSimulator setSessionWillBeginBlock:]):
+        (-[DragAndDropSimulator sessionWillBeginBlock]):
+        (-[DragAndDropSimulator _webView:dataInteraction:sessionWillBegin:]):
+
 2019-06-25  Aakash Jain  <aakash_jain@apple.com>
 
         [ews-build] UploadTestResults and ExtractTestResults clobber results in case of multiple layout test runs in a build
index f110b24..4709047 100644 (file)
@@ -1445,6 +1445,30 @@ TEST(DragAndDropTests, CancelledLiftDoesNotCauseSubsequentDragsToFail)
     checkStringArraysAreEqual(@[@"dragstart", @"dragend"], [outputText componentsSeparatedByString:@" "]);
 }
 
+TEST(DragAndDropTests, WebProcessTerminationDuringDrag)
+{
+    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, 500)]);
+    [webView synchronouslyLoadTestPageNamed:@"link-and-target-div"];
+    auto simulator = adoptNS([[DragAndDropSimulator alloc] initWithWebView:webView.get()]);
+    [simulator setSessionWillBeginBlock:^{
+        [webView _killWebContentProcessAndResetState];
+    }];
+    [simulator runFrom:CGPointMake(100, 50) to:CGPointMake(300, 50)];
+}
+
+TEST(DragAndDropTests, WebViewRemovedFromViewHierarchyDuringDrag)
+{
+    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, 500)]);
+    [webView synchronouslyLoadTestPageNamed:@"link-and-target-div"];
+    auto simulator = adoptNS([[DragAndDropSimulator alloc] initWithWebView:webView.get()]);
+    [simulator setConvertItemProvidersBlock:[webView] (NSItemProvider *item, NSArray *, NSDictionary *) -> NSArray * {
+        [webView removeFromSuperview];
+        return @[ item ];
+    }];
+    [simulator runFrom:CGPointMake(100, 50) to:CGPointMake(300, 50)];
+    EXPECT_EQ([simulator cancellationPreviews].firstObject, nil);
+}
+
 static void testDragAndDropOntoTargetElements(TestWKWebView *webView)
 {
     auto simulator = adoptNS([[DragAndDropSimulator alloc] initWithWebView:webView]);
index 3ad6698..7f31c4f 100644 (file)
@@ -104,6 +104,7 @@ typedef NSDictionary<NSNumber *, NSValue *> *ProgressToCGPointValueMap;
 @property (nonatomic, copy) NSArray *(^convertItemProvidersBlock)(NSItemProvider *, NSArray *, NSDictionary *);
 @property (nonatomic, copy) NSArray *(^overridePerformDropBlock)(id <UIDropSession>);
 @property (nonatomic, copy) void(^dropCompletionBlock)(BOOL, NSArray *);
+@property (nonatomic, copy) dispatch_block_t sessionWillBeginBlock;
 @property (nonatomic, copy) UIDropOperation(^overrideDragUpdateBlock)(UIDropOperation, id <UIDropSession>);
 
 @property (nonatomic, readonly) NSArray *sourceItemProviders;
@@ -111,6 +112,7 @@ typedef NSDictionary<NSNumber *, NSValue *> *ProgressToCGPointValueMap;
 @property (nonatomic, readonly) CGRect finalSelectionStartRect;
 @property (nonatomic, readonly) CGRect lastKnownDragCaretRect;
 @property (nonatomic, readonly) NSArray<UITargetedDragPreview *> *liftPreviews;
+@property (nonatomic, readonly) NSArray<UITargetedDragPreview *> *cancellationPreviews;
 @property (nonatomic, readonly) NSArray *dropPreviews;
 @property (nonatomic, readonly) NSArray *delayedDropPreviews;
 
index 147c36a..96d6ecb 100644 (file)
@@ -310,7 +310,8 @@ IGNORE_WARNINGS_END
 
     RetainPtr<NSMutableDictionary<NSNumber *, NSValue *>>_remainingAdditionalItemRequestLocationsByProgress;
     RetainPtr<NSMutableArray<NSValue *>>_queuedAdditionalItemRequestLocations;
-    RetainPtr<NSMutableArray<UITargetedDragPreview *>> _liftPreviews;
+    RetainPtr<NSMutableArray> _liftPreviews;
+    RetainPtr<NSMutableArray<UITargetedDragPreview *>> _cancellationPreviews;
     RetainPtr<NSMutableArray> _dropPreviews;
     RetainPtr<NSMutableArray> _delayedDropPreviews;
 
@@ -330,6 +331,7 @@ IGNORE_WARNINGS_END
     BlockPtr<NSArray *(id <UIDropSession>)> _overridePerformDropBlock;
     BlockPtr<UIDropOperation(UIDropOperation, id)> _overrideDragUpdateBlock;
     BlockPtr<void(BOOL, NSArray *)> _dropCompletionBlock;
+    BlockPtr<void()> _sessionWillBeginBlock;
 }
 
 - (instancetype)initWithWebViewFrame:(CGRect)frame
@@ -388,6 +390,7 @@ IGNORE_WARNINGS_END
     _queuedAdditionalItemRequestLocations = adoptNS([[NSMutableArray alloc] init]);
     _liftPreviews = adoptNS([[NSMutableArray alloc] init]);
     _dropPreviews = adoptNS([[NSMutableArray alloc] init]);
+    _cancellationPreviews = adoptNS([[NSMutableArray alloc] init]);
     _delayedDropPreviews = adoptNS([[NSMutableArray alloc] init]);
     _hasStartedInputSession = false;
 }
@@ -497,6 +500,15 @@ IGNORE_WARNINGS_END
     } else {
         _isDoneWithCurrentRun = true;
         _phase = DragAndDropPhaseCancelled;
+        [[_dropSession items] enumerateObjectsUsingBlock:^(UIDragItem *item, NSUInteger index, BOOL *) {
+            UITargetedDragPreview *defaultPreview = nil;
+            if ([_liftPreviews count] && [[_liftPreviews objectAtIndex:index] isEqual:NSNull.null])
+                defaultPreview = [_liftPreviews objectAtIndex:index];
+
+            UITargetedDragPreview *preview = [[_webView dragInteractionDelegate] dragInteraction:[_webView dragInteraction] previewForCancellingItem:item withDefault:defaultPreview];
+            if (preview)
+                [_cancellationPreviews addObject:preview];
+        }];
         [[_webView dropInteractionDelegate] dropInteraction:[_webView dropInteraction] concludeDrop:_dropSession.get()];
     }
 
@@ -576,9 +588,8 @@ IGNORE_WARNINGS_END
         for (UIDragItem *item in items) {
             [itemProviders addObject:item.itemProvider];
             UITargetedDragPreview *liftPreview = [[_webView dragInteractionDelegate] dragInteraction:[_webView dragInteraction] previewForLiftingItem:item session:_dragSession.get()];
-            EXPECT_TRUE(!!liftPreview);
-            if (liftPreview)
-                [_liftPreviews addObject:liftPreview];
+            EXPECT_TRUE(liftPreview || ![_webView window]);
+            [_liftPreviews addObject:liftPreview ?: NSNull.null];
         }
 
         _dropSession = adoptNS([[MockDropSession alloc] initWithProviders:itemProviders location:self._currentLocation window:[_webView window] allowMove:self.shouldAllowMoveOperation]);
@@ -662,11 +673,16 @@ IGNORE_WARNINGS_END
     return _phase;
 }
 
-- (NSArray<UITargetedDragPreview *> *)liftPreviews
+- (NSArray *)liftPreviews
 {
     return _liftPreviews.get();
 }
 
+- (NSArray<UITargetedDragPreview *> *)cancellationPreviews
+{
+    return _cancellationPreviews.get();
+}
+
 - (NSArray<UITargetedDragPreview *> *)dropPreviews
 {
     return _dropPreviews.get();
@@ -757,8 +773,24 @@ IGNORE_WARNINGS_END
     return _dropCompletionBlock.get();
 }
 
+- (void)setSessionWillBeginBlock:(dispatch_block_t)block
+{
+    _sessionWillBeginBlock = block;
+}
+
+- (dispatch_block_t)sessionWillBeginBlock
+{
+    return _sessionWillBeginBlock.get();
+}
+
 #pragma mark - WKUIDelegatePrivate
 
+- (void)_webView:(WKWebView *)webView dataInteraction:(UIDragInteraction *)interaction sessionWillBegin:(id <UIDragSession>)session
+{
+    if (_sessionWillBeginBlock)
+        _sessionWillBeginBlock();
+}
+
 - (void)_webView:(WKWebView *)webView dataInteractionOperationWasHandled:(BOOL)handled forSession:(id)session itemProviders:(NSArray<NSItemProvider *> *)itemProviders
 {
     if (self.dropCompletionBlock)