Attachment 338924 Details for Bug 184567 – Patch

[patch] Patch

bug-184567-20180426152155.patch (text/plain), 9.78 KB, created by Megan Gardner on 2018-04-26 15:21:55 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Megan Gardner

Created: 2018-04-26 15:21:55 PDT

Size: 9.78 KB

patch

obsolete

>Index: Source/WebKit/ChangeLog
>===================================================================
>--- Source/WebKit/ChangeLog	(revision 231054)
>+++ Source/WebKit/ChangeLog	(working copy)
>@@ -1,3 +1,17 @@
>+2018-04-12  Megan Gardner  <megan_gardner@apple.com>
>+
>+        Add timeout for ensurePositionInformationIsUpToDate
>+        https://bugs.webkit.org/show_bug.cgi?id=184567
>+
>+        Reviewed by NOBODY (OOPS!).
>+        
>+        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-25  Megan Gardner  <megan_gardner@apple.com>
> 
>         Activate selection when interacting with editable content
>Index: Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
>===================================================================
>--- Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm	(revision 231054)
>+++ Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm	(working copy)
>@@ -1451,16 +1451,16 @@ - (BOOL)ensurePositionInformationIsUpToD
>     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: Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj
>===================================================================
>--- Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj	(revision 231053)
>+++ Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj	(working copy)
>@@ -154,6 +154,7 @@
> 		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 */; };
>@@ -1344,6 +1345,7 @@
> 		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>"; };
>@@ -2374,6 +2376,7 @@
> 				574F55CE204D3763002948C6 /* LocalAuthenticator.mm */,
> 				7560917719259C59009EF06E /* MemoryCacheAddImageToCacheIOS.mm */,
> 				F4C8797E2059D8D3009CD00B /* ScrollViewInsetTests.mm */,
>+				4433A395208044130091ED57 /* SynchronousTimeoutTests.mm */,
> 				F45033F4206BEC95009351CE /* TextAutosizingBoost.mm */,
> 				F46849BD1EEF58E400B937FE /* UIPasteboardTests.mm */,
> 				2E1B881E2040EE5300FFF6A9 /* ViewportSizingTests.mm */,
>@@ -3735,6 +3738,7 @@
> 				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 */,
>Index: Tools/TestWebKitAPI/Tests/ios/SynchronousTimeoutTests.mm
>===================================================================
>--- Tools/TestWebKitAPI/Tests/ios/SynchronousTimeoutTests.mm	(nonexistent)
>+++ Tools/TestWebKitAPI/Tests/ios/SynchronousTimeoutTests.mm	(working copy)
>@@ -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)
>+

Actions: View | Formatted Diff | Diff

Attachments on bug 184567: 337857 | 337860 | 337869 | 338922 | 338924