Commit 5e844264 authored by Rakina Zata Amni's avatar Rakina Zata Amni Committed by Commit Bot

bfcache: Update visibility after pagehide, before pageshow

Ensures visibilitychange fires before pageshow to follow the spec at
https://html.spec.whatwg.org/multipage/browsing-the-web.html#session-history-document-visibility-change-steps

Also ensures we would fire visibiltychange after pagehide, consistent
with non-bfcache navigations.

(Current attempt to change the spec to fire visibilitychange before
pagehide is deemed to be a bit risky, see I2S:
https://groups.google.com/a/chromium.org/g/blink-dev/c/R7h6InVXzcI/m/FNSaab_GCAAJ)

TBR=altimin@chromium.org

Bug: 1133363
Change-Id: I2d99bffc8710deea6ca4e0de09544bf7f5218496
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2435142
Commit-Queue: Rakina Zata Amni <rakina@chromium.org>
Reviewed-by: default avatarAlexander Timin <altimin@chromium.org>
Reviewed-by: default avatarKentaro Hara <haraken@chromium.org>
Reviewed-by: default avatarYuzu Saijo <yuzus@chromium.org>
Cr-Commit-Position: refs/heads/master@{#812066}
parent 177637be
......@@ -2825,10 +2825,10 @@ IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest, Events) {
// both window and document.
MatchEventList(
rfh_a,
ListValueOf("document.visibilitychange", "window.visibilitychange",
"window.pagehide.persisted", "document.freeze",
"document.resume", "window.pageshow.persisted",
"document.visibilitychange", "window.visibilitychange"));
ListValueOf("window.pagehide.persisted", "document.visibilitychange",
"window.visibilitychange", "document.freeze",
"document.resume", "document.visibilitychange",
"window.visibilitychange", "window.pageshow.persisted"));
}
// Tests the events are fired for subframes when going back from the cache.
......@@ -2874,16 +2874,16 @@ IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest, EventsForSubframes) {
// both window and document.
MatchEventList(
rfh_a,
ListValueOf("document.visibilitychange", "window.visibilitychange",
"window.pagehide.persisted", "document.freeze",
"document.resume", "window.pageshow.persisted",
"document.visibilitychange", "window.visibilitychange"));
ListValueOf("window.pagehide.persisted", "document.visibilitychange",
"window.visibilitychange", "document.freeze",
"document.resume", "document.visibilitychange",
"window.visibilitychange", "window.pageshow.persisted"));
MatchEventList(
rfh_b,
ListValueOf("document.visibilitychange", "window.visibilitychange",
"window.pagehide.persisted", "document.freeze",
"document.resume", "window.pageshow.persisted",
"document.visibilitychange", "window.visibilitychange"));
ListValueOf("window.pagehide.persisted", "document.visibilitychange",
"window.visibilitychange", "document.freeze",
"document.resume", "document.visibilitychange",
"window.visibilitychange", "window.pageshow.persisted"));
}
// Tests the events are fired when going back from the cache.
......@@ -2924,10 +2924,10 @@ IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest,
// both window and document.
MatchEventList(
rfh_a,
ListValueOf("document.visibilitychange", "window.visibilitychange",
"window.pagehide.persisted", "document.freeze",
"document.resume", "window.pageshow.persisted",
"document.visibilitychange", "window.visibilitychange"));
ListValueOf("window.pagehide.persisted", "document.visibilitychange",
"window.visibilitychange", "document.freeze",
"document.resume", "document.visibilitychange",
"window.visibilitychange", "window.pageshow.persisted"));
}
IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest, EvictPageWithInfiniteLoop) {
......
......@@ -3055,9 +3055,6 @@ void WebViewImpl::SetPageLifecycleStateInternal(
if (dispatching_pagehide) {
RemoveFocusAndTextInputState();
}
if (hiding_page) {
SetVisibilityState(new_state->visibility, /*is_initial_state=*/false);
}
if (dispatching_pagehide) {
// Note that |dispatching_pagehide| is different than |hiding_page|.
// |dispatching_pagehide| will only be true when we're navigating away from
......@@ -3066,6 +3063,9 @@ void WebViewImpl::SetPageLifecycleStateInternal(
// we're navigating away from a page, if the page is already hidden.
DispatchPagehide(new_state->pagehide_dispatch);
}
if (hiding_page) {
SetVisibilityState(new_state->visibility, /*is_initial_state=*/false);
}
if (storing_in_bfcache) {
Scheduler()->SetPageBackForwardCached(new_state->is_in_back_forward_cache);
}
......@@ -3085,6 +3085,9 @@ void WebViewImpl::SetPageLifecycleStateInternal(
}
if (resuming_page)
SetPageFrozen(false);
if (showing_page) {
SetVisibilityState(new_state->visibility, /*is_initial_state=*/false);
}
if (dispatching_pageshow) {
DCHECK(restoring_from_bfcache);
DispatchPageshow(page_restore_params->navigation_start);
......@@ -3094,9 +3097,6 @@ void WebViewImpl::SetPageLifecycleStateInternal(
DCHECK(page_restore_params);
Scheduler()->SetPageBackForwardCached(new_state->is_in_back_forward_cache);
}
if (showing_page) {
SetVisibilityState(new_state->visibility, /*is_initial_state=*/false);
}
// Make sure no TrackedFeaturesUpdate message is sent after the ACK
// TODO(carlscab): Do we really need to go through LocalFrame =>
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment