UI process crash when focusing an editable image
authortimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 19 Dec 2018 23:37:02 +0000 (23:37 +0000)
committertimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 19 Dec 2018 23:37:02 +0000 (23:37 +0000)
https://bugs.webkit.org/show_bug.cgi?id=192839
<rdar://problem/46786670>

Reviewed by Wenson Hsieh.

Source/WebKit:

* SourcesCocoa.txt:
* UIProcess/ios/WKContentViewInteraction.h:
* UIProcess/ios/WKContentViewInteraction.mm:
(-[WKContentView _requiresKeyboardWhenFirstResponder]):
(-[WKContentView inputView]):
(-[WKContentView requiresAccessoryView]):
(-[WKContentView _startAssistingNode:userIsInteracting:blurPreviousNode:changingActivityState:userObject:]):
(-[WKContentView _stopAssistingNode]):
(-[WKContentView _installInkPickerForDrawingViewWithID:]):
(-[WKContentView _uninstallInkPicker]):
* UIProcess/ios/WKInkPickerView.h: Renamed from Source/WebKit/UIProcess/ios/WKInkPickerControl.h.
* UIProcess/ios/WKInkPickerView.mm: Renamed from Source/WebKit/UIProcess/ios/WKInkPickerControl.mm.
(-[WKInkPickerView initWithDrawingView:]):
(-[WKInkPickerView didPickInk]):
(-[WKInkPickerView inlineInkPickerDidToggleRuler:]):
(-[WKInkPickerView inlineInkPicker:didSelectTool:]):
(-[WKInkPickerView inlineInkPicker:didSelectColor:]):
(-[WKInkPickerView inkPickerSize]):
(-[WKInkPickerView layoutSubviews]):
(-[WKInkPickerView sizeThatFits:]):
(-[WKInkPickerView viewControllerForPopoverPresentationFromInlineInkPicker:]):
* WebKit.xcodeproj/project.pbxproj:
Make WKInkPickerView a WKWebView subview instead of an inputView.
Also, don't force the keyboard to be visible when an editable image is focused.

LayoutTests:

* editing/images/basic-editable-image-with-gesture.html: Added.
* editing/images/basic-editable-image-with-gesture-expected.txt: Added.
* resources/ui-helper.js:
(window.UIHelper.stylusTapAt.return.new.Promise):
(window.UIHelper.stylusTapAt):
Add a test that ensures that adding an editable image from a gesture
doesn't crash, and can be drawn on.

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

LayoutTests/ChangeLog
LayoutTests/editing/images/basic-editable-image-with-gesture-expected.txt [new file with mode: 0644]
LayoutTests/editing/images/basic-editable-image-with-gesture.html [new file with mode: 0644]
LayoutTests/resources/ui-helper.js
Source/WebKit/ChangeLog
Source/WebKit/SourcesCocoa.txt
Source/WebKit/UIProcess/ios/WKContentViewInteraction.h
Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
Source/WebKit/UIProcess/ios/WKInkPickerView.h [moved from Source/WebKit/UIProcess/ios/WKInkPickerControl.h with 85% similarity]
Source/WebKit/UIProcess/ios/WKInkPickerView.mm [moved from Source/WebKit/UIProcess/ios/WKInkPickerControl.mm with 82% similarity]
Source/WebKit/WebKit.xcodeproj/project.pbxproj

index f99a2a8..83e0dcf 100644 (file)
@@ -1,3 +1,19 @@
+2018-12-19  Tim Horton  <timothy_horton@apple.com>
+
+        UI process crash when focusing an editable image
+        https://bugs.webkit.org/show_bug.cgi?id=192839
+        <rdar://problem/46786670>
+
+        Reviewed by Wenson Hsieh.
+
+        * editing/images/basic-editable-image-with-gesture.html: Added.
+        * editing/images/basic-editable-image-with-gesture-expected.txt: Added.
+        * resources/ui-helper.js:
+        (window.UIHelper.stylusTapAt.return.new.Promise):
+        (window.UIHelper.stylusTapAt):
+        Add a test that ensures that adding an editable image from a gesture
+        doesn't crash, and can be drawn on.
+
 2018-12-19  Youenn Fablet  <youenn@apple.com>
 
         [ MacOS iOS ] Layout Test webrtc/no-port-zero-in-upd-candidates.html is flaky timeout
