Attachment 372844 Details for Bug 199192 – Patch

[patch] Patch

bug-199192-20190625105412.patch (text/plain), 16.05 KB, created by Wenson Hsieh on 2019-06-25 10:54:12 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Wenson Hsieh

Created: 2019-06-25 10:54:12 PDT

Size: 16.05 KB

patch

obsolete

>Subversion Revision: 246765
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index fdc36a7d9bbe9bfd18807d8bfbfc208e5a0d319e..754403378a326ac69790e12af75e655544632014 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,26 @@
>+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 NOBODY (OOPS!).
>+
>+        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, but may also occur in
>+        the case where the web 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-24  Chris Dumez  <cdumez@apple.com>
> 
>         Pages using Google's anti-flicker optimization may take ~5 seconds to do initial paint
>diff --git a/Source/WebKit/UIProcess/ios/DragDropInteractionState.mm b/Source/WebKit/UIProcess/ios/DragDropInteractionState.mm
>index 8f2807b230eb3140788ccd9ccc9f623bddf516ba..af276a303092e06a8089473709e9bace1eee4d94 100644
>--- a/Source/WebKit/UIProcess/ios/DragDropInteractionState.mm
>+++ b/Source/WebKit/UIProcess/ios/DragDropInteractionState.mm
>@@ -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()];
>diff --git a/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm b/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
>index 53b0be997b51aa919962cfa44c5b52c903863bfa..4d42f0b01eaa1e15e64ad49ac41a7d8d5702c6e9 100644
>--- a/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
>+++ b/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
>@@ -6214,6 +6214,11 @@ - (BOOL)_shouldUseContextMenus
>     return NO;
> }
> 
>+- (UIView *)containerViewForTargetedPreviews
>+{
>+    return self.unscaledView ?: self;
>+}
>+
> #if ENABLE(DRAG_SUPPORT)
> 
> static BOOL shouldEnableDragInteractionForPolicy(_WKDragInteractionPolicy policy)
>@@ -6446,7 +6451,7 @@ - (void)_deliverDelayedDropPreviewIfPossible:(Optional<WebCore::TextIndicatorDat
>     [_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 @@ - (UITargetedDragPreview *)dragInteraction:(UIDragInteraction *)interaction prev
>         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 @@ - (UITargetedPreview *)_ensureTargetedPreview
>     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();
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index 44ef1c96e14370db7e4b1f1dc4d0beab224df8b9..a079e2e12b878ecf54c47c0a9e2d62ae83e44fdd 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -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 NOBODY (OOPS!).
>+
>+        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-24  Michael Catanzaro  <mcatanzaro@igalia.com>
> 
>         contributors.json not canonicalized since r243297
>diff --git a/Tools/TestWebKitAPI/Tests/ios/DragAndDropTestsIOS.mm b/Tools/TestWebKitAPI/Tests/ios/DragAndDropTestsIOS.mm
>index f110b24b84b5f8eed2e050a0ae66a4b296e7efc2..47090476d7ce53114e3db83b926fe32cfbf1e049 100644
>--- a/Tools/TestWebKitAPI/Tests/ios/DragAndDropTestsIOS.mm
>+++ b/Tools/TestWebKitAPI/Tests/ios/DragAndDropTestsIOS.mm
>@@ -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]);
>diff --git a/Tools/TestWebKitAPI/cocoa/DragAndDropSimulator.h b/Tools/TestWebKitAPI/cocoa/DragAndDropSimulator.h
>index 3ad66987eefd20bd6d57fb704ed9dc2b1a2b6a83..7f31c4ff716b7f87a2e6c399416d0a6d5b69cbd4 100644
>--- a/Tools/TestWebKitAPI/cocoa/DragAndDropSimulator.h
>+++ b/Tools/TestWebKitAPI/cocoa/DragAndDropSimulator.h
>@@ -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;
> 
>diff --git a/Tools/TestWebKitAPI/ios/DragAndDropSimulatorIOS.mm b/Tools/TestWebKitAPI/ios/DragAndDropSimulatorIOS.mm
>index 147c36a7f1f25b6fb8f96b7ee8029164ba86a50d..96d6ecbbaaad43898564ed42aa4a92bcb0efc9c2 100644
>--- a/Tools/TestWebKitAPI/ios/DragAndDropSimulatorIOS.mm
>+++ b/Tools/TestWebKitAPI/ios/DragAndDropSimulatorIOS.mm
>@@ -310,7 +310,8 @@ @implementation DragAndDropSimulator {
> 
>     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 @@ @implementation DragAndDropSimulator {
>     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 @@ - (void)_resetSimulatedState
>     _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 @@ - (void)_concludeDropAndPerformOperationIfNecessary
>     } 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 @@ - (void)_advanceProgress
>         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 @@ - (DragAndDropPhase)phase
>     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 @@ - (void(^)(BOOL, NSArray *))dropCompletionBlock
>     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)

Actions: View | Formatted Diff | Diff

Attachments on bug 199192: 372844 | 372845