[webkit-reviews] review granted: [Bug 129275] Remove WKInteractionView, move code into WKContentView : [Attachment 225102] Patch

[bugzilla-daemon at webkit.org]

Benjamin Poulain <benjamin at webkit.org> has granted Simon Fraser (smfr)
<simon.fraser at apple.com>'s request for review:
Bug 129275: Remove WKInteractionView, move code into WKContentView
https://bugs.webkit.org/show_bug.cgi?id=129275

Attachment 225102: Patch
https://bugs.webkit.org/attachment.cgi?id=225102&action=review

------- Additional Comments from Benjamin Poulain <benjamin at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=225102&action=review


> Source/WebKit2/ChangeLog:16
> +	   have implementaiton-related data members. WKContentViewInternal.h is
removed.

Typo: implementation.

> Source/WebKit2/ChangeLog:23
> +	   WKAutoCorrectionData and InteractionInformationAtPosition are store
via unique_ptr

Typo: stored.

> Source/WebKit2/UIProcess/ios/WKContentView.h:99
> +    RefPtr<WebKit::WebPageProxy> _page;
> +
> +    RetainPtr<UIWebTouchEventsGestureRecognizer>
_touchEventGestureRecognizer;
> +    BOOL _canSendTouchEventsAsynchronously;
> +    unsigned _nativeWebTouchEventUniqueIdBeingSentSynchronously;
> +
> +    RetainPtr<UITapGestureRecognizer> _singleTapGestureRecognizer;
> +    RetainPtr<_UIWebHighlightLongPressGestureRecognizer>
_highlightLongPressGestureRecognizer;
> +    RetainPtr<UILongPressGestureRecognizer> _longPressGestureRecognizer;
> +    RetainPtr<UITapGestureRecognizer> _doubleTapGestureRecognizer;
> +    RetainPtr<UITapGestureRecognizer> _twoFingerDoubleTapGestureRecognizer;
> +    RetainPtr<UIPanGestureRecognizer> _twoFingerPanGestureRecognizer;
> +
> +    RetainPtr<UIWKTextInteractionAssistant> _textSelectionAssistant;
> +    RetainPtr<UIWKSelectionAssistant> _webSelectionAssistant;
> +
> +    UITextInputTraits *_traits;
> +    BOOL _isEditable;
> +    UIWebFormAccessory *_accessory;
> +    id <UITextInputDelegate> _inputDelegate;
> +    BOOL _showingTextStyleOptions;
> +
> +    __weak UIWebScrollView *_scrollView;
> +
> +    RetainPtr<_UIHighlightView> _highlightView;
> +    uint64_t _latestTapHighlightID;
> +    BOOL _isTapHighlightIDValid;
> +    std::unique_ptr<WebKit::WKAutoCorrectionData> _autocorrectionData;
> +    RetainPtr<NSString> _markedText;
> +    std::unique_ptr<WebKit::InteractionInformationAtPosition>
_positionInformation;
> +    BOOL _hasValidPositionInformation;
> +    RetainPtr<WKActionSheetAssistant> _actionSheetAssistant;

Those should be in the @implementation, not in the header.
(And you could put back WKAutoCorrectionData and
InteractionInformationAtPosition as member instead of pointers.

> Source/WebKit2/UIProcess/ios/WKContentView.mm:153
>  - (BOOL)isAssistingNode
>  {
> -    return [_interactionView isEditable];
> +    return [self isEditable];
>  }

We could get rid of isAssistingNode, expose isEditable, and use that from
WKWebView.

> Source/WebKit2/UIProcess/ios/WKContentView.mm:220
>  {
>      [self _updateFixedPositionRect];
> -    [_interactionView _didEndScrollingOrZooming];
> +    [self _didEndScrollingOrZooming];
>  }
>  
>  - (void)willStartZoomOrScroll
>  {
> -    [_interactionView _willStartScrollingOrZooming];
> +    [self _willStartScrollingOrZooming];
>  }
>  
>  - (void)willStartUserTriggeredScroll
>  {
> -    [_interactionView _willStartUserTriggeredScrollingOrZooming];
> +    [self _willStartUserTriggeredScrollingOrZooming];
>  }
>  
>  - (void)willStartUserTriggeredZoom
>  {
> -    [_interactionView _willStartUserTriggeredScrollingOrZooming];
> +    [self _willStartUserTriggeredScrollingOrZooming];
>      _page->willStartUserTriggeredZooming();
>  }
>  
>  - (void)didZoomToScale:(CGFloat)scale
>  {
>      _page->didFinishZooming(scale);
> -    [_interactionView _didEndScrollingOrZooming];
> +    [self _didEndScrollingOrZooming];

We should get rid of those internal calls. Don't they only exist because
InteractionView used different names?

> Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:2
> + * Copyright (C) 2012, 2013 Apple Inc. All rights reserved.

2012-2014

> Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:130
> +//@synthesize inputDelegate = _inputDelegate;

???