Skip to content

Commit a26a5b4

Browse files
mattwoodrowmattwoodrow
committed
[css-view-transitions-2] Traversal navigations to a cached entry should succeed even if the Document was created with a cross-origin redirect.
https://un5h2085w35jr3j0h78verhh.julianrbryant.com/show_bug.cgi?id=282012 <rdar://138517027> Reviewed by Charlie Wolfe. As per https://un5nj90kzk5vf152hgyfw29h1eja2.julianrbryant.com/multipage/browsing-the-web.html#updating-the-traversable:latest-entry, the cross-origin redirect check should only apply if not loading from BFCache. I'm also changing the view-transitions logic, which isn't currently in the spec, but is drafted here: w3c/csswg-drafts#11070. Without this change, navigating to a site via a cross-origin redirect marks the initial Document as having being created with a cross-origin redirect. If you then navigate within the site, and then back to the initial, the view transition is blocked due to this despite the redirect being unrelated to the 'back' traversal. * LayoutTests/http/wpt/css/css-view-transitions/navigation/cross-origin-bfcache-expected.txt: Added. * LayoutTests/http/wpt/css/css-view-transitions/navigation/cross-origin-bfcache.html: Added. * Source/WebCore/loader/DocumentLoader.cpp: (WebCore::DocumentLoader::navigationCanTriggerCrossDocumentViewTransition): * Source/WebCore/loader/DocumentLoader.h: * Source/WebCore/loader/FrameLoader.cpp: (WebCore::FrameLoader::commitProvisionalLoad): * Source/WebCore/page/Navigation.cpp: (WebCore::Navigation::createForPageswapEvent): * Source/WebCore/page/Navigation.h: Canonical link: https://un5kw2gk635jr3j0h78verhh.julianrbryant.com/285663@main
1 parent 99cf129 commit a26a5b4

File tree

7 files changed

+119
-8
lines changed

7 files changed

+119
-8
lines changed

‎LayoutTests/http/wpt/css/css-view-transitions/navigation/cross-origin-bfcache-expected.txt‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
2+
PASS VT object created on the old Document is skipped before persisting in BFCache
3+

