Skip to content

Commit

Permalink
[iOS] No media controls after restoring fullscreen from PiP
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=248834
rdar://102880747

Reviewed by Eric Carlson and Jean-Yves Avenard.

When attempting to "restore" fullscreen mode from PiP, WebFullScreenManager uses
the m_element ivar to decide which element to request element fullscreen. However
in 256812@main, m_element is cleared upon exit.

Restore the functionality by introducing a new ivar, m_elementToRestore. This WeakPtr
will be queried when requesting fullscreen and, if valid, will be used instead of
m_element.

To make it more clear that we are requesting fullscreen mode be restored, rename
all instances of requestEnterFullScreen to requestRestoreFullScreen.

Add a XCUITest that exercises entering video fullscreen, entering PiP, and restoring
to fullscreen twice.

* Source/WebKit/UIProcess/WebFullScreenManagerProxy.cpp:
(WebKit::WebFullScreenManagerProxy::requestRestoreFullScreen):
(WebKit::WebFullScreenManagerProxy::requestEnterFullScreen): Deleted.
* Source/WebKit/UIProcess/WebFullScreenManagerProxy.h:
* Source/WebKit/UIProcess/ios/fullscreen/WKFullScreenWindowControllerIOS.h:
* Source/WebKit/UIProcess/ios/fullscreen/WKFullScreenWindowControllerIOS.mm:
(-[WKFullScreenWindowController requestRestoreFullScreen]):
(-[WKFullScreenWindowController _completedExitFullScreen]):
(-[WKFullScreenWindowController didExitPictureInPicture]):
(-[WKFullScreenWindowController requestEnterFullScreen]): Deleted.
* Source/WebKit/WebProcess/FullScreen/WebFullScreenManager.cpp:
(WebKit::WebFullScreenManager::setElement):
(WebKit::WebFullScreenManager::requestRestoreFullScreen):
(WebKit::WebFullScreenManager::requestEnterFullScreen): Deleted.
* Source/WebKit/WebProcess/FullScreen/WebFullScreenManager.h:
* Source/WebKit/WebProcess/FullScreen/WebFullScreenManager.messages.in:
* Tools/MobileMiniBrowser/MobileMiniBrowser.xcodeproj/project.pbxproj:
* Tools/MobileMiniBrowser/MobileMiniBrowser.xcodeproj/xcshareddata/xcschemes/MobileMiniBrowser.xcscheme:
* Tools/MobileMiniBrowser/MobileMiniBrowserFramework/Assets.xcassets/AppIcon.appiconset/Contents.json:
* Tools/MobileMiniBrowser/MobileMiniBrowserFramework/WebViewController.m:
(-[WebViewController createWebView]):
(-[WebViewController targetURLorDefaultURL]):
* Tools/MobileMiniBrowser/MobileMiniBrowserUITests/MobileMiniBrowserUITests.m:
(-[MobileMiniBrowserUITests setUp]):
(-[MobileMiniBrowserUITests waitForWindowNamed:forApp:]):
(-[MobileMiniBrowserUITests waitForOtherElementNamed:forApp:]):
(-[MobileMiniBrowserUITests tapMiddleTopOfApp:]):
(-[MobileMiniBrowserUITests launchURL:]):
(-[MobileMiniBrowserUITests launchPageNamed:]):
(-[MobileMiniBrowserUITests testBasicVideoPlayback]):
(-[MobileMiniBrowserUITests testBasicVideoFullscreen]):
(-[MobileMiniBrowserUITests testRepeatedFullScreenToPiPAndBack]):
(-[MobileMiniBrowserUITests testVideoFullscreenAndRotationAnimation]):
(-[MobileMiniBrowserUITests testVideoFullscreenControlCenter]):
(-[MobileMiniBrowserUITests testLoopingFullscreenLockup]):

Canonical link: https://commits.webkit.org/257486@main
  • Loading branch information
