Title: [210689] trunk/Source
Revision
210689
Author
cdu...@apple.com
Date
2017-01-12 16:59:34 -0800 (Thu, 12 Jan 2017)

Log Message

Source/WebCore:
[iOS] HTML form validation popover sometimes does not go away
https://bugs.webkit.org/show_bug.cgi?id=166990
<rdar://problem/29985957>

Reviewed by Tim Horton.

The issue was that [UIViewController presentViewController:] is asynchronous
and that we sometimes tried to call [m_popoverController dismissViewControllerAnimated:]
before presentViewController had completed. This is something that UIKit does
not handle nicely and the popover just stays visible even though we have
asked for the controller to be dismissed and destroyed the ValidationBubble
object.

To address the issue, I made ValidationBubble RefCounted and make sure the
ValidationBubble object stays alive at least until the completion handler for
[UIViewController presentViewController:] has been called. This is done via
protecting the object using a RefPtr<> and capturing it in the lambda.
Because dismissViewControllerAnimated is called in the destructor, it is no
longer possible to call dismissViewControllerAnimated before the call to
presentViewController has completed.

No new tests, no easily testable since the popover was staying visible
after being destroyed (held on by UIKit).

* platform/ValidationBubble.h:
(WebCore::ValidationBubble::create):
* platform/ios/ValidationBubbleIOS.mm:
(WebCore::ValidationBubble::show):

Source/WebKit/mac:
HTML form validation popover sometimes does not go away
https://bugs.webkit.org/show_bug.cgi?id=166990
<rdar://problem/29985957>

Reviewed by Tim Horton.

Update code using ValidationBubble now that it is RefCounted.

* WebView/WebView.mm:
(-[WebView showFormValidationMessage:withAnchorRect:]):

Source/WebKit2:
[iOS] HTML form validation popover sometimes does not go away
https://bugs.webkit.org/show_bug.cgi?id=166990
<rdar://problem/29985957>

Reviewed by Tim Horton.

Update code using ValidationBubble now that it is RefCounted.

* UIProcess/PageClient.h:
* UIProcess/WebPageProxy.h:
* UIProcess/ios/PageClientImplIOS.h:
* UIProcess/ios/PageClientImplIOS.mm:
(WebKit::PageClientImpl::createValidationBubble):
* UIProcess/mac/PageClientImpl.h:
* UIProcess/mac/PageClientImpl.mm:
(WebKit::PageClientImpl::createValidationBubble):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (210688 => 210689)


--- trunk/Source/WebCore/ChangeLog	2017-01-13 00:47:37 UTC (rev 210688)
+++ trunk/Source/WebCore/ChangeLog	2017-01-13 00:59:34 UTC (rev 210689)
@@ -1,3 +1,34 @@
+2017-01-12  Chris Dumez  <cdu...@apple.com>
+
+        [iOS] HTML form validation popover sometimes does not go away
+        https://bugs.webkit.org/show_bug.cgi?id=166990
+        <rdar://problem/29985957>
+
+        Reviewed by Tim Horton.
+
+        The issue was that [UIViewController presentViewController:] is asynchronous
+        and that we sometimes tried to call [m_popoverController dismissViewControllerAnimated:]
+        before presentViewController had completed. This is something that UIKit does
+        not handle nicely and the popover just stays visible even though we have
+        asked for the controller to be dismissed and destroyed the ValidationBubble
+        object.
+
+        To address the issue, I made ValidationBubble RefCounted and make sure the
+        ValidationBubble object stays alive at least until the completion handler for
+        [UIViewController presentViewController:] has been called. This is done via
+        protecting the object using a RefPtr<> and capturing it in the lambda.
+        Because dismissViewControllerAnimated is called in the destructor, it is no
+        longer possible to call dismissViewControllerAnimated before the call to
+        presentViewController has completed.
+
+        No new tests, no easily testable since the popover was staying visible
+        after being destroyed (held on by UIKit).
+
+        * platform/ValidationBubble.h:
+        (WebCore::ValidationBubble::create):
+        * platform/ios/ValidationBubbleIOS.mm:
+        (WebCore::ValidationBubble::show):
+
 2017-01-12  Andreas Kling  <akl...@apple.com>
 
         [iOS] Purge GraphicsServices font cache on memory warning.

