Crash notifying observers of responsiveness state change
authorbeidson@apple.com <beidson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 2 Jul 2018 20:52:18 +0000 (20:52 +0000)
committerbeidson@apple.com <beidson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 2 Jul 2018 20:52:18 +0000 (20:52 +0000)
<rdar://problem/41267796> and https://bugs.webkit.org/show_bug.cgi?id=187262

Reviewed by Tim Horton.

Source/WebKit:

* UIProcess/PageLoadState.cpp:
(WebKit::PageLoadState::callObserverCallback): Copy the container ahead of time.

Tools:

* TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
* TestWebKitAPI/Tests/WebKit/ResponsivenessTimerCrash.mm: Added.
(-[RTObserver observeValueForKeyPath:ofObject:change:context:]):
(TestWebKitAPI::TEST):
* TestWebKitAPI/cocoa/TestWKWebView.h:
* TestWebKitAPI/cocoa/TestWKWebView.mm:
(-[TestWKWebView typeCharacter:]):
(-[TestWKWebView hostWindow]):

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

Source/WebKit/ChangeLog
Source/WebKit/UIProcess/PageLoadState.cpp
Tools/ChangeLog
Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj
Tools/TestWebKitAPI/Tests/WebKit/ResponsivenessTimerCrash.mm [new file with mode: 0644]
Tools/TestWebKitAPI/cocoa/TestWKWebView.h
Tools/TestWebKitAPI/cocoa/TestWKWebView.mm

index e18fd8d..b0a09b2 100644 (file)
@@ -1,3 +1,13 @@
+2018-07-02  Brady Eidson  <beidson@apple.com>
+
+        Crash notifying observers of responsiveness state change
+        <rdar://problem/41267796> and https://bugs.webkit.org/show_bug.cgi?id=187262
+
+        Reviewed by Tim Horton.
+
+        * UIProcess/PageLoadState.cpp:
+        (WebKit::PageLoadState::callObserverCallback): Copy the container ahead of time.
+
 2018-07-02  Sihui Liu  <sihui_liu@apple.com>
 
         Remove InitWebCoreThreadSystemInterface() in WKProcessPool _initWithConfiguration
index 3ada0fb..1759f8a 100644 (file)
@@ -419,8 +419,17 @@ void PageLoadState::didChangeProcessIsResponsive()
 
 void PageLoadState::callObserverCallback(void (Observer::*callback)())
 {
-    for (auto* observer : m_observers)
+    auto protectedPage = makeRef(m_webPageProxy);
+
+    auto observerCopy = m_observers;
+    for (auto* observer : observerCopy) {
+        // This appears potentially inefficient on the surface (searching in a Vector)
+        // but in practice - using only API - there will only ever be (1) observer.
+        if (!m_observers.contains(observer))
+            continue;
+
         (observer->*callback)();
+    }
 }
 
 } // namespace WebKit
index 99206e2..5020256 100644 (file)
@@ -1,3 +1,19 @@
+2018-07-02  Brady Eidson  <beidson@apple.com>
+
+        Crash notifying observers of responsiveness state change
+        <rdar://problem/41267796> and https://bugs.webkit.org/show_bug.cgi?id=187262
+
+        Reviewed by Tim Horton.
+
+        * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
+        * TestWebKitAPI/Tests/WebKit/ResponsivenessTimerCrash.mm: Added.
+        (-[RTObserver observeValueForKeyPath:ofObject:change:context:]):
+        (TestWebKitAPI::TEST):
+        * TestWebKitAPI/cocoa/TestWKWebView.h:
+        * TestWebKitAPI/cocoa/TestWKWebView.mm:
+        (-[TestWKWebView typeCharacter:]):
+        (-[TestWKWebView hostWindow]):
+
 2018-07-02  Michael Catanzaro  <mcatanzaro@igalia.com>
 
         [WPE][GTK] flatpakutils.py should respect set-webkit-configuration build type
index 482778d..87ae934 100644 (file)
                5110FCFA1E01CDB8006F8D0B /* IDBIndexUpgradeToV2.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5110FCEF1E01CBAA006F8D0B /* IDBIndexUpgradeToV2.mm */; };
                5120C83D1E6751290025B250 /* WebsiteDataStoreCustomPaths.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5120C83C1E6750790025B250 /* WebsiteDataStoreCustomPaths.mm */; };
                5120C83E1E67678F0025B250 /* WebsiteDataStoreCustomPaths.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 5120C83B1E674E350025B250 /* WebsiteDataStoreCustomPaths.html */; };