jernoble committed Dec 7, 2022
1 parent 2a77eef commit 61358f6
Show file tree
Hide file tree
Showing 13 changed files with 180 additions and 114 deletions.
2 changes: 1 addition & 1 deletion Source/WebKit/UIProcess/API/wpe/WPEView.cpp
Expand Up @@ -294,7 +294,7 @@ View::View(struct wpe_view_backend* backend, const API::PageConfiguration& baseC
[](void* data)
{
auto& view = *reinterpret_cast<View*>(data);
view.page().fullScreenManager()->requestEnterFullScreen();
view.page().fullScreenManager()->requestRestoreFullScreen();
},
// request_exit_fullscreen
[](void* data)
Expand Down
4 changes: 2 additions & 2 deletions Source/WebKit/UIProcess/WebFullScreenManagerProxy.cpp
Expand Up @@ -112,9 +112,9 @@ void WebFullScreenManagerProxy::setAnimatingFullScreen(bool animating)
m_page.send(Messages::WebFullScreenManager::SetAnimatingFullScreen(animating));
}

void WebFullScreenManagerProxy::requestEnterFullScreen()
void WebFullScreenManagerProxy::requestRestoreFullScreen()
{
m_page.send(Messages::WebFullScreenManager::RequestEnterFullScreen());
m_page.send(Messages::WebFullScreenManager::RequestRestoreFullScreen());
}

void WebFullScreenManagerProxy::requestExitFullScreen()
Expand Down
2 changes: 1 addition & 1 deletion Source/WebKit/UIProcess/WebFullScreenManagerProxy.h
Expand Up @@ -93,7 +93,7 @@ class WebFullScreenManagerProxy : public IPC::MessageReceiver {
void willExitFullScreen();
void didExitFullScreen();
void setAnimatingFullScreen(bool);
void requestEnterFullScreen();
void requestRestoreFullScreen();
void requestExitFullScreen();
void saveScrollPosition();
void restoreScrollPosition();
Expand Down
Expand Up @@ -36,7 +36,7 @@
- (id)initWithWebView:(WKWebView *)webView;
- (void)enterFullScreen:(CGSize)videoDimensions;
- (void)beganEnterFullScreenWithInitialFrame:(CGRect)initialFrame finalFrame:(CGRect)finalFrame;
- (void)requestEnterFullScreen;
- (void)requestRestoreFullScreen;
- (void)requestExitFullScreen;
- (void)exitFullScreen;
- (void)beganExitFullScreenWithInitialFrame:(CGRect)initialFrame finalFrame:(CGRect)finalFrame;
Expand Down
Expand Up @@ -774,7 +774,7 @@ - (void)beganEnterFullScreenWithInitialFrame:(CGRect)initialFrame finalFrame:(CG
}];
}

- (void)requestEnterFullScreen
- (void)requestRestoreFullScreen
{
if (_fullScreenState != WebKit::NotInFullScreen)
return;
Expand All @@ -784,7 +784,7 @@ - (void)requestEnterFullScreen
page->fullscreenMayReturnToInline();

if (auto* manager = self._manager) {
manager->requestEnterFullScreen();
manager->requestRestoreFullScreen();
return;
}

Expand Down Expand Up @@ -936,7 +936,7 @@ - (void)_completedExitFullScreen
_exitingFullScreen = NO;
if (_enterRequested) {
_enterRequested = NO;
[self requestEnterFullScreen];
[self requestRestoreFullScreen];
}
});

Expand Down Expand Up @@ -997,7 +997,7 @@ - (void)didExitPictureInPicture
if (_fullScreenState == WebKit::InFullScreen)
videoFullscreenInterface->preparedToReturnToStandby();
else
[self requestEnterFullScreen];
[self requestRestoreFullScreen];
} else
_enterRequested = YES;

Expand Down
15 changes: 10 additions & 5 deletions Source/WebKit/WebProcess/FullScreen/WebFullScreenManager.cpp
Expand Up @@ -159,6 +159,7 @@ void WebFullScreenManager::setElement(WebCore::Element& element)
clearElement();

m_element = &element;
m_elementToRestore = element;

