Changeset 243221 in webkit

Timestamp:

Mar 20, 2019, 11:59:03 AM (7 years ago)

Author:

dbates@webkit.org

Message:

[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.

Location:

trunk/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• UIProcess/ios/WKContentViewInteraction.mm (modified) (6 diffs)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+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>
 

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

@@ -3694 +3694 @@
 }
 
-// UIWebFormAccessoryDelegate
-- (void)accessoryDone
-{
-    SetForScope<BOOL> dismissingAccessoryScope { _dismissingAccessory, YES };
-    [self resignFirstResponder];
-}
-
 #if !USE(UIKIT_KEYBOARD_ADDITIONS)
 - (NSArray *)keyCommands
@@ -3723 +3716 @@
 {
     [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;
-    });
 }
 
@@ -3773 +3751 @@
 }
 
+// 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
 {
@@ -3780 +3785 @@
 }
 
-- (void)accessoryClear
-{
-    _page->setFocusedElementValue(String());
-}
-
 - (void)_updateAccessory
 {
@@ -3790 +3790 @@
     [_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;
-        }
-    }
-
-    // FIXME: hide or show the AutoFill button as needed.
-}
-
-// Keyboard interaction
+        return;
+    }
+
+    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;
+    }
+}
+
+// MARK: Keyboard interaction
 // UITextInput protocol implementation
 
@@ -4783 +4782 @@
     // FIXME: Does it make sense to call -reloadInputViews on watchOS?
     [self reloadInputViews];
-    [self _updateAccessory];
+    if (_formAccessoryView)
+        [self _updateAccessory];
 }