Modified: trunk/Source/WebCore/platform/ValidationBubble.h (210688 => 210689)


--- trunk/Source/WebCore/platform/ValidationBubble.h	2017-01-13 00:47:37 UTC (rev 210688)
+++ trunk/Source/WebCore/platform/ValidationBubble.h	2017-01-13 00:59:34 UTC (rev 210689)
@@ -53,9 +53,13 @@
 
 namespace WebCore {
 
-class ValidationBubble {
+class ValidationBubble : public RefCounted<ValidationBubble> {
 public:
-    WEBCORE_EXPORT ValidationBubble(PlatformView*, const String& message);
+    static Ref<ValidationBubble> create(PlatformView* view, const String& message)
+    {
+        return adoptRef(*new ValidationBubble(view, message));
+    }
+
     WEBCORE_EXPORT ~ValidationBubble();
 
     const String& message() const { return m_message; }
@@ -68,6 +72,8 @@
 #endif
 
 private:
+    WEBCORE_EXPORT ValidationBubble(PlatformView*, const String& message);
+
     PlatformView* m_view;
     String m_message;
 #if PLATFORM(MAC)

Modified: trunk/Source/WebCore/platform/ios/ValidationBubbleIOS.mm (210688 => 210689)


--- trunk/Source/WebCore/platform/ios/ValidationBubbleIOS.mm	2017-01-13 00:47:37 UTC (rev 210688)
+++ trunk/Source/WebCore/platform/ios/ValidationBubbleIOS.mm	2017-01-13 00:59:34 UTC (rev 210689)
@@ -129,7 +129,10 @@
 
 void ValidationBubble::show()
 {
-    [m_presentingViewController presentViewController:m_popoverController.get() animated:NO completion:nil];
+    // Protect the validation bubble so it stays alive until it is effectively presented. UIKit does not deal nicely with
+    // dismissing a popover that is being presented.
+    RefPtr<ValidationBubble> protectedThis(this);
+    [m_presentingViewController presentViewController:m_popoverController.get() animated:NO completion:[protectedThis]() { }];
 }
 
 static UIViewController *fallbackViewController(UIView *view)

Modified: trunk/Source/WebKit/mac/ChangeLog (210688 => 210689)


--- trunk/Source/WebKit/mac/ChangeLog	2017-01-13 00:47:37 UTC (rev 210688)
+++ trunk/Source/WebKit/mac/ChangeLog	2017-01-13 00:59:34 UTC (rev 210689)
@@ -1,3 +1,16 @@
+2017-01-12  Chris Dumez  <cdu...@apple.com>
+
+        HTML form validation popover sometimes does not go away
+        https://bugs.webkit.org/show_bug.cgi?id=166990
+        <rdar://problem/29985957>
+
+        Reviewed by Tim Horton.
+
+        Update code using ValidationBubble now that it is RefCounted.
+
+        * WebView/WebView.mm:
+        (-[WebView showFormValidationMessage:withAnchorRect:]):
+
 2017-01-10  Ryosuke Niwa  <rn...@webkit.org>
 
         Remove pointerLockElement from DOMDocumentPrivate.h

Modified: trunk/Source/WebKit/mac/WebView/WebView.mm (210688 => 210689)


--- trunk/Source/WebKit/mac/WebView/WebView.mm	2017-01-13 00:47:37 UTC (rev 210688)
+++ trunk/Source/WebKit/mac/WebView/WebView.mm	2017-01-13 00:59:34 UTC (rev 210689)
@@ -9309,7 +9309,7 @@
 {
     // FIXME: We should enable this on iOS as well.
 #if PLATFORM(MAC)
-    _private->formValidationBubble = std::make_unique<ValidationBubble>(self, message);
+    _private->formValidationBubble = ValidationBubble::create(self, message);
     _private->formValidationBubble->showRelativeTo(enclosingIntRect([self _convertRectFromRootView:anchorRect]));
 #else
     UNUSED_PARAM(message);

Modified: trunk/Source/WebKit/mac/WebView/WebViewData.h (210688 => 210689)


--- trunk/Source/WebKit/mac/WebView/WebViewData.h	2017-01-13 00:47:37 UTC (rev 210688)
+++ trunk/Source/WebKit/mac/WebView/WebViewData.h	2017-01-13 00:59:34 UTC (rev 210689)
@@ -207,7 +207,7 @@
     RetainPtr<NSEvent> pressureEvent;
 #endif // PLATFORM(MAC)
 
-    std::unique_ptr<WebCore::ValidationBubble> formValidationBubble;
+    RefPtr<WebCore::ValidationBubble> formValidationBubble;
 
     BOOL shouldMaintainInactiveSelection;
 

Modified: trunk/Source/WebKit2/ChangeLog (210688 => 210689)


--- trunk/Source/WebKit2/ChangeLog	2017-01-13 00:47:37 UTC (rev 210688)
+++ trunk/Source/WebKit2/ChangeLog	2017-01-13 00:59:34 UTC (rev 210689)
@@ -1,3 +1,22 @@
+2017-01-12  Chris Dumez  <cdu...@apple.com>
+
+        [iOS] HTML form validation popover sometimes does not go away
+        https://bugs.webkit.org/show_bug.cgi?id=166990
+        <rdar://problem/29985957>
+
+        Reviewed by Tim Horton.
+
+        Update code using ValidationBubble now that it is RefCounted.
+
+        * UIProcess/PageClient.h:
+        * UIProcess/WebPageProxy.h:
+        * UIProcess/ios/PageClientImplIOS.h:
+        * UIProcess/ios/PageClientImplIOS.mm:
+        (WebKit::PageClientImpl::createValidationBubble):
+        * UIProcess/mac/PageClientImpl.h:
+        * UIProcess/mac/PageClientImpl.mm:
+        (WebKit::PageClientImpl::createValidationBubble):
+
 2017-01-12  Enrica Casucci  <enr...@apple.com>
 
         Do not allow selection of editable content when not editing.

Modified: trunk/Source/WebKit2/UIProcess/PageClient.h (210688 => 210689)


--- trunk/Source/WebKit2/UIProcess/PageClient.h	2017-01-13 00:47:37 UTC (rev 210688)
+++ trunk/Source/WebKit2/UIProcess/PageClient.h	2017-01-13 00:59:34 UTC (rev 210689)
@@ -228,7 +228,7 @@
 #endif
 
 #if PLATFORM(COCOA)
-    virtual std::unique_ptr<WebCore::ValidationBubble> createValidationBubble(const String& message) = 0;
+    virtual Ref<WebCore::ValidationBubble> createValidationBubble(const String& message) = 0;
 #endif
 
 #if PLATFORM(COCOA)

Modified: trunk/Source/WebKit2/UIProcess/WebPageProxy.h (210688 => 210689)


--- trunk/Source/WebKit2/UIProcess/WebPageProxy.h	2017-01-13 00:47:37 UTC (rev 210688)
+++ trunk/Source/WebKit2/UIProcess/WebPageProxy.h	2017-01-13 00:59:34 UTC (rev 210689)
@@ -1826,7 +1826,7 @@
     RefPtr<WebColorPicker> m_colorPicker;
 #endif
 #if PLATFORM(COCOA)
-    std::unique_ptr<WebCore::ValidationBubble> m_validationBubble;
+    RefPtr<WebCore::ValidationBubble> m_validationBubble;
 #endif
 
     const uint64_t m_pageID;

Modified: trunk/Source/WebKit2/UIProcess/ios/PageClientImplIOS.h (210688 => 210689)


--- trunk/Source/WebKit2/UIProcess/ios/PageClientImplIOS.h	2017-01-13 00:47:37 UTC (rev 210688)
+++ trunk/Source/WebKit2/UIProcess/ios/PageClientImplIOS.h	2017-01-13 00:59:34 UTC (rev 210689)
@@ -96,7 +96,7 @@
 #if ENABLE(CONTEXT_MENUS)
     std::unique_ptr<WebContextMenuProxy> createContextMenuProxy(WebPageProxy&, const ContextMenuContextData&, const UserData&) override;
 #endif
-    std::unique_ptr<WebCore::ValidationBubble> createValidationBubble(const String& message) final;
+    Ref<WebCore::ValidationBubble> createValidationBubble(const String& message) final;
 
     void setTextIndicator(Ref<WebCore::TextIndicator>, WebCore::TextIndicatorWindowLifetime) override;
     void clearTextIndicator(WebCore::TextIndicatorWindowDismissalAnimation) override;

Modified: trunk/Source/WebKit2/UIProcess/ios/PageClientImplIOS.mm (210688 => 210689)


--- trunk/Source/WebKit2/UIProcess/ios/PageClientImplIOS.mm	2017-01-13 00:47:37 UTC (rev 210688)
+++ trunk/Source/WebKit2/UIProcess/ios/PageClientImplIOS.mm	2017-01-13 00:59:34 UTC (rev 210689)
@@ -744,9 +744,9 @@
     return ([UIView userInterfaceLayoutDirectionForSemanticContentAttribute:[m_webView semanticContentAttribute]] == UIUserInterfaceLayoutDirectionLeftToRight) ? WebCore::UserInterfaceLayoutDirection::LTR : WebCore::UserInterfaceLayoutDirection::RTL;
 }
 
-std::unique_ptr<ValidationBubble> PageClientImpl::createValidationBubble(const String& message)
+Ref<ValidationBubble> PageClientImpl::createValidationBubble(const String& message)
 {
-    return std::make_unique<ValidationBubble>(m_contentView, message);
+    return ValidationBubble::create(m_contentView, message);
 }
 
 } // namespace WebKit

Modified: trunk/Source/WebKit2/UIProcess/mac/PageClientImpl.h (210688 => 210689)


--- trunk/Source/WebKit2/UIProcess/mac/PageClientImpl.h	2017-01-13 00:47:37 UTC (rev 210688)
+++ trunk/Source/WebKit2/UIProcess/mac/PageClientImpl.h	2017-01-13 00:59:34 UTC (rev 210689)
@@ -131,7 +131,7 @@
     RefPtr<WebColorPicker> createColorPicker(WebPageProxy*, const WebCore::Color& initialColor, const WebCore::IntRect&) override;
 #endif
 
-    std::unique_ptr<WebCore::ValidationBubble> createValidationBubble(const String& message) final;
+    Ref<WebCore::ValidationBubble> createValidationBubble(const String& message) final;
 
     void setTextIndicator(Ref<WebCore::TextIndicator>, WebCore::TextIndicatorWindowLifetime) override;
     void clearTextIndicator(WebCore::TextIndicatorWindowDismissalAnimation) override;

Modified: trunk/Source/WebKit2/UIProcess/mac/PageClientImpl.mm (210688 => 210689)


--- trunk/Source/WebKit2/UIProcess/mac/PageClientImpl.mm	2017-01-13 00:47:37 UTC (rev 210688)
+++ trunk/Source/WebKit2/UIProcess/mac/PageClientImpl.mm	2017-01-13 00:59:34 UTC (rev 210689)
@@ -440,9 +440,9 @@
 }
 #endif
 
-std::unique_ptr<ValidationBubble> PageClientImpl::createValidationBubble(const String& message)
+Ref<ValidationBubble> PageClientImpl::createValidationBubble(const String& message)
 {
-    return std::make_unique<ValidationBubble>(m_view, message);
+    return ValidationBubble::create(m_view, message);
 }
 
 void PageClientImpl::setTextIndicator(Ref<TextIndicator> textIndicator, WebCore::TextIndicatorWindowLifetime lifetime)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to