Add timeout for ensurePositionInformationIsUpToDate
authormegan_gardner@apple.com <megan_gardner@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 26 Apr 2018 23:27:19 +0000 (23:27 +0000)
committermegan_gardner@apple.com <megan_gardner@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 26 Apr 2018 23:27:19 +0000 (23:27 +0000)
https://bugs.webkit.org/show_bug.cgi?id=184567

Reviewed by Wenson Hsieh.

We are having long hang times for WebKit, and this is one of the culprits.
If we do not get an answer for positionInformation in a reasonable amount of time, we should timeout,
so as to not hang the UI.

* UIProcess/ios/WKContentViewInteraction.mm:
(-[WKContentView ensurePositionInformationIsUpToDate:]):

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

Source/WebKit/ChangeLog
Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj
Tools/TestWebKitAPI/Tests/ios/SynchronousTimeoutTests.mm [new file with mode: 0644]

index b7034f3..2003f92 100644 (file)
@@ -1,3 +1,17 @@
+2018-04-26  Megan Gardner  <megan_gardner@apple.com>
+
+        Add timeout for ensurePositionInformationIsUpToDate
+        https://bugs.webkit.org/show_bug.cgi?id=184567
+
+        Reviewed by Wenson Hsieh.
+        
+        We are having long hang times for WebKit, and this is one of the culprits.
+        If we do not get an answer for positionInformation in a reasonable amount of time, we should timeout,
+        so as to not hang the UI.
+
+        * UIProcess/ios/WKContentViewInteraction.mm:
+        (-[WKContentView ensurePositionInformationIsUpToDate:]):
+
 2018-04-26  Andy Estes  <aestes@apple.com>
 
         Try again to fix the iOS build after r231063.
index ba1bf5b..1381f70 100644 (file)
@@ -1451,16 +1451,16 @@ static inline bool isSamePair(UIGestureRecognizer *a, UIGestureRecognizer *b, UI
     if (!connection)
         return NO;
 
-    if ([self _hasValidOutstandingPositionInformationRequest:request]) {
-        return connection->waitForAndDispatchImmediately<Messages::WebPageProxy::DidReceivePositionInformation>(_page->pageID(), Seconds::infinity(), IPC::WaitForOption::InterruptWaitingIfSyncMessageArrives);
-    }
-
-    _page->process().sendSync(Messages::WebPage::GetPositionInformation(request), Messages::WebPage::GetPositionInformation::Reply(_positionInformation), _page->pageID());
+    if ([self _hasValidOutstandingPositionInformationRequest:request])
+        return connection->waitForAndDispatchImmediately<Messages::WebPageProxy::DidReceivePositionInformation>(_page->pageID(), 1_s, IPC::WaitForOption::InterruptWaitingIfSyncMessageArrives);
 
-    _hasValidPositionInformation = YES;
-    [self _invokeAndRemovePendingHandlersValidForCurrentPositionInformation];
+    _hasValidPositionInformation = _page->process().sendSync(Messages::WebPage::GetPositionInformation(request), Messages::WebPage::GetPositionInformation::Reply(_positionInformation), _page->pageID(), 1_s);
+    
+    // FIXME: We need to clean up these handlers in the event that we are not able to collect data, or if the WebProcess crashes.
+    if (_hasValidPositionInformation)
+        [self _invokeAndRemovePendingHandlersValidForCurrentPositionInformation];
 
-    return YES;
+    return _hasValidPositionInformation;
 }
 
 - (void)requestAsynchronousPositionInformationUpdate:(WebKit::InteractionInformationRequest)request
index 6111112..c32c265 100644 (file)
                3FCC4FE81EC4E8CA0076E37C /* PictureInPictureDelegate.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 3FCC4FE61EC4E87E0076E37C /* PictureInPictureDelegate.html */; };
                4135FB842011FAA700332139 /* InjectInternals_Bundle.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 4135FB832011FAA300332139 /* InjectInternals_Bundle.cpp */; };
                4135FB852011FABF00332139 /* libWebCoreTestSupport.dylib in Frameworks */ = {isa = PBXBuildFile; fileRef = 4135FB862011FABF00332139 /* libWebCoreTestSupport.dylib */; };
