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

Unified Diff: Source/core/loader/FrameLoader.cpp

Issue 1239463005: Replace history.options with history.scrollRestoration attribute (Closed) Base URL: https://chromium.googlesource.com/chromium/blink.git@master
Patch Set: Move setScrollRestorationType to History Created 5 years, 5 months 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
« no previous file with comments | « Source/core/frame/StateOptions.idl ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: Source/core/loader/FrameLoader.cpp
diff --git a/Source/core/loader/FrameLoader.cpp b/Source/core/loader/FrameLoader.cpp
index 3d8ab8b84bc46b42a5857b61572df491501f33b4..f5d1f2473b32f5d7de3eac38178d8270b85757ea 100644
--- a/Source/core/loader/FrameLoader.cpp
+++ b/Source/core/loader/FrameLoader.cpp
@@ -377,6 +377,8 @@ void FrameLoader::setHistoryItemStateForCommit(HistoryCommitType historyCommitTy
m_currentItem->setPinchViewportScrollPoint(oldItem->pinchViewportScrollPoint());
m_currentItem->setPageScaleFactor(oldItem->pageScaleFactor());
m_currentItem->setStateObject(oldItem->stateObject());
+ m_currentItem->setScrollRestorationType(oldItem->scrollRestorationType());
+
// The item sequence number determines whether items are "the same", such back/forward navigation
// between items with the same item sequence number is a no-op. Only treat this as identical if the
// navigation did not create a back/forward entry and the url is identical or it was loaded via
@@ -571,6 +573,8 @@ void FrameLoader::checkCompleted()
m_progressTracker->progressCompleted();
// Retry restoring scroll offset since finishing loading disables content
// size clamping.
+ // TODO(majidvp): Remove this call as it appears to be a no-op because
+ // we set load type to |FrameLoadTypeStandard| just above.
restoreScrollPositionAndViewState();
m_frame->localDOMWindow()->finishedLoading();
@@ -690,9 +694,6 @@ void FrameLoader::loadInSameDocument(const KURL& url, PassRefPtr<SerializedScrip
m_frame->view()->setWasScrolledByUser(false);
- // We need to scroll to the fragment whether or not a hash change occurred, since
- // the user might have scrolled since the previous navigation.
- processFragment(url, NavigationWithinSameDocument);
checkCompleted();
m_frame->localDOMWindow()->statePopped(stateObject ? stateObject : SerializedScriptValue::nullValue());
@@ -914,6 +915,10 @@ void FrameLoader::load(const FrameLoadRequest& passedRequest, FrameLoadType fram
if (sameDocumentHistoryNavigation)
restoreScrollPositionAndViewState();
+
+ // We need to scroll to the fragment whether or not a hash change occurred, since
+ // the user might have scrolled since the previous navigation.
+ processFragment(url, NavigationWithinSameDocument);
return;
}
@@ -1108,8 +1113,10 @@ void FrameLoader::restoreScrollPositionAndViewState()
if (!needsHistoryItemRestore(m_loadType))
return;
- if (m_currentItem->scrollRestorationType() == ScrollRestorationManual)
+ if (m_currentItem->scrollRestorationType() == ScrollRestorationManual) {
+ documentLoader()->initialScrollState().didRestoreFromHistory = true;
return;
+ }
// This tries to balance 1. restoring as soon as possible, 2. detecting
// clamping to avoid repeatedly popping the scroll position down as the
@@ -1236,9 +1243,13 @@ void FrameLoader::processFragment(const KURL& url, LoadStartType loadStartType)
if (boundaryFrame && boundaryFrame->isLocalFrame())
toLocalFrame(boundaryFrame.get())->view()->setSafeToPropagateScrollToParent(false);
- view->processUrlFragment(url,
- loadStartType == NavigationToDifferentDocument && documentLoader()->initialScrollState().didRestoreFromHistory ?
- FrameView::UrlFragmentDontScroll : FrameView::UrlFragmentScroll);
+ // If scroll position is restored from history fragment then we should not override it unless
+ // this is a same document reload.
+ bool shouldScrollToFragment = (loadStartType == NavigationWithinSameDocument && !isBackForwardLoadType(m_loadType))
+ || !documentLoader()->initialScrollState().didRestoreFromHistory;
+
+ view->processUrlFragment(url, shouldScrollToFragment ?
+ FrameView::UrlFragmentScroll : FrameView::UrlFragmentDontScroll);
if (boundaryFrame && boundaryFrame->isLocalFrame())
toLocalFrame(boundaryFrame.get())->view()->setSafeToPropagateScrollToParent(true);
« no previous file with comments | « Source/core/frame/StateOptions.idl ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698