Skip to content

GitLab

T
tangled
Commit 87030295 authored by chaopeng's avatar chaopeng Committed by Commit Bot
Browse files
Options

Force layout for SameDocumentLoad when content size is not enough for restore. 

This issue is caused by in navigation backward (LoadInSameDocument) for Single
Page App, developer change DOM in onpopstate but not trigger a layout, then we
don't have correct content size for restore scroll position.

In this patch, we add a layout in RestoreScrollPositionAndViewState when the
size is not enough for restore scroll position. This change should only effect
SameDocumentLoad.

Bug: 721262
Change-Id: I2b2956828634884b926b4c5b3073210170f42263
Reviewed-on: https://chromium-review.googlesource.com/556979
Commit-Queue: Jianpeng Chao <chaopeng@chromium.org>
Reviewed-by: default avatarMajid Valipour <majidvp@chromium.org>
Reviewed-by: default avatarMike West <mkwst@chromium.org>
Cr-Commit-Position: refs/heads/master@{#484944}
parent f2255a2e
Hide whitespace changes
Inline Side-by-side
Showing with 69 additions and 12 deletions
third_party/WebKit/LayoutTests/fast/history/scroll-restoration/scroll-restoration-same-doc.html 0 → 100644
View file @ 87030295
<!DOCTYPE html>
<style>
#bigdiv {
width: 200px;
height: 1100px;
}
</style>
<div id="bigdiv">
</div>
<script src="../../../resources/testharness.js"></script>
<script src="../../../resources/testharnessreport.js"></script>
<script>
'use strict';
const bigdiv = document.getElementById("bigdiv");
async_test((t) => {
// 1. scroll to bottom.
window.scrollBy(0, 100);
assert_equals(window.scrollY, 100);
// 2. remove bigdiv and push state.
history.pushState(1, null, "page2.html");
document.body.removeChild(bigdiv);
assert_equals(window.scrollY, 0);
window.onpopstate = () => {
document.body.appendChild(bigdiv);
setTimeout(() => {
t.step(() => {
assert_equals(window.scrollY, 100);
});
t.done();
}, 0);
};
// 3. navigation back. page should restore scroll position to 100, see below.
history.back();
}, "load-in-same-doc-and-change-DOM-onpopstate");
</script>
third_party/WebKit/Source/core/loader/FrameLoader.cpp
View file @ 87030295
......@@ -595,8 +595,10 @@ void FrameLoader::LoadInSameDocument(
? std::move(state_object)
: SerializedScriptValue::NullValue());
if (history_item)
RestoreScrollPositionAndViewStateForLoadType(frame_load_type);
if (history_item) {
RestoreScrollPositionAndViewStateForLoadType(frame_load_type,
kHistorySameDocumentLoad);
}
// We need to scroll to the fragment whether or not a hash change occurred,
// since the user might have scrolled since the previous navigation.
......@@ -1093,11 +1095,13 @@ bool FrameLoader::IsLoadingMainFrame() const {
void FrameLoader::RestoreScrollPositionAndViewState() {
if (!frame_->GetPage() || !GetDocumentLoader())
return;
RestoreScrollPositionAndViewStateForLoadType(GetDocumentLoader()->LoadType());
RestoreScrollPositionAndViewStateForLoadType(GetDocumentLoader()->LoadType(),
kHistoryDifferentDocumentLoad);
}
void FrameLoader::RestoreScrollPositionAndViewStateForLoadType(
FrameLoadType load_type) {
FrameLoadType load_type,
HistoryLoadType history_load_type) {
LocalFrameView* view = frame_->View();
if (!view || !view->LayoutViewportScrollableArea() ||
!state_machine_.CommittedFirstRealDocumentLoad() ||
......@@ -1115,19 +1119,28 @@ void FrameLoader::RestoreScrollPositionAndViewStateForLoadType(
bool should_restore_scale = history_item->PageScaleFactor();
// This tries to balance:
// 1. restoring as soon as possible
// 2. not overriding user scroll (TODO(majidvp): also respect user scale)
// 1. restoring as soon as possible.
// 2. not overriding user scroll (TODO(majidvp): also respect user scale).
// 3. detecting clamping to avoid repeatedly popping the scroll position down
// as the page height increases
// 4. ignore clamp detection if we are not restoring scroll or after load
// completes because that may be because the page will never reach its
// previous height
// as the page height increases.
// 4. forcing a layout if necessary to avoid clamping.
// 5. ignoring clamp detection if scroll state is not being restored, if load
// is complete, or if the navigation is same-document (as the new page may
// be smaller than the previous page).
bool can_restore_without_clamping =
view->LayoutViewportScrollableArea()->ClampScrollOffset(
history_item->GetScrollOffset()) == history_item->GetScrollOffset();
bool should_force_clamping =
!frame_->IsLoading() || history_load_type == kHistorySameDocumentLoad;
// Here |can_restore_without_clamping| is false, but layout might be necessary
// to ensure correct content size.
if (!can_restore_without_clamping && should_force_clamping)
frame_->GetDocument()->UpdateStyleAndLayout();
bool can_restore_without_annoying_user =
!GetDocumentLoader()->GetInitialScrollState().was_scrolled_by_user &&
(can_restore_without_clamping || !frame_->IsLoading() ||
(can_restore_without_clamping || should_force_clamping ||
!should_restore_scroll);
if (!can_restore_without_annoying_user)
return;
......
third_party/WebKit/Source/core/loader/FrameLoader.h
View file @ 87030295
......@@ -245,7 +245,8 @@ class CORE_EXPORT FrameLoader final {
HistoryItem*,
ClientRedirectPolicy,
Document*);
void RestoreScrollPositionAndViewStateForLoadType(FrameLoadType);
void RestoreScrollPositionAndViewStateForLoadType(FrameLoadType,
HistoryLoadType);
void ScheduleCheckCompleted();
......
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