for (auto& eventName : eventsToObserve())
m_element->addEventListener(eventName, *this, { true });
Expand Down Expand Up @@ -328,14 +329,18 @@ void WebFullScreenManager::setAnimatingFullScreen(bool animating)
m_element->document().fullscreenManager().setAnimatingFullscreen(animating);
}

void WebFullScreenManager::requestEnterFullScreen()
void WebFullScreenManager::requestRestoreFullScreen()
{
ASSERT(m_element);
if (!m_element)
ASSERT(!m_element);
if (m_element)
return;

auto element = RefPtr { m_elementToRestore.get() };
if (!element)
return;

WebCore::UserGestureIndicator gestureIndicator(WebCore::ProcessingUserGesture);
m_element->document().fullscreenManager().requestFullscreenForElement(*m_element, nullptr, WebCore::FullscreenManager::ExemptIFrameAllowFullscreenRequirement);
WebCore::UserGestureIndicator gestureIndicator(WebCore::ProcessingUserGesture, &element->document());
element->document().fullscreenManager().requestFullscreenForElement(*element, nullptr, WebCore::FullscreenManager::ExemptIFrameAllowFullscreenRequirement);
}

void WebFullScreenManager::requestExitFullScreen()
Expand Down
3 changes: 2 additions & 1 deletion Source/WebKit/WebProcess/FullScreen/WebFullScreenManager.h
Expand Up @@ -81,7 +81,7 @@ class WebFullScreenManager final : public WebCore::EventListener {
void setPIPStandbyElement(WebCore::HTMLVideoElement*);

void setAnimatingFullScreen(bool);
void requestEnterFullScreen();
void requestRestoreFullScreen();
void requestExitFullScreen();
void saveScrollPosition();
void restoreScrollPosition();
Expand All @@ -97,6 +97,7 @@ class WebFullScreenManager final : public WebCore::EventListener {
float m_topContentInset { 0 };
RefPtr<WebPage> m_page;
RefPtr<WebCore::Element> m_element;
WeakPtr<WebCore::Element, WebCore::WeakPtrImplWithEventTargetData> m_elementToRestore;
#if ENABLE(VIDEO)
RefPtr<WebCore::HTMLVideoElement> m_pipStandbyElement;
#endif
Expand Down
Expand Up @@ -22,7 +22,7 @@

#if ENABLE(FULLSCREEN_API)
messages -> WebFullScreenManager LegacyReceiver {
RequestEnterFullScreen()
RequestRestoreFullScreen()
RequestExitFullScreen()
WillEnterFullScreen()
DidEnterFullScreen()
Expand Down
46 changes: 23 additions & 23 deletions Tools/MobileMiniBrowser/MobileMiniBrowser.xcodeproj/project.pbxproj
Expand Up @@ -21,8 +21,6 @@
/* End PBXAggregateTarget section */

/* Begin PBXBuildFile section */
3F0B439B1D908D0C00D186B5 /* looping2s.html in Resources */ = {isa = PBXBuildFile; fileRef = 3F0B439A1D908D0C00D186B5 /* looping2s.html */; };
3F0B439D1D908DE700D186B5 /* test2s.mp4 in Resources */ = {isa = PBXBuildFile; fileRef = 3F0B439C1D908DE700D186B5 /* test2s.mp4 */; };
457D0F7A29088D9000AE5914 /* SettingsViewController.h in Headers */ = {isa = PBXBuildFile; fileRef = 457D0F7729088C9500AE5914 /* SettingsViewController.h */; };
45BE567C2908C74600FD7A95 /* SettingsViewController.m in Sources */ = {isa = PBXBuildFile; fileRef = 457D0F7829088D4000AE5914 /* SettingsViewController.m */; };
CD1DAF971D709E3600017CF0 /* main.m in Sources */ = {isa = PBXBuildFile; fileRef = CD1DAF961D709E3600017CF0 /* main.m */; };
Expand All @@ -39,9 +37,11 @@
CD498B501D763D7600681FA7 /* AppDelegate.h in Headers */ = {isa = PBXBuildFile; fileRef = CD1DAF981D709E3600017CF0 /* AppDelegate.h */; settings = {ATTRIBUTES = (Public, ); }; };
CD498B511D763D7F00681FA7 /* Main.storyboard in Resources */ = {isa = PBXBuildFile; fileRef = CD1DAF9E1D709E3600017CF0 /* Main.storyboard */; };
CD498B521D763D8800681FA7 /* Assets.xcassets in Resources */ = {isa = PBXBuildFile; fileRef = CD1DAFA11D709E3600017CF0 /* Assets.xcassets */; };
CDA985191D76483400EBC399 /* test.mp4 in Resources */ = {isa = PBXBuildFile; fileRef = CDA985151D76477900EBC399 /* test.mp4 */; };
CDA9851A1D76483400EBC399 /* index.html in Resources */ = {isa = PBXBuildFile; fileRef = CDA985131D76474100EBC399 /* index.html */; };
CDA9851B1D76483400EBC399 /* looping.html in Resources */ = {isa = PBXBuildFile; fileRef = CDA985171D7647CD00EBC399 /* looping.html */; };
CDC2792A2935418400151088 /* looping.html in Resources */ = {isa = PBXBuildFile; fileRef = CDC279272935417100151088 /* looping.html */; };
CDC2792B2935418400151088 /* test2s.mp4 in Resources */ = {isa = PBXBuildFile; fileRef = CDC279252935417100151088 /* test2s.mp4 */; };
CDC2792C2935418400151088 /* test.mp4 in Resources */ = {isa = PBXBuildFile; fileRef = CDC279282935417100151088 /* test.mp4 */; };
CDC2792D2935418400151088 /* looping2s.html in Resources */ = {isa = PBXBuildFile; fileRef = CDC279292935417100151088 /* looping2s.html */; };
CDC2792E2935418400151088 /* index.html in Resources */ = {isa = PBXBuildFile; fileRef = CDC279262935417100151088 /* index.html */; };
DD403C5828500FE300D899FC /* WebKit.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = CD1DAFC11D70E12D00017CF0 /* WebKit.framework */; };
/* End PBXBuildFile section */

Expand Down Expand Up @@ -84,8 +84,6 @@
/* End PBXCopyFilesBuildPhase section */

/* Begin PBXFileReference section */
3F0B439A1D908D0C00D186B5 /* looping2s.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; name = looping2s.html; path = Resources/looping2s.html; sourceTree = "<group>"; };
3F0B439C1D908DE700D186B5 /* test2s.mp4 */ = {isa = PBXFileReference; lastKnownFileType = file; name = test2s.mp4; path = Resources/test2s.mp4; sourceTree = "<group>"; };
457D0F7729088C9500AE5914 /* SettingsViewController.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = SettingsViewController.h; sourceTree = "<group>"; };
457D0F7829088D4000AE5914 /* SettingsViewController.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = SettingsViewController.m; sourceTree = "<group>"; };
CD1DAF921D709E3600017CF0 /* MobileMiniBrowser.app */ = {isa = PBXFileReference; explicitFileType = wrapper.application; includeInIndex = 0; path = MobileMiniBrowser.app; sourceTree = BUILT_PRODUCTS_DIR; };
Expand All @@ -110,9 +108,11 @@
CD4DEEE21D78C6FF00625986 /* Base.xcconfig */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.xcconfig; name = Base.xcconfig; path = Configurations/Base.xcconfig; sourceTree = "<group>"; };
CD4DEEE31D78C6FF00625986 /* DebugRelease.xcconfig */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.xcconfig; name = DebugRelease.xcconfig; path = Configurations/DebugRelease.xcconfig; sourceTree = "<group>"; };
CD4DEEE41D78C6FF00625986 /* MobileMiniBrowser.xcconfig */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.xcconfig; name = MobileMiniBrowser.xcconfig; path = Configurations/MobileMiniBrowser.xcconfig; sourceTree = "<group>"; };
CDA985131D76474100EBC399 /* index.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; name = index.html; path = Resources/index.html; sourceTree = "<group>"; };
CDA985151D76477900EBC399 /* test.mp4 */ = {isa = PBXFileReference; lastKnownFileType = file; name = test.mp4; path = Resources/test.mp4; sourceTree = "<group>"; };
CDA985171D7647CD00EBC399 /* looping.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; name = looping.html; path = Resources/looping.html; sourceTree = "<group>"; };
CDC279252935417100151088 /* test2s.mp4 */ = {isa = PBXFileReference; lastKnownFileType = file; path = test2s.mp4; sourceTree = "<group>"; };
CDC279262935417100151088 /* index.html */ = {isa = PBXFileReference; lastKnownFileType = text.html; path = index.html; sourceTree = "<group>"; };
CDC279272935417100151088 /* looping.html */ = {isa = PBXFileReference; lastKnownFileType = text.html; path = looping.html; sourceTree = "<group>"; };
CDC279282935417100151088 /* test.mp4 */ = {isa = PBXFileReference; lastKnownFileType = file; path = test.mp4; sourceTree = "<group>"; };
CDC279292935417100151088 /* looping2s.html */ = {isa = PBXFileReference; lastKnownFileType = text.html; path = looping2s.html; sourceTree = "<group>"; };
/* End PBXFileReference section */