diff --git a/LayoutTests/editing/images/basic-editable-image-with-gesture-expected.txt b/LayoutTests/editing/images/basic-editable-image-with-gesture-expected.txt
new file mode 100644 (file)
index 0000000..9101159
--- /dev/null
@@ -0,0 +1,3 @@
+
+Had 0 strokes in editable image before drawing.
+Had 1 stroke in editable image after drawing.
diff --git a/LayoutTests/editing/images/basic-editable-image-with-gesture.html b/LayoutTests/editing/images/basic-editable-image-with-gesture.html
new file mode 100644 (file)
index 0000000..70c16ec
--- /dev/null
@@ -0,0 +1,22 @@
+<!DOCTYPE html><!-- webkit-test-runner [ enableEditableImages=true ] -->
+<head>
+<script src="../../resources/ui-helper.js"></script>
+<script>
+if (window.testRunner) {
+    testRunner.dumpAsText();
+    testRunner.waitUntilDone();
+}
+
+addEventListener("load", async () => {
+    await UIHelper.stylusTapAt(0, 0);
+    const initialNumberOfStrokesInEditableImage = (await UIHelper.numberOfStrokesInEditableImage());
+    await UIHelper.drawSquareInEditableImage();
+    const numberOfStrokesInEditableImageAfterDrawing = (await UIHelper.numberOfStrokesInEditableImage());
+    document.getElementById("log").innerHTML = `Had ${initialNumberOfStrokesInEditableImage} strokes in editable image before drawing.<br/>Had ${numberOfStrokesInEditableImageAfterDrawing} stroke in editable image after drawing.`;
+    testRunner.notifyDone();
+});
+</script>
+</head>
+<body contenteditable>
+<div id="log"></div>
+</body>
index 67c52c0..1ee35c5 100644 (file)
@@ -443,6 +443,19 @@ window.UIHelper = class UIHelper {
         return new Promise(resolve => testRunner.runUIScript(`uiController.drawSquareInEditableImage()`, resolve));
     }
 