‎LayoutTests/http/wpt/css/css-view-transitions/navigation/cross-origin-bfcache.html‎

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
<!-- webkit-test-runner [ UsesBackForwardCache=true ] -->
2+
<!DOCTYPE html>
3+
<title>VT object created on the old Document is skipped before persisting in BFCache</title>
4+
<link rel="help" href="https://un5n798jx6qx6j0rmf2verhh.julianrbryant.com/css-view-transitions-2/">
5+
<link rel="author" href="mailto:khushalsagar@chromium.org">
6+
<script src="/resources/testharness.js"></script>
7+
<script src="/resources/testharnessreport.js"></script>
8+
<script src="/common/utils.js"></script>
9+
<script src="/common/dispatcher/dispatcher.js"></script>
10+
<script src="/html/browsers/browsing-the-web/back-forward-cache/resources/helper.sub.js"></script>
11+
<script>
12+
// runBfcacheTest opens a popup to pageA which navigates to pageB and then
13+
// back, ensuring pageA is stored in the BFCache.
14+
15+
let originSameSiteCrossSiteRedirect = "https://127.0.0.1:8800/common/redirect.py?location=" + originSameOrigin;
16+
17+
runBfcacheTest({
18+
openFunc: url =>
19+
window.open(
20+
originSameSiteCrossSiteRedirect + url,
21+
"_blank",
22+
"noopener"
23+
),
24+
funcBeforeNavigation: async () => {
25+
// This function executes in pageA
26+
27+
function addTransitionOptIn() {
28+
let css = '@view-transition { navigation: auto; }';
29+
let style = document.createElement('style');
30+
style.appendChild(document.createTextNode(css));
31+
document.head.appendChild(style);
32+
}
33+
addTransitionOptIn();
34+
35+
// Wait for at least one frame to ensure there is a transition on the
36+
// navigation.
37+
function waitForAtLeastOneFrame() {
38+
return new Promise(resolve => {
39+
// Different web engines work slightly different on this area but waiting
40+
// for two requestAnimationFrames() to happen, one after another, should be
41+
// sufficient to ensure at least one frame has been generated anywhere.
42+
window.requestAnimationFrame(() => {
43+
window.requestAnimationFrame(() => {
44+
resolve();
45+
});
46+
});
47+
});
48+
}
49+
await waitForAtLeastOneFrame();
50+
51+
onpageswap = (e) => {
52+
if (e.viewTransition == null)
53+
return;
54+
55+
document.documentElement.classList.add('transition');
56+
57+
e.viewTransition.ready.catch(() => { });
58+
}
59+
onpagereveal = (e) => {
60+
if (e.viewTransition == null)
61+
return;
62+
63+
document.documentElement.classList.add('transition-back');
64+
}
65+
},
66+
funcBeforeBackNavigation: async () => {
67+
// This function executes in pageB
68+
69+
function addTransitionOptIn() {
70+
let css = '@view-transition { navigation: auto; }';
71+
let style = document.createElement('style');
72+
style.appendChild(document.createTextNode(css));
73+
document.head.appendChild(style);
74+
}
75+
addTransitionOptIn();
76+
77+
// Wait for at least one frame to ensure there is a transition on the
78+
// navigation.
79+
function waitForAtLeastOneFrame() {
80+
return new Promise(resolve => {
81+
// Different web engines work slightly different on this area but waiting
82+
// for two requestAnimationFrames() to happen, one after another, should be
83+
// sufficient to ensure at least one frame has been generated anywhere.
84+
window.requestAnimationFrame(() => {
85+
window.requestAnimationFrame(() => {
86+
resolve();
87+
});
88+
});
89+
});
90+
}
91+
await waitForAtLeastOneFrame();
92+
},
93+
funcAfterAssertion: async (pageA, pageB, t) => {
94+
assert_true(
95+
await pageA.execute_script(() => {
96+
return document.documentElement.classList.contains('transition');
97+
}),
98+
'navigation had viewTransition');
99+
100+
assert_true(
101+
await pageA.execute_script(() => {
102+
return document.documentElement.classList.contains('transition-back');
103+
}),
104+
'back navigation had viewTransition');
105+
},
106+
targetOrigin: originSameOrigin,
107+
});
108+
</script>

‎Source/WebCore/loader/DocumentLoader.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2490,7 +2490,7 @@ ShouldOpenExternalURLsPolicy DocumentLoader::shouldOpenExternalURLsPolicyToPropa
24902490
}
24912491

