Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(34)

Unified Diff: third_party/WebKit/Source/web/FullscreenController.cpp

Issue 2573773002: Sync requestFullscreen() and exitFullscreen() algorithms with the spec (Closed)
Patch Set: address feedback Created 4 years ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: third_party/WebKit/Source/web/FullscreenController.cpp
diff --git a/third_party/WebKit/Source/web/FullscreenController.cpp b/third_party/WebKit/Source/web/FullscreenController.cpp
index 4099914d4cc5d3efcc3efca0d683062cd25b36d1..bc592d27f0e00ab04bc54a074dbd36fb46d3addd 100644
--- a/third_party/WebKit/Source/web/FullscreenController.cpp
+++ b/third_party/WebKit/Source/web/FullscreenController.cpp
@@ -88,6 +88,9 @@ void FullscreenController::didEnterFullscreen() {
fullscreen->didEnterFullscreen();
}
}
+
+ // TODO(foolip): If the top level browsing context (main frame) ends up with
+ // no fullscreen element, exit fullscreen again to recover.
}
void FullscreenController::didExitFullscreen() {
@@ -100,33 +103,32 @@ void FullscreenController::didExitFullscreen() {
updatePageScaleConstraints(true);
- // Set |m_state| so that any |exitFullscreen()| calls from within
- // |Fullscreen::didExitFullscreen()| do not call
- // |WebFrameClient::exitFullscreen()| again.
- // TODO(foolip): Remove this when state changes and events are synchronized
- // with animation frames. https://crbug.com/402376
- m_state = State::ExitingFullscreen;
+ // We need to wait until style and layout are updated in order to properly
+ // restore scroll offsets since content may not be overflowing in the same way
+ // until they are.
+ m_state = State::NeedsScrollAndScaleRestore;
- // Notify all local frames that we have exited fullscreen.
- // TODO(foolip): This should only need to notify the topmost local roots. That
- // doesn't currently work because |Fullscreen::m_currentFullScreenElement|
- // isn't set for the topmost document when an iframe goes fullscreen, but can
- // be done once |m_currentFullScreenElement| is gone and all state is in the
- // fullscreen element stack. https://crbug.com/402421
- for (Frame* frame = m_webViewImpl->page()->mainFrame(); frame;
- frame = frame->tree().traverseNext()) {
- if (!frame->isLocalFrame())
+ // Notify the topmost local frames that we have exited fullscreen.
+ // |Fullscreen::didExitFullscreen()| will take care of descendant frames.
+ for (Frame* frame = m_webViewImpl->page()->mainFrame(); frame;) {
+ Frame* nextFrame = frame->tree().traverseNext();
+
+ if (frame->isRemoteFrame()) {
+ frame = nextFrame;
continue;
+ }
+
+ DCHECK(frame->isLocalRoot());
if (Document* document = toLocalFrame(frame)->document()) {
if (Fullscreen* fullscreen = Fullscreen::fromIfExists(*document))
fullscreen->didExitFullscreen();
}
- }
- // We need to wait until style and layout are updated in order to properly
- // restore scroll offsets since content may not be overflowing in the same way
- // until they are.
- m_state = State::NeedsScrollAndScaleRestore;
+ // Skip over all descendant frames.
+ while (nextFrame && nextFrame->tree().isDescendantOf(frame))
+ nextFrame = nextFrame->tree().traverseNext();
+ frame = nextFrame;
+ }
}
void FullscreenController::enterFullscreen(LocalFrame& frame) {
@@ -183,7 +185,7 @@ void FullscreenController::fullscreenElementChanged(Element* fromElement,
DCHECK_NE(fromElement, toElement);
if (toElement) {
- DCHECK(Fullscreen::isCurrentFullScreenElement(*toElement));
+ DCHECK(Fullscreen::isFullscreenElement(*toElement));
if (isHTMLVideoElement(*toElement)) {
HTMLVideoElement& videoElement = toHTMLVideoElement(*toElement);
@@ -199,7 +201,7 @@ void FullscreenController::fullscreenElementChanged(Element* fromElement,
}
if (fromElement) {
- DCHECK(!Fullscreen::isCurrentFullScreenElement(*fromElement));
+ DCHECK(!Fullscreen::isFullscreenElement(*fromElement));
if (isHTMLVideoElement(*fromElement)) {
// If the video used overlay fullscreen mode, restore the transparency.
« no previous file with comments | « third_party/WebKit/Source/web/FullscreenController.h ('k') | third_party/WebKit/Source/web/WebPluginContainerImpl.cpp » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698