- Revision
- 294301
- Author
- alanc...@apple.com
- Date
- 2022-05-16 23:10:55 -0700 (Mon, 16 May 2022)
Log Message
Cherry-pick r293894. rdar://problem/74201964
[iOS] Infinite recursion at -[WKFullScreenWindowController _exitFullscreenImmediately]
https://bugs.webkit.org/show_bug.cgi?id=239744
<rdar://74201964>
Reviewed by Eric Carlson.
Source/WebCore:
Even after fullscreen is correctly torn down during an immediate exit, fullscreen can never again
be entered for the lifetime of the page. This is due to the ivar m_pendingExitFullscreen being set
to true with no opportunity for that state to be cleared. This occurs because the callbacks for
exiting fullscreen are performed before the lambda form FullscreenManager::exitFullscreen() is
called.
Set m_pendingExitFullscreen inside exitFullscreen(), rather than the lambda, and clear it explicitly
in the cases where fullscreen exiting is not actually necessary.
The mock behavior of Fullscreen inside DumpRenderTree and WebKitTestRunner are not able to exercise
this same path. Track improvements to this mock in bug #239747.
* dom/FullscreenManager.cpp:
(WebCore::FullscreenManager::requestFullscreenForElement):
(WebCore::FullscreenManager::exitFullscreen):
(WebCore::FullscreenManager::setAnimatingFullscreen):
(WebCore::FullscreenManager::setFullscreenControlsHidden):
Source/WebKit:
When exiting fullscreen immediately, we trigger the correct state transition by calling
-[self exitFullScreen]. However, when no fullscreen manager exists (as may happen during
navigation), -exitFullscreen may itself call -_exitFullscreenImmediately, which causes an
infinite recursion and stack overflow.
Unroll the implementation by performing all the same pieces as would have been done by
-exitFullScreen, but inline. Refactor out common behavior into -_reinsertWebViewUnderPlaceholder,
and call that both from -_exitFullscreenImmediately and _completedExitFullScreen.
Unfortunately, no API tests are possible here because TestWebKitAPI is not an actual iOS UI
application.
* UIProcess/ios/fullscreen/WKFullScreenWindowControllerIOS.mm:
(-[WKFullScreenWindowController exitFullScreen]):
(-[WKFullScreenWindowController _reinsertWebViewUnderPlaceholder]):
(-[WKFullScreenWindowController _completedExitFullScreen]):
(-[WKFullScreenWindowController _exitFullscreenImmediately]):
Canonical link: https://commits.webkit.org/250352@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@293894 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Modified Paths
Diff
Modified: branches/safari-613-branch/Source/WebCore/ChangeLog (294300 => 294301)
--- branches/safari-613-branch/Source/WebCore/ChangeLog 2022-05-17 06:10:51 UTC (rev 294300)
+++ branches/safari-613-branch/Source/WebCore/ChangeLog 2022-05-17 06:10:55 UTC (rev 294301)
@@ -1,5 +1,84 @@
2022-05-16 Alan Coon <alanc...@apple.com>
+ Cherry-pick r293894. rdar://problem/74201964
+
+ [iOS] Infinite recursion at -[WKFullScreenWindowController _exitFullscreenImmediately]
+ https://bugs.webkit.org/show_bug.cgi?id=239744
+ <rdar://74201964>
+
+ Reviewed by Eric Carlson.
+
+ Source/WebCore:
+
+ Even after fullscreen is correctly torn down during an immediate exit, fullscreen can never again
+ be entered for the lifetime of the page. This is due to the ivar m_pendingExitFullscreen being set
+ to true with no opportunity for that state to be cleared. This occurs because the callbacks for
+ exiting fullscreen are performed before the lambda form FullscreenManager::exitFullscreen() is
+ called.
+
+ Set m_pendingExitFullscreen inside exitFullscreen(), rather than the lambda, and clear it explicitly
+ in the cases where fullscreen exiting is not actually necessary.
+
+ The mock behavior of Fullscreen inside DumpRenderTree and WebKitTestRunner are not able to exercise
+ this same path. Track improvements to this mock in bug #239747.
+
+ * dom/FullscreenManager.cpp:
+ (WebCore::FullscreenManager::requestFullscreenForElement):
+ (WebCore::FullscreenManager::exitFullscreen):
+ (WebCore::FullscreenManager::setAnimatingFullscreen):
+ (WebCore::FullscreenManager::setFullscreenControlsHidden):
+
+ Source/WebKit:
+
+ When exiting fullscreen immediately, we trigger the correct state transition by calling
+ -[self exitFullScreen]. However, when no fullscreen manager exists (as may happen during
+ navigation), -exitFullscreen may itself call -_exitFullscreenImmediately, which causes an
+ infinite recursion and stack overflow.
+
+ Unroll the implementation by performing all the same pieces as would have been done by
+ -exitFullScreen, but inline. Refactor out common behavior into -_reinsertWebViewUnderPlaceholder,
+ and call that both from -_exitFullscreenImmediately and _completedExitFullScreen.
+
+ Unfortunately, no API tests are possible here because TestWebKitAPI is not an actual iOS UI
+ application.
+
+ * UIProcess/ios/fullscreen/WKFullScreenWindowControllerIOS.mm:
+ (-[WKFullScreenWindowController exitFullScreen]):
+ (-[WKFullScreenWindowController _reinsertWebViewUnderPlaceholder]):
+ (-[WKFullScreenWindowController _completedExitFullScreen]):
+ (-[WKFullScreenWindowController _exitFullscreenImmediately]):
+
+ Canonical link: https://commits.webkit.org/250352@main
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@293894 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2022-05-06 Jer Noble <jer.no...@apple.com>
+
+ [iOS] Infinite recursion at -[WKFullScreenWindowController _exitFullscreenImmediately]
+ https://bugs.webkit.org/show_bug.cgi?id=239744
+ <rdar://74201964>
+
+ Reviewed by Eric Carlson.
+
+ Even after fullscreen is correctly torn down during an immediate exit, fullscreen can never again
+ be entered for the lifetime of the page. This is due to the ivar m_pendingExitFullscreen being set
+ to true with no opportunity for that state to be cleared. This occurs because the callbacks for
+ exiting fullscreen are performed before the lambda form FullscreenManager::exitFullscreen() is
+ called.
+
+ Set m_pendingExitFullscreen inside exitFullscreen(), rather than the lambda, and clear it explicitly
+ in the cases where fullscreen exiting is not actually necessary.
+
+ The mock behavior of Fullscreen inside DumpRenderTree and WebKitTestRunner are not able to exercise
+ this same path. Track improvements to this mock in bug #239747.
+
+ * dom/FullscreenManager.cpp:
+ (WebCore::FullscreenManager::requestFullscreenForElement):
+ (WebCore::FullscreenManager::exitFullscreen):
+ (WebCore::FullscreenManager::setAnimatingFullscreen):
+ (WebCore::FullscreenManager::setFullscreenControlsHidden):
+
+2022-05-16 Alan Coon <alanc...@apple.com>
+
Cherry-pick r293647. rdar://problem/92257660
Only stretch the percent height <body> when it is the document element's child
Modified: branches/safari-613-branch/Source/WebCore/dom/FullscreenManager.cpp (294300 => 294301)
--- branches/safari-613-branch/Source/WebCore/dom/FullscreenManager.cpp 2022-05-17 06:10:51 UTC (rev 294300)
+++ branches/safari-613-branch/Source/WebCore/dom/FullscreenManager.cpp 2022-05-17 06:10:55 UTC (rev 294301)
@@ -113,6 +113,8 @@
}
}
+ INFO_LOG(LOGIDENTIFIER);
+
m_pendingFullscreenElement = RefPtr { element.ptr() };
m_document.eventLoop().queueTask(TaskSource::MediaElement, [this, weakThis = WeakPtr { *this }, element = WTFMove(element), checkType, hasKeyboardAccess, failedPreflights, identifier = LOGIDENTIFIER] () mutable {
@@ -289,6 +291,8 @@
return;
}
+ INFO_LOG(LOGIDENTIFIER);
+
// 3. Let descendants be all the doc's descendant browsing context's documents with a non-empty fullscreen
// element stack (if any), ordered so that the child of the doc is last and the document furthest
// away from the doc is first.
@@ -332,6 +336,8 @@
currentDoc = nullptr;
}
+ m_pendingExitFullscreen = true;
+
// 6. Return, and run the remaining steps asynchronously.
// 7. Optionally, perform some animation.
m_document.eventLoop().queueTask(TaskSource::MediaElement, [this, weakThis = WeakPtr { *this }, newTop = RefPtr { newTop }, fullscreenElement = m_fullscreenElement, identifier = LOGIDENTIFIER] {
@@ -340,6 +346,7 @@
auto* page = this->page();
if (!page) {
+ m_pendingExitFullscreen = false;
ERROR_LOG(identifier, "task - Document not in page; bailing.");
return;
}
@@ -350,6 +357,7 @@
if (!fullscreenElement && m_pendingFullscreenElement) {
INFO_LOG(identifier, "task - Cancelling pending fullscreen request.");
m_pendingFullscreenElement = nullptr;
+ m_pendingExitFullscreen = false;
return;
}
@@ -356,7 +364,6 @@
// Only exit out of full screen window mode if there are no remaining elements in the
// full screen stack.
if (!newTop) {
- m_pendingExitFullscreen = true;
INFO_LOG(identifier, "task - Empty fullscreen stack; exiting.");
page->chrome().client().exitFullScreenForElement(fullscreenElement.get());
return;
@@ -363,6 +370,8 @@
}
// Otherwise, notify the chrome of the new full screen element.
+ m_pendingExitFullscreen = false;
+
INFO_LOG(identifier, "task - New top of fullscreen stack.");
page->chrome().client().enterFullScreenForElement(*newTop);
});
@@ -629,6 +638,8 @@
return;
m_isAnimatingFullscreen = flag;
+ INFO_LOG(LOGIDENTIFIER, flag);
+
if (m_fullscreenElement && m_fullscreenElement->isDescendantOf(document())) {
m_fullscreenElement->invalidateStyleForSubtree();
scheduleFullStyleRebuild();
@@ -644,6 +655,9 @@
{
if (m_areFullscreenControlsHidden == flag)
return;
+
+ INFO_LOG(LOGIDENTIFIER, flag);
+
m_areFullscreenControlsHidden = flag;
if (m_fullscreenElement && m_fullscreenElement->isDescendantOf(document())) {
Modified: branches/safari-613-branch/Source/WebKit/ChangeLog (294300 => 294301)
--- branches/safari-613-branch/Source/WebKit/ChangeLog 2022-05-17 06:10:51 UTC (rev 294300)
+++ branches/safari-613-branch/Source/WebKit/ChangeLog 2022-05-17 06:10:55 UTC (rev 294301)
@@ -1,5 +1,84 @@
2022-05-16 Alan Coon <alanc...@apple.com>
+ Cherry-pick r293894. rdar://problem/74201964
+
+ [iOS] Infinite recursion at -[WKFullScreenWindowController _exitFullscreenImmediately]
+ https://bugs.webkit.org/show_bug.cgi?id=239744
+ <rdar://74201964>
+
+ Reviewed by Eric Carlson.
+
+ Source/WebCore:
+
+ Even after fullscreen is correctly torn down during an immediate exit, fullscreen can never again
+ be entered for the lifetime of the page. This is due to the ivar m_pendingExitFullscreen being set
+ to true with no opportunity for that state to be cleared. This occurs because the callbacks for
+ exiting fullscreen are performed before the lambda form FullscreenManager::exitFullscreen() is
+ called.
+
+ Set m_pendingExitFullscreen inside exitFullscreen(), rather than the lambda, and clear it explicitly
+ in the cases where fullscreen exiting is not actually necessary.
+
+ The mock behavior of Fullscreen inside DumpRenderTree and WebKitTestRunner are not able to exercise
+ this same path. Track improvements to this mock in bug #239747.
+
+ * dom/FullscreenManager.cpp:
+ (WebCore::FullscreenManager::requestFullscreenForElement):
+ (WebCore::FullscreenManager::exitFullscreen):
+ (WebCore::FullscreenManager::setAnimatingFullscreen):
+ (WebCore::FullscreenManager::setFullscreenControlsHidden):
+
+ Source/WebKit:
+
+ When exiting fullscreen immediately, we trigger the correct state transition by calling
+ -[self exitFullScreen]. However, when no fullscreen manager exists (as may happen during
+ navigation), -exitFullscreen may itself call -_exitFullscreenImmediately, which causes an
+ infinite recursion and stack overflow.
+
+ Unroll the implementation by performing all the same pieces as would have been done by
+ -exitFullScreen, but inline. Refactor out common behavior into -_reinsertWebViewUnderPlaceholder,
+ and call that both from -_exitFullscreenImmediately and _completedExitFullScreen.
+
+ Unfortunately, no API tests are possible here because TestWebKitAPI is not an actual iOS UI
+ application.
+
+ * UIProcess/ios/fullscreen/WKFullScreenWindowControllerIOS.mm:
+ (-[WKFullScreenWindowController exitFullScreen]):
+ (-[WKFullScreenWindowController _reinsertWebViewUnderPlaceholder]):
+ (-[WKFullScreenWindowController _completedExitFullScreen]):
+ (-[WKFullScreenWindowController _exitFullscreenImmediately]):
+
+ Canonical link: https://commits.webkit.org/250352@main
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@293894 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2022-05-06 Jer Noble <jer.no...@apple.com>
+
+ [iOS] Infinite recursion at -[WKFullScreenWindowController _exitFullscreenImmediately]
+ https://bugs.webkit.org/show_bug.cgi?id=239744
+ <rdar://74201964>
+
+ Reviewed by Eric Carlson.
+
+ When exiting fullscreen immediately, we trigger the correct state transition by calling
+ -[self exitFullScreen]. However, when no fullscreen manager exists (as may happen during
+ navigation), -exitFullscreen may itself call -_exitFullscreenImmediately, which causes an
+ infinite recursion and stack overflow.
+
+ Unroll the implementation by performing all the same pieces as would have been done by
+ -exitFullScreen, but inline. Refactor out common behavior into -_reinsertWebViewUnderPlaceholder,
+ and call that both from -_exitFullscreenImmediately and _completedExitFullScreen.
+
+ Unfortunately, no API tests are possible here because TestWebKitAPI is not an actual iOS UI
+ application.
+
+ * UIProcess/ios/fullscreen/WKFullScreenWindowControllerIOS.mm:
+ (-[WKFullScreenWindowController exitFullScreen]):
+ (-[WKFullScreenWindowController _reinsertWebViewUnderPlaceholder]):
+ (-[WKFullScreenWindowController _completedExitFullScreen]):
+ (-[WKFullScreenWindowController _exitFullscreenImmediately]):
+
+2022-05-16 Alan Coon <alanc...@apple.com>
+
Cherry-pick r293433. rdar://problem/91981865
[WebAuthn] Do not pass ASCCredentialRequestTypePlatform... if LocalService unavailable
Modified: branches/safari-613-branch/Source/WebKit/UIProcess/ios/fullscreen/WKFullScreenWindowControllerIOS.mm (294300 => 294301)
--- branches/safari-613-branch/Source/WebKit/UIProcess/ios/fullscreen/WKFullScreenWindowControllerIOS.mm 2022-05-17 06:10:51 UTC (rev 294300)
+++ branches/safari-613-branch/Source/WebKit/UIProcess/ios/fullscreen/WKFullScreenWindowControllerIOS.mm 2022-05-17 06:10:55 UTC (rev 294301)
@@ -742,6 +742,9 @@
- (void)exitFullScreen
{
+ if (_fullScreenState == WebKit::NotInFullScreen)
+ return;
+
if (_fullScreenState < WebKit::InFullScreen) {
_exitRequested = YES;
return;
@@ -786,12 +789,8 @@
[self _dismissFullscreenViewController];
}
-- (void)_completedExitFullScreen
+- (void)_reinsertWebViewUnderPlaceholder
{
- if (_fullScreenState != WebKit::ExitingFullScreen)
- return;
- _fullScreenState = WebKit::NotInFullScreen;
-
[CATransaction begin];
[CATransaction setDisableActions:YES];
@@ -811,7 +810,16 @@
[webView layoutIfNeeded];
[CATransaction commit];
+}
+- (void)_completedExitFullScreen
+{
+ if (_fullScreenState != WebKit::ExitingFullScreen)
+ return;
+ _fullScreenState = WebKit::NotInFullScreen;
+
+ [self _reinsertWebViewUnderPlaceholder];
+
if (auto* manager = self._manager) {
manager->restoreScrollPosition();
manager->setAnimatingFullScreen(false);
@@ -978,19 +986,39 @@
return;
_shouldReturnToFullscreenFromPictureInPicture = false;
+ _exitRequested = NO;
+ _exitingFullScreen = NO;
+ _fullScreenState = WebKit::NotInFullScreen;
+ _shouldReturnToFullscreenFromPictureInPicture = false;
- auto* manager = self._manager;
- if (manager)
+ auto* page = [self._webView _page].get();
+ if (page)
+ page->setSuppressVisibilityUpdates(true);
+
+ [self _reinsertWebViewUnderPlaceholder];
+
+ if (auto* manager = self._manager) {
manager->requestExitFullScreen();
- [self exitFullScreen];
- _fullScreenState = WebKit::ExitingFullScreen;
- [self _completedExitFullScreen];
- RetainPtr<WKWebView> webView = self._webView;
- _webViewPlaceholder.get().parent = nil;
- WebKit::replaceViewWithView(_webViewPlaceholder.get(), webView.get());
- if (auto page = [webView _page])
+ manager->setAnimatingFullScreen(true);
+ manager->willExitFullScreen();
+ manager->restoreScrollPosition();
+ manager->setAnimatingFullScreen(false);
+ manager->didExitFullScreen();
+ }
+
+ [_webViewPlaceholder removeFromSuperview];
+ _webViewPlaceholder = nil;
+ [_window setHidden:YES];
+ _window = nil;
+
+ if (page) {
page->setSuppressVisibilityUpdates(false);
- _webViewPlaceholder = nil;
+ page->setNeedsDOMWindowResizeEvent();
+ }
+
+ [_fullscreenViewController setPrefersStatusBarHidden:YES];
+ [_fullscreenViewController invalidate];
+ _fullscreenViewController = nil;
}
- (void)_invalidateEVOrganizationName
