[iOS] Group UIWebFormAccessoryDelegate-related code and tighten it up a bit
authordbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 20 Mar 2019 18:59:03 +0000 (18:59 +0000)
committerdbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 20 Mar 2019 18:59:03 +0000 (18:59 +0000)
https://bugs.webkit.org/show_bug.cgi?id=196018

Reviewed by Wenson Hsieh.

* UIProcess/ios/WKContentViewInteraction.mm:
(-[WKContentView accessoryClear]): Use uniform initializer syntax. Code could send the empty
string, but I resisted since null string, as we do now, likely encodes more compactly and we
avoid a per-process alloc.
(-[WKContentView accessoryTab:]): Fix style nit; missing space between capture list and arguments
in lambda. Also use lamdba capture initializer syntax and remove a local.
(-[WKContentView _updateAccessory]): Remove a FIXME as it can't be satified with the current
design without more bookkeeping. The design for showing and hiding an AutoFill button added in
r166933 requires knowing the title for the button when showing it via -setAccessoryViewCustomButtonTitle.
We could re-implement such that -setAccessoryViewCustomButtonTitle: stores the title and calls
-_updateAccessory, but that has the disadvantage of increasing the memory footprint of WKContentView
for the stored title and that seems worse than centralizing the logic in _updateAccessory. So,
let's not fix this FIXME. Now that we are removing the FIXME, change to use an early return style.
(-[WKContentView _hideKeyboard]): Micro optimization; only call _updateAccessory if we have
a form accessory view. This method is called everytime we load a page (more precisely when we
commit the load for a page) in addition to everytime we blur (defocus) an element. No need to
update an accessory if we don't have one.

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

Source/WebKit/ChangeLog
Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm

index bda96f4..58b2d91 100644 (file)
@@ -1,3 +1,28 @@
+2019-03-20  Daniel Bates  <dabates@apple.com>
+
+        [iOS] Group UIWebFormAccessoryDelegate-related code and tighten it up a bit
+        https://bugs.webkit.org/show_bug.cgi?id=196018
+
+        Reviewed by Wenson Hsieh.
+
+        * UIProcess/ios/WKContentViewInteraction.mm:
+        (-[WKContentView accessoryClear]): Use uniform initializer syntax. Code could send the empty
+        string, but I resisted since null string, as we do now, likely encodes more compactly and we
+        avoid a per-process alloc.
+        (-[WKContentView accessoryTab:]): Fix style nit; missing space between capture list and arguments
+        in lambda. Also use lamdba capture initializer syntax and remove a local.
+        (-[WKContentView _updateAccessory]): Remove a FIXME as it can't be satified with the current
+        design without more bookkeeping. The design for showing and hiding an AutoFill button added in
+        r166933 requires knowing the title for the button when showing it via -setAccessoryViewCustomButtonTitle.
+        We could re-implement such that -setAccessoryViewCustomButtonTitle: stores the title and calls
+        -_updateAccessory, but that has the disadvantage of increasing the memory footprint of WKContentView
+        for the stored title and that seems worse than centralizing the logic in _updateAccessory. So,
+        let's not fix this FIXME. Now that we are removing the FIXME, change to use an early return style.
+        (-[WKContentView _hideKeyboard]): Micro optimization; only call _updateAccessory if we have
+        a form accessory view. This method is called everytime we load a page (more precisely when we
+        commit the load for a page) in addition to everytime we blur (defocus) an element. No need to
+        update an accessory if we don't have one.
 2019-03-20  Olivier Robin  <olivierrobin@chromium.org>
 
         Fix _getContentsAsAttributedStringWithCompletionHandler availability for iOS.
index ff08351..d58c467 100644 (file)
@@ -3693,13 +3693,6 @@ static void selectionChangedWithTouch(WKContentView *view, const WebCore::IntPoi
     [self _invokePendingAutocorrectionContextHandler:[WKAutocorrectionContext autocorrectionContextWithContext:context]];
 }
 