24922492
// https://un5gmtkzgkj46tygt32g.julianrbryant.com/TR/css-view-transitions-2/#navigation-can-trigger-a-cross-document-view-transition
2493-
bool DocumentLoader::navigationCanTriggerCrossDocumentViewTransition(Document& oldDocument)
2493+
bool DocumentLoader::navigationCanTriggerCrossDocumentViewTransition(Document& oldDocument, bool fromBackForwardCache)
24942494
{
24952495
// FIXME: Consider adding implementation-defined navigation experience step.
24962496

@@ -2504,7 +2504,7 @@ bool DocumentLoader::navigationCanTriggerCrossDocumentViewTransition(Document& o
25042504
if (!newOrigin->isSameOriginAs(oldDocument.securityOrigin()))
25052505
return false;
25062506

2507-
if (const auto* metrics = response().deprecatedNetworkLoadMetricsOrNull()) {
2507+
if (const auto* metrics = response().deprecatedNetworkLoadMetricsOrNull(); metrics && !fromBackForwardCache) {
25082508
if (metrics->crossOriginRedirect())
25092509
return false;
25102510
}

‎Source/WebCore/loader/DocumentLoader.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -524,7 +524,7 @@ class DocumentLoader
524524

525525
bool isInitialAboutBlank() const { return m_isInitialAboutBlank; }
526526

527-
bool navigationCanTriggerCrossDocumentViewTransition(Document& oldDocument);
527+
bool navigationCanTriggerCrossDocumentViewTransition(Document& oldDocument, bool fromBackForwardCache);
528528
WEBCORE_EXPORT void whenDocumentIsCreated(Function<void(Document*)>&&);
529529

530530
protected:

‎Source/WebCore/loader/FrameLoader.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2288,7 +2288,7 @@ void FrameLoader::commitProvisionalLoad()
22882288
bool canTriggerCrossDocumentViewTransition = false;
22892289
RefPtr<NavigationActivation> activation;
22902290
if (pdl) {
2291-
canTriggerCrossDocumentViewTransition = pdl->navigationCanTriggerCrossDocumentViewTransition(*document);
2291+
canTriggerCrossDocumentViewTransition = pdl->navigationCanTriggerCrossDocumentViewTransition(*document, !!cachedPage);
22922292

22932293
RefPtr domWindow = document->domWindow();
22942294
auto navigationAPIType = pdl->triggeringAction().navigationAPIType();
@@ -2301,7 +2301,7 @@ void FrameLoader::commitProvisionalLoad()
23012301
if (RefPtr page = frame->page(); page && *navigationAPIType != NavigationNavigationType::Reload)
23022302
newItem = frame->checkedHistory()->createItemWithLoader(page->historyItemClient(), pdl.get());
23032303

2304-
activation = domWindow->protectedNavigation()->createForPageswapEvent(newItem.get(), pdl.get());
2304+
activation = domWindow->protectedNavigation()->createForPageswapEvent(newItem.get(), pdl.get(), !!cachedPage);
23052305
}
23062306
}
23072307
document->dispatchPageswapEvent(canTriggerCrossDocumentViewTransition, WTFMove(activation));

‎Source/WebCore/page/Navigation.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -208,15 +208,15 @@ void Navigation::updateForActivation(HistoryItem* previousItem, std::optional<Na
208208
}
209209

210210
// https://un5nj90kzk5vf152hgyfw29h1eja2.julianrbryant.com/multipage/browsing-the-web.html#fire-the-pageswap-event
211-
RefPtr<NavigationActivation> Navigation::createForPageswapEvent(HistoryItem* newItem, DocumentLoader* documentLoader)
211+
RefPtr<NavigationActivation> Navigation::createForPageswapEvent(HistoryItem* newItem, DocumentLoader* documentLoader, bool fromBackForwardCache)
212212
{
213213
auto type = documentLoader->triggeringAction().navigationAPIType();
214214
if (!type || !frame())
215215
return nullptr;
216216

217217
// Skip cross-origin requests, or if any cross-origin redirects have been made.
218218
bool isSameOrigin = SecurityOrigin::create(documentLoader->documentURL())->isSameOriginAs(window()->protectedDocument()->securityOrigin());
219-
if (!isSameOrigin || !documentLoader->request().isSameSite())
219+
if (!isSameOrigin || (!documentLoader->request().isSameSite() && !fromBackForwardCache))
220220
return nullptr;
221221

222222
RefPtr<NavigationHistoryEntry> oldEntry;

‎Source/WebCore/page/Navigation.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ class Navigation final : public RefCounted<Navigation>, public EventTarget, publ
146146
void updateForReactivation(Vector<Ref<HistoryItem>>& newHistoryItems, HistoryItem& reactivatedItem);
147147
void updateForActivation(HistoryItem* previousItem, std::optional<NavigationNavigationType>);
148148

149-
RefPtr<NavigationActivation> createForPageswapEvent(HistoryItem* newItem, DocumentLoader*);
149+
RefPtr<NavigationActivation> createForPageswapEvent(HistoryItem* newItem, DocumentLoader*, bool fromBackForwardCache);
150150

151151
void abortOngoingNavigationIfNeeded();
152152

0 commit comments

Comments
 (0)