Source/WebCore:
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 13 Jan 2017 00:59:34 +0000 (00:59 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 13 Jan 2017 00:59:34 +0000 (00:59 +0000)
[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):

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

13 files changed:
Source/WebCore/ChangeLog
Source/WebCore/platform/ValidationBubble.h
Source/WebCore/platform/ios/ValidationBubbleIOS.mm
Source/WebKit/mac/ChangeLog
Source/WebKit/mac/WebView/WebView.mm
Source/WebKit/mac/WebView/WebViewData.h
Source/WebKit2/ChangeLog
Source/WebKit2/UIProcess/PageClient.h
Source/WebKit2/UIProcess/WebPageProxy.h
Source/WebKit2/UIProcess/ios/PageClientImplIOS.h
Source/WebKit2/UIProcess/ios/PageClientImplIOS.mm
Source/WebKit2/UIProcess/mac/PageClientImpl.h
Source/WebKit2/UIProcess/mac/PageClientImpl.mm

index 175045a..e899ae6 100644 (file)
@@ -1,3 +1,34 @@
+2017-01-12  Chris Dumez  <cdumez@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  <akling@apple.com>
 
         [iOS] Purge GraphicsServices font cache on memory warning.
index 17ca167..b9aa75c 100644 (file)
@@ -53,9 +53,13 @@ using PlatformView = void;
 
 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 @@ public:
 #endif
 
 private:
+    WEBCORE_EXPORT ValidationBubble(PlatformView*, const String& message);
+
     PlatformView* m_view;
     String m_message;
 #if PLATFORM(MAC)
index fde4154..9cdfb28 100644 (file)
@@ -129,7 +129,10 @@ ValidationBubble::~ValidationBubble()
 
 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)
index dbb0a2e..df820ae 100644 (file)
@@ -1,3 +1,16 @@
+2017-01-12  Chris Dumez  <cdumez@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  <rniwa@webkit.org>
 
         Remove pointerLockElement from DOMDocumentPrivate.h
index 0c5fda0..d4e50b8 100644 (file)
@@ -9309,7 +9309,7 @@ bool LayerFlushController::flushLayers()
 {
     // 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);
index 8c52eff..631fae1 100644 (file)
@@ -207,7 +207,7 @@ private:
     RetainPtr<NSEvent> pressureEvent;
 #endif // PLATFORM(MAC)
 
-    std::unique_ptr<WebCore::ValidationBubble> formValidationBubble;
+    RefPtr<WebCore::ValidationBubble> formValidationBubble;
 
     BOOL shouldMaintainInactiveSelection;
 
index ab1118d..4514920 100644 (file)
@@ -1,3 +1,22 @@
+2017-01-12  Chris Dumez  <cdumez@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  <enrica@apple.com>
 
         Do not allow selection of editable content when not editing.
index 1c210b1..d32c791 100644 (file)
@@ -228,7 +228,7 @@ public:
 #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)
index ef56b1c..3275f9c 100644 (file)
@@ -1826,7 +1826,7 @@ private:
     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;
index 69b6135..87be830 100644 (file)
@@ -96,7 +96,7 @@ private:
 #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;
index ce36405..7ab3115 100644 (file)
@@ -744,9 +744,9 @@ WebCore::UserInterfaceLayoutDirection PageClientImpl::userInterfaceLayoutDirecti
     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
index 9d7ce04..0d2ddaa 100644 (file)
@@ -131,7 +131,7 @@ private:
     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;
index feea263..b0e4676 100644 (file)
@@ -440,9 +440,9 @@ RefPtr<WebColorPicker> PageClientImpl::createColorPicker(WebPageProxy* page, con
 }
 #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)