+    static stylusTapAt(x, y)
+    {
+        if (!this.isWebKit2())
+            return Promise.resolve();
+
+        return new Promise((resolve) => {
+            testRunner.runUIScript(`
+                uiController.stylusTapAtPoint(${x}, ${y}, 2, 1, 0.5, function() {
+                    uiController.uiScriptComplete('Done');
+                });`, resolve);
+        });
+    }
+
     static numberOfStrokesInEditableImage()
     {
         if (!this.isWebKit2())
index 0904517..d7e45ef 100644 (file)
@@ -1,3 +1,36 @@
+2018-12-19  Tim Horton  <timothy_horton@apple.com>
+
+        UI process crash when focusing an editable image
+        https://bugs.webkit.org/show_bug.cgi?id=192839
+        <rdar://problem/46786670>
+
+        Reviewed by Wenson Hsieh.
+
+        * SourcesCocoa.txt:
+        * UIProcess/ios/WKContentViewInteraction.h:
+        * UIProcess/ios/WKContentViewInteraction.mm:
+        (-[WKContentView _requiresKeyboardWhenFirstResponder]):
+        (-[WKContentView inputView]):
+        (-[WKContentView requiresAccessoryView]):
+        (-[WKContentView _startAssistingNode:userIsInteracting:blurPreviousNode:changingActivityState:userObject:]):
+        (-[WKContentView _stopAssistingNode]):
+        (-[WKContentView _installInkPickerForDrawingViewWithID:]):
+        (-[WKContentView _uninstallInkPicker]):
+        * UIProcess/ios/WKInkPickerView.h: Renamed from Source/WebKit/UIProcess/ios/WKInkPickerControl.h.
+        * UIProcess/ios/WKInkPickerView.mm: Renamed from Source/WebKit/UIProcess/ios/WKInkPickerControl.mm.
+        (-[WKInkPickerView initWithDrawingView:]):
+        (-[WKInkPickerView didPickInk]):
+        (-[WKInkPickerView inlineInkPickerDidToggleRuler:]):
+        (-[WKInkPickerView inlineInkPicker:didSelectTool:]):
+        (-[WKInkPickerView inlineInkPicker:didSelectColor:]):
+        (-[WKInkPickerView inkPickerSize]):
+        (-[WKInkPickerView layoutSubviews]):
+        (-[WKInkPickerView sizeThatFits:]):
+        (-[WKInkPickerView viewControllerForPopoverPresentationFromInlineInkPicker:]):
+        * WebKit.xcodeproj/project.pbxproj:
+        Make WKInkPickerView a WKWebView subview instead of an inputView.
+        Also, don't force the keyboard to be visible when an editable image is focused.
+
 2018-12-19  Eric Carlson  <eric.carlson@apple.com>
 
         [MediaStream] Force system camera/microphone TCC prompt if necessary
index f1a057a..ed65b8f 100644 (file)
@@ -390,7 +390,7 @@ UIProcess/ios/WKContentViewInteraction.mm @no-unify
 UIProcess/ios/WKDrawingView.mm
 UIProcess/ios/WKGeolocationProviderIOS.mm
 UIProcess/ios/WKGeolocationProviderIOSObjCSecurityOrigin.mm
-UIProcess/ios/WKInkPickerControl.mm
+UIProcess/ios/WKInkPickerView.mm
 UIProcess/ios/WKInspectorNodeSearchGestureRecognizer.mm
 UIProcess/ios/WKKeyboardScrollingAnimator.mm
 UIProcess/ios/WKLegacyPDFView.mm
index e794e3c..5dd18f0 100644 (file)
@@ -96,6 +96,7 @@ class WebPageProxy;
 @class WKFocusedFormControlView;
 @class WKFormInputControl;
 @class WKFormInputSession;
+@class WKInkPickerView;
 @class WKInspectorNodeSearchGestureRecognizer;
 
 typedef void (^UIWKAutocorrectionCompletionHandler)(UIWKAutocorrectionRects *rectsForInput);
@@ -284,6 +285,10 @@ struct WKAutoCorrectionData {
     RetainPtr<NSArray<UITextSuggestion *>> _dataListTextSuggestions;
 #endif
 
+#if HAVE(PENCILKIT)
+    RetainPtr<WKInkPickerView> _inkPicker;
+#endif
+
     BOOL _isEditable;
     BOOL _showingTextStyleOptions;
     BOOL _hasValidPositionInformation;
index dee8988..543a5e2 100644 (file)
@@ -47,7 +47,7 @@
 #import "WKFormInputControl.h"
 #import "WKFormSelectControl.h"
 #import "WKImagePreviewViewController.h"
-#import "WKInkPickerControl.h"
+#import "WKInkPickerView.h"
 #import "WKInspectorNodeSearchGestureRecognizer.h"
 #import "WKNSURLExtras.h"
 #import "WKPreviewActionItemIdentifiers.h"
@@ -1365,6 +1365,7 @@ static NSValue *nsSizeForTapHighlightBorderRadius(WebCore::IntSize borderRadius,
     // FIXME: We should add the logic to handle keyboard visibility during focus redirects.
     switch (_assistedNodeInformation.elementType) {
     case WebKit::InputType::None:
+    case WebKit::InputType::Drawing:
         return NO;
     case WebKit::InputType::Select:
 #if ENABLE(INPUT_TYPE_COLOR)
@@ -1375,8 +1376,6 @@ static NSValue *nsSizeForTapHighlightBorderRadius(WebCore::IntSize borderRadius,
     case WebKit::InputType::DateTimeLocal:
     case WebKit::InputType::Time:
         return !currentUserInterfaceIdiomIsPad();
-    case WebKit::InputType::Drawing:
-        return YES;
     default:
         return !_assistedNodeInformation.isReadOnly;
     }
@@ -1426,14 +1425,6 @@ static NSValue *nsSizeForTapHighlightBorderRadius(WebCore::IntSize borderRadius,
             _inputPeripheral = adoptNS([[WKFormColorControl alloc] initWithView:self]);
             break;
 #endif
-        case WebKit::InputType::Drawing:
-#if HAVE(PENCILKIT)
-            _inputPeripheral = adoptNS([[WKInkPickerControl alloc] initWithDrawingView:_page->editableImageController().editableImage(_assistedNodeInformation.embeddedViewID)->drawingView.get()]);
-            break;
-#else
-            ASSERT_NOT_REACHED();
-            return [[UIView new] autorelease];
-#endif
         default:
             _inputPeripheral = adoptNS([[WKFormInputControl alloc] initWithView:self]);
             break;
@@ -2194,6 +2185,7 @@ static void cancelPotentialTapIfNecessary(WKContentView* contentView)
 
     switch (_assistedNodeInformation.elementType) {
     case WebKit::InputType::None:
+    case WebKit::InputType::Drawing:
         return NO;
     case WebKit::InputType::Text:
     case WebKit::InputType::Password:
@@ -2216,8 +2208,6 @@ static void cancelPotentialTapIfNecessary(WKContentView* contentView)
     case WebKit::InputType::Color:
 #endif
         return !currentUserInterfaceIdiomIsPad();
-    case WebKit::InputType::Drawing:
-        return YES;
     }
 }
 
@@ -4447,6 +4437,11 @@ static bool isAssistableInputType(WebKit::InputType type)
     if (blurPreviousNode)
         [self _stopAssistingNode];
 
+#if HAVE(PENCILKIT)
+    if (information.elementType == WebKit::InputType::Drawing)
+        [self _installInkPickerForDrawingViewWithID:information.embeddedViewID];
+#endif
+
     if (!shouldShowKeyboard)
         return;
 
@@ -4497,6 +4492,7 @@ static bool isAssistableInputType(WebKit::InputType type)
     case WebKit::InputType::Time:
     case WebKit::InputType::Month:
     case WebKit::InputType::Date:
+    case WebKit::InputType::Drawing:
 #if ENABLE(INPUT_TYPE_COLOR)
     case WebKit::InputType::Color:
 #endif
@@ -4530,6 +4526,11 @@ static bool isAssistableInputType(WebKit::InputType type)
 {
     SetForScope<BOOL> isBlurringFocusedNodeForScope { _isBlurringFocusedNode, YES };
 
+#if HAVE(PENCILKIT)
+    if (_inkPicker)
+        [self _uninstallInkPicker];
+#endif
+
     [_formInputSession invalidate];
     _formInputSession = nil;
 
@@ -6210,6 +6211,30 @@ static NSArray<NSItemProvider *> *extractItemProvidersFromDropSession(id <UIDrop
 }
 #endif
 
+#if HAVE(PENCILKIT)
+- (void)_installInkPickerForDrawingViewWithID:(WebCore::GraphicsLayer::EmbeddedViewID)embeddedViewID
+{
+    _inkPicker = adoptNS([[WKInkPickerView alloc] initWithDrawingView:_page->editableImageController().editableImage(embeddedViewID)->drawingView.get()]);
+    [_inkPicker sizeToFit];
+    [_inkPicker setTranslatesAutoresizingMaskIntoConstraints:NO];
+    [_webView addSubview:_inkPicker.get()];
+
+    [NSLayoutConstraint activateConstraints:@[
+        [[_inkPicker heightAnchor] constraintEqualToConstant:[_inkPicker frame].size.height],
+        [[_inkPicker bottomAnchor] constraintEqualToAnchor:_webView.safeAreaLayoutGuide.bottomAnchor],
+        [[_inkPicker leftAnchor] constraintEqualToAnchor:_webView.safeAreaLayoutGuide.leftAnchor],
+        [[_inkPicker rightAnchor] constraintEqualToAnchor:_webView.safeAreaLayoutGuide.rightAnchor],
+    ]];
+}
+
+- (void)_uninstallInkPicker
+{
+    [_inkPicker removeFromSuperview];
+    _inkPicker = nil;
+}
+
+#endif // HAVE(PENCILKIT)
+
 @end
 
 @implementation WKContentView (WKTesting)
 
 #if HAVE(PENCILKIT)
 
-#import "WKFormPeripheral.h"
+#import <UIKit/UIKit.h>
 
 OBJC_CLASS WKDrawingView;
 
-@interface WKInkPickerControl : NSObject <WKFormPeripheral>
+@interface WKInkPickerView : UIView
 
-- (instancetype)initWithDrawingView:(WKDrawingView *)drawingView;
+- (instancetype)init NS_UNAVAILABLE;
+- (instancetype)initWithFrame:(CGRect)frame NS_UNAVAILABLE;
+- (instancetype)initWithCoder:(NSCoder *)coder NS_UNAVAILABLE;
 
-@end
+- (instancetype)initWithDrawingView:(WKDrawingView *)drawingView NS_DESIGNATED_INITIALIZER;
 
+@end
 
 #endif // HAVE(PENCILKIT)
@@ -24,7 +24,7 @@
  */
 
 #import "config.h"
-#import "WKInkPickerControl.h"
+#import "WKInkPickerView.h"
 
 #if HAVE(PENCILKIT)
 
@@ -32,7 +32,7 @@
 
 #import "PencilKitSoftLink.h"
 
-@interface WKInkPickerView : UIView <PKInlineInkPickerDelegate>
+@interface WKInkPickerView () <PKInlineInkPickerDelegate>
 @end
 
 @implementation WKInkPickerView {
@@ -40,9 +40,9 @@
     RetainPtr<WKDrawingView> _drawingView;
 }
 
-- (instancetype)initWithFrame:(CGRect)frame drawingView:(WKDrawingView *)drawingView
+- (instancetype)initWithDrawingView:(WKDrawingView *)drawingView
 {
-    self = [super initWithFrame:frame];
+    self = [super initWithFrame:CGRectZero];
     if (!self)
         return nil;
 
 
 @end
 
-@implementation WKInkPickerControl {
-    RetainPtr<WKInkPickerView> _picker;
-    RetainPtr<WKDrawingView> _drawingView;
-}
-
-- (instancetype)initWithDrawingView:(WKDrawingView *)drawingView
-{
-    self = [super init];
-    if (!self)
-        return nil;
-
-    _drawingView = drawingView;
-
-    return self;
-}
-
-- (void)beginEditing
-{
-
-}
-
-- (void)endEditing
-{
-
-}
-
-- (UIView *)assistantView
-{
-    if (!_picker)
-        _picker = adoptNS([[WKInkPickerView alloc] initWithFrame:CGRectZero drawingView:_drawingView.get()]);
-    [_picker sizeToFit];
-    return _picker.get();
-}
-
-@end
-
 #endif // HAVE(PENCILKIT)
index ffb8f95..ef1b05d 100644 (file)
                2D6AB540192B1C4A003A9FD1 /* WKPDFPageNumberIndicator.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; name = WKPDFPageNumberIndicator.mm; path = ios/WKPDFPageNumberIndicator.mm; sourceTree = "<group>"; };
                2D6B371918A967AD0042AE80 /* _WKThumbnailView.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = _WKThumbnailView.h; sourceTree = "<group>"; };
                2D6B371A18A967AD0042AE80 /* _WKThumbnailView.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = _WKThumbnailView.mm; sourceTree = "<group>"; };
-               2D6BF11E21AE145F001E79C9 /* WKInkPickerControl.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; name = WKInkPickerControl.mm; path = ios/WKInkPickerControl.mm; sourceTree = "<group>"; };
-               2D6BF11F21AE145F001E79C9 /* WKInkPickerControl.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = WKInkPickerControl.h; path = ios/WKInkPickerControl.h; sourceTree = "<group>"; };
                2D6BF12121AF56E1001E79C9 /* PencilKitSoftLink.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; name = PencilKitSoftLink.mm; path = ios/PencilKitSoftLink.mm; sourceTree = "<group>"; };
                2D6BF12221AF56E1001E79C9 /* PencilKitSoftLink.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = PencilKitSoftLink.h; path = ios/PencilKitSoftLink.h; sourceTree = "<group>"; };
                2D6CD117189058A500E5A4A0 /* ViewSnapshotStore.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ViewSnapshotStore.h; sourceTree = "<group>"; };
                2DE6943B18BD2A68005C15E5 /* SmartMagnificationControllerMessageReceiver.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = SmartMagnificationControllerMessageReceiver.cpp; path = DerivedSources/WebKit2/SmartMagnificationControllerMessageReceiver.cpp; sourceTree = BUILT_PRODUCTS_DIR; };
                2DE6943C18BD2A68005C15E5 /* SmartMagnificationControllerMessages.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = SmartMagnificationControllerMessages.h; path = DerivedSources/WebKit2/SmartMagnificationControllerMessages.h; sourceTree = BUILT_PRODUCTS_DIR; };
                2DEAC5CE1AC368BB00A195D8 /* _WKFindOptions.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = _WKFindOptions.h; sourceTree = "<group>"; };
+               2DF3962A21C8DC50008835E3 /* WKInkPickerView.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; name = WKInkPickerView.mm; path = ios/WKInkPickerView.mm; sourceTree = "<group>"; };
+               2DF3962B21C8DC50008835E3 /* WKInkPickerView.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = WKInkPickerView.h; path = ios/WKInkPickerView.h; sourceTree = "<group>"; };
                2DF9593418A42412009785A1 /* ViewGestureControllerIOS.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; name = ViewGestureControllerIOS.mm; path = ios/ViewGestureControllerIOS.mm; sourceTree = "<group>"; };
                2DF9EEE31A781FB400B6CFBE /* APIFrameInfo.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = APIFrameInfo.cpp; sourceTree = "<group>"; };
                2DF9EEE41A781FB400B6CFBE /* APIFrameInfo.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = APIFrameInfo.h; sourceTree = "<group>"; };
                                0FCB4E4118BBE044000FCFC9 /* WKGeolocationProviderIOSObjCSecurityOrigin.mm */,
                                933DF82D1B3BC09000AEA9E3 /* WKImagePreviewViewController.h */,
                                933DF82F1B3BC0B400AEA9E3 /* WKImagePreviewViewController.mm */,
-                               2D6BF11F21AE145F001E79C9 /* WKInkPickerControl.h */,
-                               2D6BF11E21AE145F001E79C9 /* WKInkPickerControl.mm */,
+                               2DF3962B21C8DC50008835E3 /* WKInkPickerView.h */,
+                               2DF3962A21C8DC50008835E3 /* WKInkPickerView.mm */,
                                0F3C7259196F5F6800AEDD0C /* WKInspectorHighlightView.h */,
                                0F3C7257196F5F5000AEDD0C /* WKInspectorHighlightView.mm */,
                                A54293A2195A43C6002782C7 /* WKInspectorNodeSearchGestureRecognizer.h */,