Move HistoryItem handling to DocumentLoader

third_party/WebKit/Source/core/loader/FrameLoader.cpp

Index: third_party/WebKit/Source/core/loader/FrameLoader.cpp
diff --git a/third_party/WebKit/Source/core/loader/FrameLoader.cpp b/third_party/WebKit/Source/core/loader/FrameLoader.cpp
index f108a6c88c12da44aaeccb5f074de389cf29677e..ae0d76954490da9cf1ab9b3183a2ef73191d0451 100644
--- a/third_party/WebKit/Source/core/loader/FrameLoader.cpp
+++ b/third_party/WebKit/Source/core/loader/FrameLoader.cpp
@@ -140,23 +140,6 @@ static void checkForLegacyProtocolInSubresource(
       document, UseCounter::LegacyProtocolEmbeddedAsSubresource);
 }
 
-// static
-ResourceRequest FrameLoader::resourceRequestFromHistoryItem(

[Nate Chapin, 2017/03/27 21:02:01]

Moved to HistoryItem, since it actually depends on HistoryItem.

-    HistoryItem* item,
-    WebCachePolicy cachePolicy) {
-  RefPtr<EncodedFormData> formData = item->formData();
-  ResourceRequest request(item->url());
-  request.setHTTPReferrer(item->referrer());
-  request.setCachePolicy(cachePolicy);
-  if (formData) {
-    request.setHTTPMethod(HTTPNames::POST);
-    request.setHTTPBody(formData);
-    request.setHTTPContentType(item->formContentType());
-    request.addHTTPOriginIfNeeded(item->referrer().referrer);
-  }
-  return request;
-}
-
 ResourceRequest FrameLoader::resourceRequestForReload(
     FrameLoadType frameLoadType,
     const KURL& overrideURL,
@@ -166,10 +149,10 @@ ResourceRequest FrameLoader::resourceRequestForReload(
       frameLoadType == FrameLoadTypeReloadBypassingCache
           ? WebCachePolicy::BypassingCache
           : WebCachePolicy::ValidatingCacheData;
-  if (!m_currentItem)
+  if (!m_documentLoader || !m_documentLoader->historyItem())
     return ResourceRequest();
   ResourceRequest request =
-      resourceRequestFromHistoryItem(m_currentItem.get(), cachePolicy);
+      m_documentLoader->historyItem()->generateResourceRequest(cachePolicy);
 
   // ClientRedirectPolicy is an indication that this load was triggered by some
   // direct interaction with the page. If this reload is not a client redirect,
@@ -219,8 +202,6 @@ DEFINE_TRACE(FrameLoader) {
   visitor->trace(m_progressTracker);
   visitor->trace(m_documentLoader);
   visitor->trace(m_provisionalDocumentLoader);
-  visitor->trace(m_currentItem);
-  visitor->trace(m_provisionalItem);
   visitor->trace(m_deferredHistoryLoad);
 }
 
@@ -279,23 +260,23 @@ void FrameLoader::setDefersLoading(bool defers) {
 }
 
 void FrameLoader::saveScrollState() {
-  if (!m_currentItem || !m_frame->view())
+  if (!m_documentLoader || !m_documentLoader->historyItem() || !m_frame->view())
     return;
 
   // Shouldn't clobber anything if we might still restore later.
-  if (m_documentLoader &&
-      needsHistoryItemRestore(m_documentLoader->loadType()) &&
+  if (needsHistoryItemRestore(m_documentLoader->loadType()) &&
       !m_documentLoader->initialScrollState().wasScrolledByUser)
     return;
 
+  HistoryItem* historyItem = m_documentLoader->historyItem();
   if (ScrollableArea* layoutScrollableArea =
           m_frame->view()->layoutViewportScrollableArea())
-    m_currentItem->setScrollOffset(layoutScrollableArea->getScrollOffset());
-  m_currentItem->setVisualViewportScrollOffset(toScrollOffset(
+    historyItem->setScrollOffset(layoutScrollableArea->getScrollOffset());
+  historyItem->setVisualViewportScrollOffset(toScrollOffset(
       m_frame->page()->visualViewport().visibleRect().location()));
 
   if (m_frame->isMainFrame())
-    m_currentItem->setPageScaleFactor(m_frame->page()->pageScaleFactor());
+    historyItem->setPageScaleFactor(m_frame->page()->pageScaleFactor());
 
   client()->didUpdateCurrentHistoryItem();
 }
@@ -398,111 +379,6 @@ void FrameLoader::replaceDocumentWhileExecutingJavaScriptURL(
   documentLoader->replaceDocumentWhileExecutingJavaScriptURL(init, source);
 }
 
-void FrameLoader::clearProvisionalHistoryItem() {
-  m_provisionalItem.clear();
-}
-
-void FrameLoader::setHistoryItemStateForCommit(
-    FrameLoadType loadType,
-    HistoryCommitType historyCommitType,
-    HistoryNavigationType navigationType) {
-  HistoryItem* oldItem = m_currentItem;
-  if (isBackForwardLoadType(loadType) && m_provisionalItem)
-    m_currentItem = m_provisionalItem.release();
-  else
-    m_currentItem = HistoryItem::create();
-  m_currentItem->setURL(m_documentLoader->urlForHistory());
-  m_currentItem->setDocumentState(m_frame->document()->formElementsState());
-  m_currentItem->setReferrer(SecurityPolicy::generateReferrer(
-      m_documentLoader->getRequest().getReferrerPolicy(), m_currentItem->url(),
-      m_documentLoader->getRequest().httpReferrer()));
-  m_currentItem->setFormInfoFromRequest(m_documentLoader->getRequest());
-
-  // Don't propagate state from the old item to the new item if there isn't an
-  // old item (obviously), or if this is a back/forward navigation, since we
-  // explicitly want to restore the state we just committed.
-  if (!oldItem || isBackForwardLoadType(loadType))
-    return;
-  // Don't propagate state from the old item if this is a different-document
-  // navigation, unless the before and after pages are logically related. This
-  // means they have the same url (ignoring fragment) and the new item was
-  // loaded via reload or client redirect.
-  if (navigationType == HistoryNavigationType::DifferentDocument &&
-      (historyCommitType != HistoryInertCommit ||
-       !equalIgnoringFragmentIdentifier(oldItem->url(), m_currentItem->url())))
-    return;
-  m_currentItem->setDocumentSequenceNumber(oldItem->documentSequenceNumber());
-  m_currentItem->setScrollOffset(oldItem->getScrollOffset());
-  m_currentItem->setDidSaveScrollOrScaleState(
-      oldItem->didSaveScrollOrScaleState());
-  m_currentItem->setVisualViewportScrollOffset(
-      oldItem->visualViewportScrollOffset());
-  m_currentItem->setPageScaleFactor(oldItem->pageScaleFactor());
-  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
-  // history.replaceState().
-  if (historyCommitType == HistoryInertCommit &&
-      (navigationType == HistoryNavigationType::HistoryApi ||
-       oldItem->url() == m_currentItem->url())) {
-    m_currentItem->setStateObject(oldItem->stateObject());
-    m_currentItem->setItemSequenceNumber(oldItem->itemSequenceNumber());
-  }
-}
-
-static HistoryCommitType loadTypeToCommitType(FrameLoadType type) {
-  switch (type) {
-    case FrameLoadTypeStandard:
-      return StandardCommit;
-    case FrameLoadTypeInitialInChildFrame:
-    case FrameLoadTypeInitialHistoryLoad:
-      return InitialCommitInChildFrame;
-    case FrameLoadTypeBackForward:
-      return BackForwardCommit;
-    default:
-      break;
-  }
-  return HistoryInertCommit;
-}
-
-void FrameLoader::receivedFirstData() {
-  FrameLoadType loadType = m_documentLoader->loadType();
-  HistoryCommitType historyCommitType = loadTypeToCommitType(loadType);
-  if (historyCommitType == StandardCommit &&
-      (m_documentLoader->urlForHistory().isEmpty() ||
-       (opener() && !m_currentItem &&
-        m_documentLoader->originalRequest().url().isEmpty())))
-    historyCommitType = HistoryInertCommit;
-  setHistoryItemStateForCommit(loadType, historyCommitType,
-                               HistoryNavigationType::DifferentDocument);
-
-  if (!m_stateMachine.committedMultipleRealLoads() &&
-      loadType == FrameLoadTypeStandard) {
-    m_stateMachine.advanceTo(
-        FrameLoaderStateMachine::CommittedMultipleRealLoads);
-  }
-
-  client()->dispatchDidCommitLoad(m_currentItem.get(), historyCommitType);
-
-  // When the embedder gets notified (above) that the new navigation has
-  // committed, the embedder will drop the old Content Security Policy and
-  // therefore now is a good time to report to the embedder the Content Security
-  // Policies that have accumulated so far for the new navigation.
-  m_frame->securityContext()->contentSecurityPolicy()->reportAccumulatedHeaders(
-      client());
-}
-
-void FrameLoader::didInstallNewDocument() {
-  if (m_provisionalItem &&
-      isBackForwardLoadType(m_documentLoader->loadType())) {
-    m_frame->document()->setStateForNewFormElements(
-        m_provisionalItem->getDocumentState());
-  }
-}
-
 void FrameLoader::finishedParsing() {
   if (m_stateMachine.creatingInitialEmptyDocument())
     return;
@@ -697,12 +573,6 @@ void FrameLoader::updateForSameDocumentNavigation(
     Document* initiatingDocument) {
   TRACE_EVENT1("blink", "FrameLoader::updateForSameDocumentNavigation", "url",
                newURL.getString().ascii().data());
-  // Update the data source's request with the new URL to fake the URL change
-  m_frame->document()->setURL(newURL);
-  documentLoader()->setReplacesCurrentHistoryItem(type !=
-                                                  FrameLoadTypeStandard);
-  documentLoader()->updateForSameDocumentNavigation(
-      newURL, sameDocumentNavigationSource);
 
   // Generate start and stop notifications only when loader is completed so that
   // we don't fire them for fragment redirection that happens in window.onload
@@ -712,26 +582,12 @@ void FrameLoader::updateForSameDocumentNavigation(
   if (m_frame->document()->loadEventFinished() && !m_provisionalDocumentLoader)
     client()->didStartLoading(NavigationWithinSameDocument);
 
-  HistoryCommitType historyCommitType = loadTypeToCommitType(type);
-  if (!m_currentItem)

[Nate Chapin, 2017/03/27 21:02:01]

I couldn't find any evidence this clause is needed. Everything else in this
function moved to DocumentLoader::updateForSameDocumentNavigation.

-    historyCommitType = HistoryInertCommit;
-  if (m_frame->settings()->getHistoryEntryRequiresUserGesture() &&
-      initiatingDocument &&
-      !initiatingDocument->frame()->hasReceivedUserGesture()) {
-    historyCommitType = HistoryInertCommit;
-  }
+  // Update the data source's request with the new URL to fake the URL change
+  m_frame->document()->setURL(newURL);
+  documentLoader()->updateForSameDocumentNavigation(
+      newURL, sameDocumentNavigationSource, std::move(data),
+      scrollRestorationType, type, initiatingDocument);
 
-  setHistoryItemStateForCommit(
-      type, historyCommitType,
-      sameDocumentNavigationSource == SameDocumentNavigationHistoryApi
-          ? HistoryNavigationType::HistoryApi
-          : HistoryNavigationType::Fragment);
-  if (sameDocumentNavigationSource == SameDocumentNavigationHistoryApi) {
-    m_currentItem->setStateObject(std::move(data));
-    m_currentItem->setScrollRestorationType(scrollRestorationType);
-  }
-  client()->dispatchDidNavigateWithinPage(
-      m_currentItem.get(), historyCommitType, !!initiatingDocument);
   client()->dispatchDidReceiveTitle(m_frame->document()->title());
   if (m_frame->document()->loadEventFinished() && !m_provisionalDocumentLoader)
     client()->didStopLoading();
@@ -750,7 +606,7 @@ void FrameLoader::loadInSameDocument(
     const KURL& url,
     PassRefPtr<SerializedScriptValue> stateObject,
     FrameLoadType frameLoadType,
-    HistoryLoadType historyLoadType,
+    HistoryItem* historyItem,
     ClientRedirectPolicy clientRedirect,
     Document* initiatingDocument) {
   // If we have a state object, we cannot also be a new navigation.
@@ -775,6 +631,8 @@ void FrameLoader::loadInSameDocument(
   }
   m_documentLoader->setIsClientRedirect(clientRedirect ==
                                         ClientRedirectPolicy::ClientRedirect);
+  if (historyItem)
+    m_documentLoader->setItemForHistoryNavigation(historyItem);
   updateForSameDocumentNavigation(url, SameDocumentNavigationDefault, nullptr,
                                   ScrollRestorationAuto, frameLoadType,
                                   initiatingDocument);
@@ -787,7 +645,7 @@ void FrameLoader::loadInSameDocument(
                                         ? std::move(stateObject)
                                         : SerializedScriptValue::nullValue());
 
-  if (historyLoadType == HistorySameDocumentLoad)
+  if (historyItem)
     restoreScrollPositionAndViewStateForLoadType(frameLoadType);
 
   // We need to scroll to the fragment whether or not a hash change occurred,
@@ -826,13 +684,11 @@ FrameLoadType FrameLoader::determineFrameLoadType(
   if (m_frame->tree().parent() &&
       !m_stateMachine.committedFirstRealDocumentLoad())
     return FrameLoadTypeInitialInChildFrame;
-  if (!m_frame->tree().parent() && !client()->backForwardLength())
+  if (!m_frame->tree().parent() && !client()->backForwardLength()) {
+    if (opener() && request.resourceRequest().url().isEmpty())

[Nate Chapin, 2017/03/27 21:02:01]

Was a special case in FrameLoader::receivedFirstData() to override the history
commit type.

+      return FrameLoadTypeReplaceCurrentItem;
     return FrameLoadTypeStandard;
-  if (m_provisionalDocumentLoader &&
-      request.substituteData().failingURL() ==
-          m_provisionalDocumentLoader->url() &&
-      m_provisionalDocumentLoader->loadType() == FrameLoadTypeBackForward)

[Nate Chapin, 2017/03/27 21:02:01]

This clause moved to WebLocalFrameImpl::loadData(), its only user, and where it
can easily get hold of the HistoryItem needed for the back/forward navigation.

-    return FrameLoadTypeBackForward;
+  }
   if (request.resourceRequest().getCachePolicy() ==
       WebCachePolicy::ValidatingCacheData)
     return FrameLoadTypeReload;
@@ -866,6 +722,11 @@ FrameLoadType FrameLoader::determineFrameLoadType(
       !request.originDocument()->frame()->hasReceivedUserGesture())
     return FrameLoadTypeReplaceCurrentItem;
 
+  if (request.resourceRequest().url().isEmpty() &&
+      request.substituteData().failingURL().isEmpty()) {

[Nate Chapin, 2017/03/27 21:02:01]

Was a special case in FrameLoader::receivedFirstData() to override the history
commit type.

+    return FrameLoadTypeReplaceCurrentItem;
+  }
+
   return FrameLoadTypeStandard;
 }
 
@@ -1016,11 +877,6 @@ void FrameLoader::load(const FrameLoadRequest& passedRequest,
   if (!prepareRequestForThisFrame(request))
     return;
 
-  if (isBackForwardLoadType(frameLoadType)) {

[Nate Chapin, 2017/03/27 21:02:01]

Equivalent moved to startLoad(), after the provisional DocumentLoader is
created.

-    DCHECK(historyItem);
-    m_provisionalItem = historyItem;
-  }
-
   // Form submissions appear to need their special-case of finding the target at
   // schedule rather than at fire.
   Frame* targetFrame = request.form()
@@ -1088,7 +944,7 @@ void FrameLoader::load(const FrameLoadRequest& passedRequest,
         newLoadType = FrameLoadTypeReplaceCurrentItem;
     }
 
-    loadInSameDocument(url, stateObject, newLoadType, historyLoadType,
+    loadInSameDocument(url, stateObject, newLoadType, historyItem,
                        request.clientRedirect(), request.originDocument());
     return;
   }
@@ -1101,7 +957,7 @@ void FrameLoader::load(const FrameLoadRequest& passedRequest,
   if (request.resourceRequest().isSameDocumentNavigation())
     return;
 
-  startLoad(request, newLoadType, policy);
+  startLoad(request, newLoadType, policy, historyItem);
 }
 
 SubstituteData FrameLoader::defaultSubstituteDataForURL(const KURL& url) {
@@ -1158,8 +1014,8 @@ void FrameLoader::stopAllLoaders() {
   // committing an empty document. In that case, emulate a failed navigation.
   if (!m_provisionalDocumentLoader && m_documentLoader &&
       m_frame->isLoading()) {
-    loadFailed(m_documentLoader.get(),
-               ResourceError::cancelledError(m_documentLoader->url()));
+    m_documentLoader->loadFailed(
+        ResourceError::cancelledError(m_documentLoader->url()));
   }
 
   m_inStopAllLoaders = false;
@@ -1229,6 +1085,8 @@ bool FrameLoader::prepareForCommit() {
   if (m_frame->document())
     m_frame->document()->shutdown();
   m_documentLoader = m_provisionalDocumentLoader.release();
+  if (m_documentLoader)
+    m_documentLoader->markAsCommitted();

[Nate Chapin, 2017/03/27 21:02:01]

(1) m_documentLoader can be nullptr here, which is bizarre.
(2) Due to ordering issues, there isn't a good time to mark the DocumentLoader
as committed from DocumentLoader. However, this sets the bit in the next best
place: exactly when the committing DocumentLoader transitions from
m_provisionalDocumentLoader to m_documentLoader.

   takeObjectSnapshot();
 
   return true;
@@ -1294,18 +1152,19 @@ void FrameLoader::restoreScrollPositionAndViewState() {
 void FrameLoader::restoreScrollPositionAndViewStateForLoadType(
     FrameLoadType loadType) {
   FrameView* view = m_frame->view();
-  if (!view || !view->layoutViewportScrollableArea() || !m_currentItem ||
+  if (!view || !view->layoutViewportScrollableArea() ||
       !m_stateMachine.committedFirstRealDocumentLoad()) {
     return;
   }
   if (!needsHistoryItemRestore(loadType))
     return;
-  if (!m_currentItem->didSaveScrollOrScaleState())
+  HistoryItem* historyItem = m_documentLoader->historyItem();
+  if (!historyItem || !historyItem->didSaveScrollOrScaleState())
     return;
 
   bool shouldRestoreScroll =
-      m_currentItem->scrollRestorationType() != ScrollRestorationManual;
-  bool shouldRestoreScale = m_currentItem->pageScaleFactor();
+      historyItem->scrollRestorationType() != ScrollRestorationManual;
+  bool shouldRestoreScale = historyItem->pageScaleFactor();
 
   // This tries to balance:
   // 1. restoring as soon as possible
@@ -1317,7 +1176,7 @@ void FrameLoader::restoreScrollPositionAndViewStateForLoadType(
   //    previous height
   bool canRestoreWithoutClamping =
       view->layoutViewportScrollableArea()->clampScrollOffset(
-          m_currentItem->getScrollOffset()) == m_currentItem->getScrollOffset();
+          historyItem->getScrollOffset()) == historyItem->getScrollOffset();
   bool canRestoreWithoutAnnoyingUser =
       !documentLoader()->initialScrollState().wasScrolledByUser &&
       (canRestoreWithoutClamping || !m_frame->isLoading() ||
@@ -1327,13 +1186,13 @@ void FrameLoader::restoreScrollPositionAndViewStateForLoadType(
 
   if (shouldRestoreScroll) {
     view->layoutViewportScrollableArea()->setScrollOffset(
-        m_currentItem->getScrollOffset(), ProgrammaticScroll);
+        historyItem->getScrollOffset(), ProgrammaticScroll);
   }
 
   // For main frame restore scale and visual viewport position
   if (m_frame->isMainFrame()) {
     ScrollOffset visualViewportOffset(
-        m_currentItem->visualViewportScrollOffset());
+        historyItem->visualViewportScrollOffset());
 
     // If the visual viewport's offset is (-1, -1) it means the history item
     // is an old version of HistoryItem so distribute the scroll between
@@ -1341,16 +1200,16 @@ void FrameLoader::restoreScrollPositionAndViewStateForLoadType(
     if (visualViewportOffset.width() == -1 &&
         visualViewportOffset.height() == -1) {
       visualViewportOffset =
-          m_currentItem->getScrollOffset() -
+          historyItem->getScrollOffset() -
           view->layoutViewportScrollableArea()->getScrollOffset();
     }
 
     VisualViewport& visualViewport = m_frame->page()->visualViewport();
     if (shouldRestoreScale && shouldRestoreScroll) {
-      visualViewport.setScaleAndLocation(m_currentItem->pageScaleFactor(),
+      visualViewport.setScaleAndLocation(historyItem->pageScaleFactor(),
                                          FloatPoint(visualViewportOffset));
     } else if (shouldRestoreScale) {
-      visualViewport.setScale(m_currentItem->pageScaleFactor());
+      visualViewport.setScale(historyItem->pageScaleFactor());
     } else if (shouldRestoreScroll) {
       visualViewport.setLocation(FloatPoint(visualViewportOffset));
     }
@@ -1384,34 +1243,9 @@ void FrameLoader::detach() {
   TRACE_EVENT_OBJECT_DELETED_WITH_ID("loading", "FrameLoader", this);
 }
 
-void FrameLoader::loadFailed(DocumentLoader* loader,
-                             const ResourceError& error) {
-  if (!error.isCancellation() && m_frame->owner()) {
-    // FIXME: For now, fallback content doesn't work cross process.
-    if (m_frame->owner()->isLocal())
-      m_frame->deprecatedLocalOwner()->renderFallbackContent();
-  }
-
-  HistoryCommitType historyCommitType =
-      loadTypeToCommitType(loader->loadType());
-  if (loader == m_provisionalDocumentLoader) {
-    if (!m_provisionalDocumentLoader->didStart())
-      probe::frameClearedScheduledClientNavigation(m_frame);
-    m_provisionalDocumentLoader->setSentDidFinishLoad();
-    client()->dispatchDidFailProvisionalLoad(error, historyCommitType);
-    if (loader != m_provisionalDocumentLoader)
-      return;
-    detachDocumentLoader(m_provisionalDocumentLoader);
-  } else {
-    DCHECK_EQ(loader, m_documentLoader);
-    if (m_frame->document()->parser())
-      m_frame->document()->parser()->stopParsing();
-    if (!m_documentLoader->sentDidFinishLoad()) {
-      m_documentLoader->setSentDidFinishLoad();
-      client()->dispatchDidFailLoad(error, historyCommitType);
-    }
-  }
-  checkCompleted();
+void FrameLoader::detachProvisionalDocumentLoader(DocumentLoader* loader) {

[Nate Chapin, 2017/03/27 21:02:01]

I don't like having this, but a DocumentLoader needs a way to self-detach from
loadFailed(), and this is the only interaction it needs with FrameLoader
(besides the very general checkCompleted() call).

+  DCHECK_EQ(loader, m_provisionalDocumentLoader);
+  detachDocumentLoader(m_provisionalDocumentLoader);
 }
 
 bool FrameLoader::shouldPerformFragmentNavigation(bool isFormSubmission,
@@ -1457,10 +1291,10 @@ void FrameLoader::processFragment(const KURL& url,
   bool shouldScrollToFragment =
       (loadStartType == NavigationWithinSameDocument &&
        !isBackForwardLoadType(frameLoadType)) ||
-      (documentLoader() &&

[yhirano, 2017/03/29 09:38:13]

Can you tell me why this can be removed safely?

[Nate Chapin, 2017/03/29 18:23:06]

The FrameView null-check at the top of processFragment should be triggered if
documentLoader() is null, and neither of the calls above should be able to
trigger detach.

-       !documentLoader()->initialScrollState().didRestoreFromHistory &&
-       !(m_currentItem &&
-         m_currentItem->scrollRestorationType() == ScrollRestorationManual));
+      (!documentLoader()->initialScrollState().didRestoreFromHistory &&
+       !(documentLoader()->historyItem() &&
+         documentLoader()->historyItem()->scrollRestorationType() ==
+             ScrollRestorationManual));
 
   view->processUrlFragment(url, shouldScrollToFragment
                                     ? FrameView::UrlFragmentScroll
@@ -1616,7 +1450,8 @@ NavigationPolicy FrameLoader::checkLoadCanStart(
 
 void FrameLoader::startLoad(FrameLoadRequest& frameLoadRequest,
                             FrameLoadType type,
-                            NavigationPolicy navigationPolicy) {
+                            NavigationPolicy navigationPolicy,
+                            HistoryItem* historyItem) {
   DCHECK(client()->hasWebView());
   ResourceRequest& resourceRequest = frameLoadRequest.resourceRequest();
   NavigationType navigationType = determineNavigationType(
@@ -1677,6 +1512,12 @@ void FrameLoader::startLoad(FrameLoadRequest& frameLoadRequest,
 
   m_provisionalDocumentLoader->appendRedirect(
       m_provisionalDocumentLoader->getRequest().url());
+
+  if (isBackForwardLoadType(type)) {
+    DCHECK(historyItem);
+    m_provisionalDocumentLoader->setItemForHistoryNavigation(historyItem);
+  }
+
   // TODO(ananta):
   // We should get rid of the dependency on the DocumentLoader in consumers of
   // the didStartProvisionalLoad() notification.
@@ -1710,7 +1551,8 @@ void FrameLoader::applyUserAgent(ResourceRequest& request) {
 }
 
 bool FrameLoader::shouldTreatURLAsSameAsCurrent(const KURL& url) const {
-  return m_currentItem && url == m_currentItem->url();
+  return m_documentLoader->historyItem() &&
+         url == m_documentLoader->historyItem()->url();
 }
 
 bool FrameLoader::shouldTreatURLAsSrcdocDocument(const KURL& url) const {
@@ -1903,8 +1745,9 @@ DocumentLoader* FrameLoader::createDocumentLoader(
 
   loader->setLoadType(loadType);
   loader->setNavigationType(navigationType);
-  loader->setReplacesCurrentHistoryItem(loadType ==
-                                        FrameLoadTypeReplaceCurrentItem);
+  bool replaceCurrentItem = loadType == FrameLoadTypeReplaceCurrentItem &&

[Nate Chapin, 2017/03/27 21:02:01]

The browser process DCHECKS when it received a blank navigation in a brand new
window.open()ed window, if the navigation is marked as replacing it's current
entry. This hack gives the browser process what it expects, but I don't
understand the relevant change (https://codereview.chromium.org/1272453002) well
enough to be sure whether that invariant can be safely reworked or removed.

[kinuko, 2017/03/30 05:04:59]

Could we have some of this note as a code comment?  It'd be hard to reason about
otherwise.

[Nate Chapin, 2017/03/30 20:13:56]

Added an explanation/TODO.

+                            (!opener() || !request.url().isEmpty());
+  loader->setReplacesCurrentHistoryItem(replaceCurrentItem);
   return loader;
 }