/* Begin PBXFrameworksBuildPhase section */
Expand Down Expand Up @@ -178,6 +178,7 @@
CD1DAFAE1D709E3600017CF0 /* MobileMiniBrowserUITests */ = {
isa = PBXGroup;
children = (
CDC279242935417100151088 /* Resources */,
CD1DAFB11D709E3600017CF0 /* Info.plist */,
CD1DAFAF1D709E3600017CF0 /* MobileMiniBrowserUITests.m */,
);
Expand All @@ -195,7 +196,6 @@
CD498B3C1D76348000681FA7 /* MobileMiniBrowser Framework */ = {
isa = PBXGroup;
children = (
CDA985121D76471700EBC399 /* Resources */,
CD1DAF981D709E3600017CF0 /* AppDelegate.h */,
CD1DAF991D709E3600017CF0 /* AppDelegate.m */,
CD1DAFA11D709E3600017CF0 /* Assets.xcassets */,
Expand Down Expand Up @@ -223,16 +223,16 @@
name = Configurations;
sourceTree = "<group>";
};
CDA985121D76471700EBC399 /* Resources */ = {
CDC279242935417100151088 /* Resources */ = {
isa = PBXGroup;
children = (
CDA985131D76474100EBC399 /* index.html */,
CDA985171D7647CD00EBC399 /* looping.html */,
3F0B439A1D908D0C00D186B5 /* looping2s.html */,
CDA985151D76477900EBC399 /* test.mp4 */,
3F0B439C1D908DE700D186B5 /* test2s.mp4 */,
CDC279262935417100151088 /* index.html */,
CDC279272935417100151088 /* looping.html */,
CDC279292935417100151088 /* looping2s.html */,
CDC279282935417100151088 /* test.mp4 */,
CDC279252935417100151088 /* test2s.mp4 */,
);
name = Resources;
path = Resources;
sourceTree = "<group>";
};
/* End PBXGroup section */
Expand Down Expand Up @@ -367,6 +367,11 @@
isa = PBXResourcesBuildPhase;
buildActionMask = 2147483647;
files = (
CDC2792E2935418400151088 /* index.html in Resources */,
CDC2792A2935418400151088 /* looping.html in Resources */,
CDC2792D2935418400151088 /* looping2s.html in Resources */,
CDC2792C2935418400151088 /* test.mp4 in Resources */,
CDC2792B2935418400151088 /* test2s.mp4 in Resources */,
);
runOnlyForDeploymentPostprocessing = 0;
};
Expand All @@ -375,12 +380,7 @@
buildActionMask = 2147483647;
files = (
CD498B521D763D8800681FA7 /* Assets.xcassets in Resources */,
CDA9851A1D76483400EBC399 /* index.html in Resources */,
CDA9851B1D76483400EBC399 /* looping.html in Resources */,
3F0B439B1D908D0C00D186B5 /* looping2s.html in Resources */,
CD498B511D763D7F00681FA7 /* Main.storyboard in Resources */,
CDA985191D76483400EBC399 /* test.mp4 in Resources */,
3F0B439D1D908DE700D186B5 /* test2s.mp4 in Resources */,
);
runOnlyForDeploymentPostprocessing = 0;
};
Expand Down
Expand Up @@ -28,6 +28,16 @@
selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB"
shouldUseLaunchSchemeArgsEnv = "YES">
<Testables>
<TestableReference
skipped = "NO">
<BuildableReference
BuildableIdentifier = "primary"
BlueprintIdentifier = "CD1DAFAA1D709E3600017CF0"
BuildableName = "MobileMiniBrowserUITests.xctest"
BlueprintName = "MobileMiniBrowserUITests"
ReferencedContainer = "container:MobileMiniBrowser.xcodeproj">
</BuildableReference>
</TestableReference>
</Testables>
</TestAction>
<LaunchAction
Expand Down
Expand Up @@ -2,92 +2,97 @@
"images" : [
{
"idiom" : "iphone",
"size" : "20x20",
"scale" : "2x"
"scale" : "2x",
"size" : "20x20"
},
{
"idiom" : "iphone",
"size" : "20x20",
"scale" : "3x"
"scale" : "3x",
"size" : "20x20"
},
{
"idiom" : "iphone",
"size" : "29x29",
"scale" : "2x"
"scale" : "2x",
"size" : "29x29"
},
{
"idiom" : "iphone",
"size" : "29x29",
"scale" : "3x"
"scale" : "3x",
"size" : "29x29"
},
{
"idiom" : "iphone",
"size" : "40x40",
"scale" : "2x"
"scale" : "2x",
"size" : "40x40"
},
{
"idiom" : "iphone",
"size" : "40x40",
"scale" : "3x"
"scale" : "3x",
"size" : "40x40"
},
{
"idiom" : "iphone",
"size" : "60x60",
"scale" : "2x"
"scale" : "2x",
"size" : "60x60"
},
{
"idiom" : "iphone",
"size" : "60x60",
"scale" : "3x"
"scale" : "3x",
"size" : "60x60"
},
{
"idiom" : "ipad",
"size" : "20x20",
"scale" : "1x"
"scale" : "1x",
"size" : "20x20"
},
{
"idiom" : "ipad",
"size" : "20x20",
"scale" : "2x"
"scale" : "2x",
"size" : "20x20"
},
{
"idiom" : "ipad",
"size" : "29x29",
"scale" : "1x"
"scale" : "1x",
"size" : "29x29"
},
{
"idiom" : "ipad",
"size" : "29x29",
"scale" : "2x"
"scale" : "2x",
"size" : "29x29"
},
{
"idiom" : "ipad",
"size" : "40x40",
"scale" : "1x"
"scale" : "1x",
"size" : "40x40"
},
{
"idiom" : "ipad",
"size" : "40x40",
"scale" : "2x"
"scale" : "2x",
"size" : "40x40"
},
{
"idiom" : "ipad",
"size" : "76x76",
"scale" : "1x"
"scale" : "1x",
"size" : "76x76"
},
{
"idiom" : "ipad",
"size" : "76x76",
"scale" : "2x"
"scale" : "2x",
"size" : "76x76"
},
{
"idiom" : "ipad",
"size" : "83.5x83.5",
"scale" : "2x"
"scale" : "2x",
"size" : "83.5x83.5"
},
{
"idiom" : "ios-marketing",
"scale" : "1x",
"size" : "1024x1024"
}
],
"info" : {
"version" : 1,
"author" : "xcode"
"author" : "xcode",
"version" : 1
}
}
}

0 comments on commit 61358f6

Please sign in to comment.