+               512C4C9E20EAA40D004945EA /* ResponsivenessTimerCrash.mm in Sources */ = {isa = PBXBuildFile; fileRef = 512C4C9D20EAA405004945EA /* ResponsivenessTimerCrash.mm */; };
                51393E221523952D005F39C5 /* DOMWindowExtensionBasic_Bundle.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 51393E1D1523944A005F39C5 /* DOMWindowExtensionBasic_Bundle.cpp */; };
                5142B2731517C8C800C32B19 /* ContextMenuCanCopyURL.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 5142B2721517C89100C32B19 /* ContextMenuCanCopyURL.html */; };
                51460E1220D421F2005345F2 /* SimpleServiceWorkerRegistrations-2.sqlite3 in Copy Resources */ = {isa = PBXBuildFile; fileRef = 51460E0F20D4216E005345F2 /* SimpleServiceWorkerRegistrations-2.sqlite3 */; };
                5110FCF31E01CD77006F8D0B /* IndexUpgrade.sqlite3 */ = {isa = PBXFileReference; lastKnownFileType = file; path = IndexUpgrade.sqlite3; sourceTree = "<group>"; };
                5120C83B1E674E350025B250 /* WebsiteDataStoreCustomPaths.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = WebsiteDataStoreCustomPaths.html; sourceTree = "<group>"; };
                5120C83C1E6750790025B250 /* WebsiteDataStoreCustomPaths.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = WebsiteDataStoreCustomPaths.mm; sourceTree = "<group>"; };
+               512C4C9D20EAA405004945EA /* ResponsivenessTimerCrash.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = ResponsivenessTimerCrash.mm; sourceTree = "<group>"; };
                51393E1D1523944A005F39C5 /* DOMWindowExtensionBasic_Bundle.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = DOMWindowExtensionBasic_Bundle.cpp; sourceTree = "<group>"; };
                51393E1E1523944A005F39C5 /* DOMWindowExtensionBasic.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = DOMWindowExtensionBasic.cpp; sourceTree = "<group>"; };
                5142B2701517C88B00C32B19 /* ContextMenuCanCopyURL.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = ContextMenuCanCopyURL.mm; sourceTree = "<group>"; };
                                2DD7D3A9178205D00026E1E3 /* ResizeReversePaginatedWebView.cpp */,
                                8A2C750D16CED9550024F352 /* ResizeWindowAfterCrash.cpp */,
                                83DE134C1EF1C4FE00C1B355 /* ResponsivenessTimer.cpp */,
+                               512C4C9D20EAA405004945EA /* ResponsivenessTimerCrash.mm */,
                                C0BD669C131D3CF700E18F2A /* ResponsivenessTimerDoesntFireEarly.cpp */,
                                C0BD669E131D3CFF00E18F2A /* ResponsivenessTimerDoesntFireEarly_Bundle.cpp */,
                                83B6DE6E1EE7520F001E792F /* RestoreSessionState.cpp */,
                                7CCE7F061A411AE600447C4C /* LayoutMilestonesWithAllContentInFrame.cpp in Sources */,
                                7CCE7EDF1A411A9200447C4C /* LayoutUnit.cpp in Sources */,
                                7A66BDB61EAF14EF00CCC924 /* LimitTitleSize.cpp in Sources */,
+                               512C4C9E20EAA40D004945EA /* ResponsivenessTimerCrash.mm in Sources */,
                                7A7B0E7F1EAFE4C3006AB8AE /* LimitTitleSize.mm in Sources */,
                                C25CCA061E51380B0026CB8A /* LineBreaking.mm in Sources */,
                                37D36ED71AF42ECD00BAF5D9 /* LoadAlternateHTMLString.mm in Sources */,