+               4433A396208044140091ED57 /* SynchronousTimeoutTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 4433A395208044130091ED57 /* SynchronousTimeoutTests.mm */; };
                44817A2F1F0486BF00003810 /* WKRequestActivatedElementInfo.mm in Sources */ = {isa = PBXBuildFile; fileRef = 44817A2E1F0486BF00003810 /* WKRequestActivatedElementInfo.mm */; };
                448D7E471EA6C55500ECC756 /* EnvironmentUtilitiesTest.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 448D7E451EA6C55500ECC756 /* EnvironmentUtilitiesTest.cpp */; };
                46397B951DC2C850009A78AE /* DOMNode.mm in Sources */ = {isa = PBXBuildFile; fileRef = 46397B941DC2C850009A78AE /* DOMNode.mm */; };
                41973B5C1AF22875006C7B36 /* SharedBuffer.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = SharedBuffer.cpp; sourceTree = "<group>"; };
                440A1D3814A0103A008A66F2 /* URL.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = URL.cpp; sourceTree = "<group>"; };
                442BBF681C91CAD90017087F /* RefLogger.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = RefLogger.cpp; sourceTree = "<group>"; };
+               4433A395208044130091ED57 /* SynchronousTimeoutTests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = SynchronousTimeoutTests.mm; sourceTree = "<group>"; };
                44817A2E1F0486BF00003810 /* WKRequestActivatedElementInfo.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = WKRequestActivatedElementInfo.mm; sourceTree = "<group>"; };
                448D7E451EA6C55500ECC756 /* EnvironmentUtilitiesTest.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = EnvironmentUtilitiesTest.cpp; sourceTree = "<group>"; };
                44A622C114A0E2B60048515B /* WTFStringUtilities.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = WTFStringUtilities.h; sourceTree = "<group>"; };
                                574F55CE204D3763002948C6 /* LocalAuthenticator.mm */,
                                7560917719259C59009EF06E /* MemoryCacheAddImageToCacheIOS.mm */,
                                F4C8797E2059D8D3009CD00B /* ScrollViewInsetTests.mm */,
+                               4433A395208044130091ED57 /* SynchronousTimeoutTests.mm */,
                                F45033F4206BEC95009351CE /* TextAutosizingBoost.mm */,
                                F46849BD1EEF58E400B937FE /* UIPasteboardTests.mm */,
                                2E1B881E2040EE5300FFF6A9 /* ViewportSizingTests.mm */,
                                ECA680CE1E68CC0900731D20 /* StringUtilities.mm in Sources */,
                                CE4D5DE71F6743BA0072CFC6 /* StringWithDirection.cpp in Sources */,
                                7CCE7ED21A411A7E00447C4C /* SubresourceErrorCrash.mm in Sources */,
+                               4433A396208044140091ED57 /* SynchronousTimeoutTests.mm in Sources */,
                                7CCE7EA81A411A1900447C4C /* SyntheticBackingScaleFactorWindow.m in Sources */,
                                1C734B5320788C4800F430EA /* SystemColors.mm in Sources */,
                                7CCE7F161A411AE600447C4C /* TerminateTwice.cpp in Sources */,
diff --git a/Tools/TestWebKitAPI/Tests/ios/SynchronousTimeoutTests.mm b/Tools/TestWebKitAPI/Tests/ios/SynchronousTimeoutTests.mm
new file mode 100644 (file)
index 0000000..49c181b
--- /dev/null
@@ -0,0 +1,80 @@
+/*
+ * Copyright (C) 2018 Apple Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
+ * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
+ * THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#import "config.h"
+
+#import "PlatformUtilities.h"
+#import "Test.h"
+#import "TestWKWebView.h"
+#import <WebKit/WKWebViewPrivate.h>
+
+#if WK_API_ENABLED && PLATFORM(IOS)
+
+using namespace TestWebKitAPI;
+
+@interface NSObject ()
+
+- (BOOL)hasSelectablePositionAtPoint:(CGPoint)point;
+
+@end
+
+namespace TestWebKitAPI {
+    
+static UIGestureRecognizer *recursiveFindHighlightLongPressRecognizer(UIView *view)
+{
+    for (UIGestureRecognizer *recognizer in view.gestureRecognizers) {
+        if ([recognizer isKindOfClass:NSClassFromString(@"_UIWebHighlightLongPressGestureRecognizer")])
+            return recognizer;
+    }
+    
+    for (UIView *subview in view.subviews) {
+        UIGestureRecognizer *recognizer = recursiveFindHighlightLongPressRecognizer(subview);
+        if (recognizer)
+            return recognizer;
+    }
+    
+    return nil;
+}
+    
+TEST(SynchronousTimeoutTests, UnresponsivePageDoesNotCausePositionInformationToHangUI)
+{
+    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, 500)]);
+    [webView synchronouslyLoadTestPageNamed:@"simple"];
+    
+    UIGestureRecognizer *highlightLongPressRecognizer = recursiveFindHighlightLongPressRecognizer(webView.get());
+    UIView *interactionView = highlightLongPressRecognizer.view;
+    EXPECT_NOT_NULL(highlightLongPressRecognizer);
+    
+    [webView evaluateJavaScript:@"while(1);" completionHandler:nil];
+    
+    // The test passes if we can long press and still finish the test.
+    [interactionView hasSelectablePositionAtPoint:CGPointMake(100, 100)];
+}
+
+    
+} // namespace TestWebKitAPI
+
+#endif // WK_API_ENABLED && PLATFORM(IOS)
+