- Revision
- 290875
- Author
- [email protected]
- Date
- 2022-03-06 09:39:33 -0800 (Sun, 06 Mar 2022)
Log Message
[iOS] Layer tree can get indefinitely frozen if WKWebView is unparented underneath -[UIWindow dealloc]
https://bugs.webkit.org/show_bug.cgi?id=237505
rdar://85563958
Reviewed by Tim Horton.
Source/WebKit:
It's currently possible for the web page to get permanently stuck in frozen state, due to the
`BackgroundApplication` layer tree freeze reason; this occurs when the web view is unparented from the view
hierarchy underneath the scope of UIWindow's `-dealloc` method.
During `-[UIWindow dealloc]`, the backpointer underlying the implementation of `-[UIView window]` is set to `nil`
immediately before the subclassing method hook `-willMoveToWindow:` is invoked on the view hierarchy. This means
that when `-willMoveToWindow:` is invoked, `self.window` will return `nil`. This, in turn, puts
`WKApplicationStateTrackingView` in a bad state because we bail early before resetting `_applicationStateTracker`
in the early return below, since we (erroneously) believe that we've already been unparented from the view
hierarchy, so we don't need to do anything.
```
if (!self._contentView.window || newWindow)
return;
```
As a result, if the same web view is eventually moved back into another visible window, `-didMoveToWindow` bails
before setting up the `_applicationStateTracker` again, since it already exists from when the previous window
was still active. This means `-_applicationWillEnterForeground` is never called when the web view is
reintroduced to the view hierarchy, so `LayerTreeFreezeReason::BackgroundApplication` is never lifted.
To address this, we simply remove the debug assertion for `_applicationStateTracker`, and instead check whether
the application state tracker exists or not for the logic of the early return. Doing so also makes the early
return in `-willMoveToWindow:` consistent with the logic in one in `-didMoveToWindow`, which already consults
`_applicationStateTracker`:
```
- (void)didMoveToWindow
{
if (!self._contentView.window || _applicationStateTracker)
return;
```
Test: ApplicationStateTracking.WindowDeallocDoesNotPermanentlyFreezeLayerTree
* UIProcess/ios/WKApplicationStateTrackingView.mm:
(-[WKApplicationStateTrackingView willMoveToWindow:]): See above.
Tools:
Add an API test to exercise the bug. This API test is comprised of the following series of steps:
1. Create the web view and add it under window #1.
2. Post a "did enter background" notification.
3. Deallocate window #1 (thereby unparenting the web view in the process).
4. Post a "will enter foreground" notification.
5. Add the web view under window #2.
6. Load some HTML content and wait for a presentation update.
Before the fix, this test times out because the layer tree is permanently frozen after step (3), due to the
`BackgroundApplication` reason, so the presentation update in step (6) never finishes.
* TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
* TestWebKitAPI/Tests/ios/ApplicationStateTracking.mm: Added.
(TestWebKitAPI::TEST):
Modified Paths
Added Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (290874 => 290875)
--- trunk/Source/WebKit/ChangeLog 2022-03-06 17:28:26 UTC (rev 290874)
+++ trunk/Source/WebKit/ChangeLog 2022-03-06 17:39:33 UTC (rev 290875)
@@ -1,3 +1,49 @@
+2022-03-06 Wenson Hsieh <[email protected]>
+
+ [iOS] Layer tree can get indefinitely frozen if WKWebView is unparented underneath -[UIWindow dealloc]
+ https://bugs.webkit.org/show_bug.cgi?id=237505
+ rdar://85563958
+
+ Reviewed by Tim Horton.
+
+ It's currently possible for the web page to get permanently stuck in frozen state, due to the
+ `BackgroundApplication` layer tree freeze reason; this occurs when the web view is unparented from the view
+ hierarchy underneath the scope of UIWindow's `-dealloc` method.
+
+ During `-[UIWindow dealloc]`, the backpointer underlying the implementation of `-[UIView window]` is set to `nil`
+ immediately before the subclassing method hook `-willMoveToWindow:` is invoked on the view hierarchy. This means
+ that when `-willMoveToWindow:` is invoked, `self.window` will return `nil`. This, in turn, puts
+ `WKApplicationStateTrackingView` in a bad state because we bail early before resetting `_applicationStateTracker`
+ in the early return below, since we (erroneously) believe that we've already been unparented from the view
+ hierarchy, so we don't need to do anything.
+
+ ```
+ if (!self._contentView.window || newWindow)
+ return;
+ ```
+
+ As a result, if the same web view is eventually moved back into another visible window, `-didMoveToWindow` bails
+ before setting up the `_applicationStateTracker` again, since it already exists from when the previous window
+ was still active. This means `-_applicationWillEnterForeground` is never called when the web view is
+ reintroduced to the view hierarchy, so `LayerTreeFreezeReason::BackgroundApplication` is never lifted.
+
+ To address this, we simply remove the debug assertion for `_applicationStateTracker`, and instead check whether
+ the application state tracker exists or not for the logic of the early return. Doing so also makes the early
+ return in `-willMoveToWindow:` consistent with the logic in one in `-didMoveToWindow`, which already consults
+ `_applicationStateTracker`:
+
+ ```
+ - (void)didMoveToWindow
+ {
+ if (!self._contentView.window || _applicationStateTracker)
+ return;
+ ```
+
+ Test: ApplicationStateTracking.WindowDeallocDoesNotPermanentlyFreezeLayerTree
+
+ * UIProcess/ios/WKApplicationStateTrackingView.mm:
+ (-[WKApplicationStateTrackingView willMoveToWindow:]): See above.
+
2022-03-06 Eric Carlson <[email protected]>
Push HTMLMediaElement renderer's contentBox rect to GPU process
Modified: trunk/Source/WebKit/UIProcess/ios/WKApplicationStateTrackingView.mm (290874 => 290875)
--- trunk/Source/WebKit/UIProcess/ios/WKApplicationStateTrackingView.mm 2022-03-06 17:28:26 UTC (rev 290874)
+++ trunk/Source/WebKit/UIProcess/ios/WKApplicationStateTrackingView.mm 2022-03-06 17:39:33 UTC (rev 290875)
@@ -51,12 +51,11 @@
- (void)willMoveToWindow:(UIWindow *)newWindow
{
- if (!self._contentView.window || newWindow)
+ if (!_applicationStateTracker || newWindow)
return;
auto page = [_webViewToTrack _page];
RELEASE_LOG(ViewState, "%p - WKApplicationStateTrackingView: View with page [%p, pageProxyID=%" PRIu64 "] was removed from a window, _lastObservedStateWasBackground=%d", self, page.get(), page ? page->identifier().toUInt64() : 0, page ? page->lastObservedStateWasBackground() : false);
- ASSERT(_applicationStateTracker);
_applicationStateTracker = nullptr;
}
Modified: trunk/Tools/ChangeLog (290874 => 290875)
--- trunk/Tools/ChangeLog 2022-03-06 17:28:26 UTC (rev 290874)
+++ trunk/Tools/ChangeLog 2022-03-06 17:39:33 UTC (rev 290875)
@@ -1,3 +1,26 @@
+2022-03-06 Wenson Hsieh <[email protected]>
+
+ [iOS] Layer tree can get indefinitely frozen if WKWebView is unparented underneath -[UIWindow dealloc]
+ https://bugs.webkit.org/show_bug.cgi?id=237505
+ rdar://85563958
+
+ Reviewed by Tim Horton.
+
+ Add an API test to exercise the bug. This API test is comprised of the following series of steps:
+ 1. Create the web view and add it under window #1.
+ 2. Post a "did enter background" notification.
+ 3. Deallocate window #1 (thereby unparenting the web view in the process).
+ 4. Post a "will enter foreground" notification.
+ 5. Add the web view under window #2.
+ 6. Load some HTML content and wait for a presentation update.
+
+ Before the fix, this test times out because the layer tree is permanently frozen after step (3), due to the
+ `BackgroundApplication` reason, so the presentation update in step (6) never finishes.
+
+ * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
+ * TestWebKitAPI/Tests/ios/ApplicationStateTracking.mm: Added.
+ (TestWebKitAPI::TEST):
+
2022-03-04 Sihui Liu <[email protected]>
Add null check for path in makeAllDirectories
Modified: trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj (290874 => 290875)
--- trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj 2022-03-06 17:28:26 UTC (rev 290874)
+++ trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj 2022-03-06 17:39:33 UTC (rev 290875)
@@ -1086,6 +1086,7 @@
F46A095A1ED8A6E600D4AA55 /* apple.gif in Copy Resources */ = {isa = PBXBuildFile; fileRef = F47D30EB1ED28619000482E1 /* apple.gif */; };
F46A095B1ED8A6E600D4AA55 /* gif-and-file-input.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = F47D30ED1ED28A6C000482E1 /* gif-and-file-input.html */; };
F46BD56924870643008282D6 /* dragstart-data.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = F46BD5682487062D008282D6 /* dragstart-data.html */; };
+ F46C45C427D42A2F00ECED2C /* ApplicationStateTracking.mm in Sources */ = {isa = PBXBuildFile; fileRef = F46C45C327D42A2F00ECED2C /* ApplicationStateTracking.mm */; };
F46D43AB26D7092800969E5E /* test.jpg in Copy Resources */ = {isa = PBXBuildFile; fileRef = F46D43AA26D7090300969E5E /* test.jpg */; };
F472874727816BCE003EBE7F /* NSResponderTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = F43C3823278133190099ABCE /* NSResponderTests.mm */; };
F47728991E4AE3C1007ABF6A /* full-page-contenteditable.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = F47728981E4AE3AD007ABF6A /* full-page-contenteditable.html */; };
@@ -3103,6 +3104,7 @@
F46849BF1EEF5EDC00B937FE /* rich-and-plain-text.html */ = {isa = PBXFileReference; lastKnownFileType = text.html; path = "rich-and-plain-text.html"; sourceTree = "<group>"; };
F469FB231F01803500401539 /* contenteditable-and-target.html */ = {isa = PBXFileReference; lastKnownFileType = text.html; path = "contenteditable-and-target.html"; sourceTree = "<group>"; };
F46BD5682487062D008282D6 /* dragstart-data.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = "dragstart-data.html"; sourceTree = "<group>"; };
+ F46C45C327D42A2F00ECED2C /* ApplicationStateTracking.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = ApplicationStateTracking.mm; sourceTree = "<group>"; };
F46D43AA26D7090300969E5E /* test.jpg */ = {isa = PBXFileReference; lastKnownFileType = image.jpeg; path = test.jpg; sourceTree = "<group>"; };
F47728981E4AE3AD007ABF6A /* full-page-contenteditable.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = "full-page-contenteditable.html"; sourceTree = "<group>"; };
F47D30EB1ED28619000482E1 /* apple.gif */ = {isa = PBXFileReference; lastKnownFileType = image.gif; path = apple.gif; sourceTree = "<group>"; };
@@ -3957,6 +3959,7 @@
A1C4FB6F1BACCEFA003742D0 /* Resources */,
2E205BA31F527746005952DD /* AccessibilityTestsIOS.mm */,
F45B63FC1F19D410009D38B9 /* ActionSheetTests.mm */,
+ F46C45C327D42A2F00ECED2C /* ApplicationStateTracking.mm */,
F42D634322A1729F00D2FB3A /* AutocorrectionTestsIOS.mm */,
44077BB0231449D200179E2D /* DataDetectorsTestIOS.mm */,
F4D4F3B71E4E36E400BB2767 /* DragAndDropTestsIOS.mm */,
@@ -5522,6 +5525,7 @@
712E4BC125DDC52A0007201C /* AnimationFrameRate.cpp in Sources */,
57152B5E21CC2045000C37CA /* ApduTest.cpp in Sources */,
6354F4D11F7C3AB500D89DF3 /* ApplicationManifestParser.cpp in Sources */,
+ F46C45C427D42A2F00ECED2C /* ApplicationStateTracking.mm in Sources */,
DF6580942722168900B3F1C1 /* ASN1Utilities.cpp in Sources */,
7CCE7EB41A411A7E00447C4C /* AttributedString.mm in Sources */,
F4FA2A4F24D1F05700618A46 /* AttributedSubstringForProposedRange.mm in Sources */,
Added: trunk/Tools/TestWebKitAPI/Tests/ios/ApplicationStateTracking.mm (0 => 290875)
--- trunk/Tools/TestWebKitAPI/Tests/ios/ApplicationStateTracking.mm (rev 0)
+++ trunk/Tools/TestWebKitAPI/Tests/ios/ApplicationStateTracking.mm 2022-03-06 17:39:33 UTC (rev 290875)
@@ -0,0 +1,64 @@
+/*
+ * Copyright (C) 2022 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 PLATFORM(IOS_FAMILY)
+
+#import "TestWKWebView.h"
+
+namespace TestWebKitAPI {
+
+TEST(ApplicationStateTracking, WindowDeallocDoesNotPermanentlyFreezeLayerTree)
+{
+ auto configuration = adoptNS([WKWebViewConfiguration new]);
+
+ RetainPtr<TestWKWebView> webView;
+ RetainPtr<UIWindow> window;
+
+ auto postWithScene = [&](NSNotificationName name) {
+ [NSNotificationCenter.defaultCenter postNotificationName:name object:[window windowScene] userInfo:nil];
+ };
+
+ @autoreleasepool {
+ webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, 568) configuration:configuration.get() addToWindow:NO]);
+ window = adoptNS([[UIWindow alloc] initWithFrame:CGRectMake(0, 0, 320, 568)]);
+ [window addSubview:webView.get()];
+ [webView synchronouslyLoadHTMLString:@"<body>Foo</body>"];
+ [webView waitForNextPresentationUpdate];
+ postWithScene(UISceneDidEnterBackgroundNotification);
+ window = nil;
+ }
+
+ window = adoptNS([[UIWindow alloc] initWithFrame:CGRectMake(0, 0, 320, 568)]);
+ postWithScene(UISceneWillEnterForegroundNotification);
+ [window addSubview:webView.get()];
+ [webView synchronouslyLoadHTMLString:@"<body>Bar</body>"];
+ [webView waitForNextPresentationUpdate];
+}
+
+} // namespace TestWebKitAPI
+
+#endif // PLATFORM(IOS_FAMILY)