diff --git a/Tools/TestWebKitAPI/Tests/WebKit/ResponsivenessTimerCrash.mm b/Tools/TestWebKitAPI/Tests/WebKit/ResponsivenessTimerCrash.mm
new file mode 100644 (file)
index 0000000..ace7ddf
--- /dev/null
@@ -0,0 +1,88 @@
+/*
+ * 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"
+
+#if WK_API_ENABLED
+
+#import "PlatformUtilities.h"
+#import "TestWKWebView.h"
+#import <WebKit/WKPagePrivateMac.h>
+#import <WebKit/WKWebViewPrivate.h>
+#import <wtf/RetainPtr.h>
+#import <wtf/Vector.h>
+
+static bool didBecomeUnresponsive;
+static RetainPtr<TestWKWebView> webView;
+static Vector<RetainPtr<id>> observableStates;
+static bool webViewSeen;
+
+@interface ResponsivenessTimerObserver : NSObject
+@end
+
+@implementation ResponsivenessTimerObserver
+
+-(void)observeValueForKeyPath:(NSString *)keyPath ofObject:(id)object change:(NSDictionary *)change context:(void *)context
+{
+    if (object == webView.get())
+        webViewSeen = true;
+
+    if (!observableStates.isEmpty())
+        observableStates.removeLast();
+    
+    if (webViewSeen && observableStates.isEmpty())
+        didBecomeUnresponsive = true;
+}
+
+@end
+
+namespace TestWebKitAPI {
+
+TEST(WebKit, ResponsivenessTimerCrash)
+{
+    RetainPtr<ResponsivenessTimerObserver> observer = adoptNS([[ResponsivenessTimerObserver alloc] init]);
+    @autoreleasepool {
+        webView = adoptNS([[TestWKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600)]);
+        
+        [webView addObserver:observer.get() forKeyPath:@"_webProcessIsResponsive" options:0 context:nullptr];
+
+        auto pageRef = [webView _pageRefForTransitionToWKWebView];
+        
+        for (size_t i = 0; i < 10; ++i) {
+            RetainPtr<id> observableState = adoptNS(WKPageCreateObservableState(pageRef));
+            [observableState.get() addObserver:observer.get() forKeyPath:@"_webProcessIsResponsive" options:0 context:nullptr];
+            observableStates.append(WTFMove(observableState));
+        }
+
+        [webView synchronouslyLoadHTMLString:@"<script>document.addEventListener('keydown', function(){while(1){}});</script>"];
+        [webView typeCharacter:'a'];
+    }
+    
+    Util::run(&didBecomeUnresponsive);
+}
+
+} // namespace TestWebKitAPI
+
+#endif // WK_API_ENABLED
index 45d9e8a..d773037 100644 (file)
@@ -57,6 +57,7 @@
 - (void)waitForMessage:(NSString *)message;
 - (void)performAfterLoading:(dispatch_block_t)actions;
 - (void)waitForNextPresentationUpdate;
+- (void)typeCharacter:(char)character;
 @end
 
 #if PLATFORM(IOS)
@@ -74,7 +75,6 @@
 - (void)mouseUpAtPoint:(NSPoint)point;
 - (void)mouseMoveToPoint:(NSPoint)point withFlags:(NSEventModifierFlags)flags;
 - (void)sendClicksAtPoint:(NSPoint)point numberOfClicks:(NSUInteger)numberOfClicks;
-- (void)typeCharacter:(char)character;
 - (NSWindow *)hostWindow;
 @end
 #endif
index 75f37d0..818a02d 100644 (file)
@@ -311,6 +311,14 @@ NSEventMask __simulated_forceClickAssociatedEventsMask(id self, SEL _cmd)
     TestWebKitAPI::Util::run(&done);
 }
 
+- (void)typeCharacter:(char)character {
+    NSString *characterAsString = [NSString stringWithFormat:@"%c" , character];
+    NSEventType keyDownEventType = NSEventTypeKeyDown;
+    NSEventType keyUpEventType = NSEventTypeKeyUp;
+    [self keyDown:[NSEvent keyEventWithType:keyDownEventType location:NSZeroPoint modifierFlags:0 timestamp:GetCurrentEventTime() windowNumber:[_hostWindow windowNumber] context:nil characters:characterAsString charactersIgnoringModifiers:characterAsString isARepeat:NO keyCode:character]];
+    [self keyUp:[NSEvent keyEventWithType:keyUpEventType location:NSZeroPoint modifierFlags:0 timestamp:GetCurrentEventTime() windowNumber:[_hostWindow windowNumber] context:nil characters:characterAsString charactersIgnoringModifiers:characterAsString isARepeat:NO keyCode:character]];
+}
+
 @end
 
 #if PLATFORM(IOS)
@@ -383,14 +391,6 @@ NSEventMask __simulated_forceClickAssociatedEventsMask(id self, SEL _cmd)
 {
     return _hostWindow.get();
 }
-
-- (void)typeCharacter:(char)character {
-    NSString *characterAsString = [NSString stringWithFormat:@"%c" , character];
-    NSEventType keyDownEventType = NSEventTypeKeyDown;
-    NSEventType keyUpEventType = NSEventTypeKeyUp;
-    [self keyDown:[NSEvent keyEventWithType:keyDownEventType location:NSZeroPoint modifierFlags:0 timestamp:GetCurrentEventTime() windowNumber:[_hostWindow windowNumber] context:nil characters:characterAsString charactersIgnoringModifiers:characterAsString isARepeat:NO keyCode:character]];
-    [self keyUp:[NSEvent keyEventWithType:keyUpEventType location:NSZeroPoint modifierFlags:0 timestamp:GetCurrentEventTime() windowNumber:[_hostWindow windowNumber] context:nil characters:characterAsString charactersIgnoringModifiers:characterAsString isARepeat:NO keyCode:character]];
-}
 @end
 #endif