-// UIWebFormAccessoryDelegate
-- (void)accessoryDone
-{
-    SetForScope<BOOL> dismissingAccessoryScope { _dismissingAccessory, YES };
-    [self resignFirstResponder];
-}
-
 #if !USE(UIKIT_KEYBOARD_ADDITIONS)
 - (NSArray *)keyCommands
 {
@@ -3724,21 +3717,6 @@ static void selectionChangedWithTouch(WKContentView *view, const WebCore::IntPoi
     [self accessoryTab:NO];
 }
 
-- (void)accessoryTab:(BOOL)isNext
-{
-    [self _endEditing];
-    _inputPeripheral = nil;
-
-    _isChangingFocusUsingAccessoryTab = YES;
-    [self beginSelectionChange];
-    RetainPtr<WKContentView> view = self;
-    _page->focusNextFocusedElement(isNext, [view](WebKit::CallbackBase::Error) {
-        [view endSelectionChange];
-        [view reloadInputViews];
-        view->_isChangingFocusUsingAccessoryTab = NO;
-    });
-}
-
 - (void)_becomeFirstResponderWithSelectionMovingForward:(BOOL)selectingForward completionHandler:(void (^)(BOOL didBecomeFirstResponder))completionHandler
 {
     auto completionHandlerCopy = Block_copy(completionHandler);
@@ -3772,6 +3750,33 @@ static void selectionChangedWithTouch(WKContentView *view, const WebCore::IntPoi
     [self _resetIsDoubleTapPending];
 }
 
+// MARK: UIWebFormAccessoryDelegate protocol and accessory methods
+
+- (void)accessoryClear
+{
+    _page->setFocusedElementValue({ });
+}
+
+- (void)accessoryDone
+{
+    SetForScope<BOOL> dismissingAccessoryScope { _dismissingAccessory, YES };
+    [self resignFirstResponder];
+}
+
+- (void)accessoryTab:(BOOL)isNext
+{
+    [self _endEditing];
+    _inputPeripheral = nil;
+
+    _isChangingFocusUsingAccessoryTab = YES;
+    [self beginSelectionChange];
+    _page->focusNextFocusedElement(isNext, [protectedSelf = retainPtr(self)] (WebKit::CallbackBase::Error) {
+        [protectedSelf endSelectionChange];
+        [protectedSelf reloadInputViews];
+        protectedSelf->_isChangingFocusUsingAccessoryTab = NO;
+    });
+}
+
 - (void)accessoryAutoFill
 {
     id <_WKInputDelegate> inputDelegate = [_webView _inputDelegate];
@@ -3779,36 +3784,30 @@ static void selectionChangedWithTouch(WKContentView *view, const WebCore::IntPoi
         [inputDelegate _webView:_webView accessoryViewCustomButtonTappedInFormInputSession:_formInputSession.get()];
 }
 
-- (void)accessoryClear
-{
-    _page->setFocusedElementValue(String());
-}
-
 - (void)_updateAccessory
 {
     [_formAccessoryView setNextEnabled:_focusedElementInformation.hasNextNode];
     [_formAccessoryView setPreviousEnabled:_focusedElementInformation.hasPreviousNode];
 
-    if (currentUserInterfaceIdiomIsPad())
+    if (currentUserInterfaceIdiomIsPad()) {
         [_formAccessoryView setClearVisible:NO];
-    else {
-        switch (_focusedElementInformation.elementType) {
-        case WebKit::InputType::Date:
-        case WebKit::InputType::Month:
-        case WebKit::InputType::DateTimeLocal:
-        case WebKit::InputType::Time:
-            [_formAccessoryView setClearVisible:YES];
-            break;
-        default:
-            [_formAccessoryView setClearVisible:NO];
-            break;
-        }
+        return;
     }
 
-    // FIXME: hide or show the AutoFill button as needed.
+    switch (_focusedElementInformation.elementType) {
+    case WebKit::InputType::Date:
+    case WebKit::InputType::Month:
+    case WebKit::InputType::DateTimeLocal:
+    case WebKit::InputType::Time:
+        [_formAccessoryView setClearVisible:YES];
+        return;
+    default:
+        [_formAccessoryView setClearVisible:NO];
+        return;
+    }
 }
 
-// Keyboard interaction
+// MARK: Keyboard interaction
 // UITextInput protocol implementation
 
 - (BOOL)_allowAnimatedUpdateSelectionRectViews
@@ -4782,7 +4781,8 @@ static NSString *contentTypeFromFieldName(WebCore::AutofillFieldName fieldName)
 
     // FIXME: Does it make sense to call -reloadInputViews on watchOS?
     [self reloadInputViews];
-    [self _updateAccessory];
+    if (_formAccessoryView)
+        [self _updateAccessory];
 }
 
 - (const WebKit::FocusedElementInformation&)focusedElementInformation