Move HistoryItem handling to DocumentLoader

third_party/WebKit/Source/web/WebLocalFrameImpl.cpp

 /*
  * Copyright (C) 2009 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 1660 matching lines @@
   // If we're moving in the back/forward list, we might want to replace the
   // content of this child frame with whatever was there at that point.
   HistoryItem* childItem = nullptr;
   if (isBackForwardLoadType(frame()->loader().documentLoader()->loadType()) &&
       !frame()->document()->loadEventFinished())
     childItem = webframeChild->client()->historyItemForNewChildFrame();
 
   FrameLoadRequest newRequest = request;
   FrameLoadType loadType = FrameLoadTypeStandard;
   if (childItem) {
-    newRequest = FrameLoadRequest(
-        request.originDocument(),
-        FrameLoader::resourceRequestFromHistoryItem(
-            childItem, WebCachePolicy::UseProtocolCachePolicy));
+    newRequest = FrameLoadRequest(request.originDocument(),
+                                  childItem->generateResourceRequest(
+                                      WebCachePolicy::UseProtocolCachePolicy));
     loadType = FrameLoadTypeInitialHistoryLoad;
   }
   webframeChild->frame()->loader().load(newRequest, loadType, childItem);
 
   // Note a synchronous navigation (about:blank) would have already processed
   // onload, so it is possible for the child frame to have already been
   // detached by script in the page.
   if (!webframeChild->parent())
     return nullptr;
   return webframeChild->frame();
@@ skipping 293 matching lines @@
   if (!frame())
     return true;
 
   return frame()->loader().shouldClose(isReload);
 }
 
 WebURLRequest WebLocalFrameImpl::requestFromHistoryItem(
     const WebHistoryItem& item,
     WebCachePolicy cachePolicy) const {
   HistoryItem* historyItem = item;
-  ResourceRequest request =
-      FrameLoader::resourceRequestFromHistoryItem(historyItem, cachePolicy);
-  return WrappedResourceRequest(request);
+  return WrappedResourceRequest(
+      historyItem->generateResourceRequest(cachePolicy));
 }
 
 WebURLRequest WebLocalFrameImpl::requestForReload(
     WebFrameLoadType loadType,
     const WebURL& overrideUrl) const {
   DCHECK(frame());
   ResourceRequest request = frame()->loader().resourceRequestForReload(
       static_cast<FrameLoadType>(loadType), overrideUrl);
   return WrappedResourceRequest(request);
 }
@@ skipping 34 matching lines @@
                                  bool isClientRedirect) {
   DCHECK(frame());
 
   // If we are loading substitute data to replace an existing load, then
   // inherit all of the properties of that original request. This way,
   // reload will re-attempt the original request. It is essential that
   // we only do this when there is an unreachableURL since a non-empty
   // unreachableURL informs FrameLoader::reload to load unreachableURL
   // instead of the currently loaded URL.
   ResourceRequest request;
-  if (replace && !unreachableURL.isEmpty() &&
-      frame()->loader().provisionalDocumentLoader())
-    request = frame()->loader().provisionalDocumentLoader()->originalRequest();
+  HistoryItem* historyItem = item;
+  DocumentLoader* provisionalDocumentLoader =
+      frame()->loader().provisionalDocumentLoader();
+  if (replace && !unreachableURL.isEmpty() && provisionalDocumentLoader) {
+    request = provisionalDocumentLoader->originalRequest();
+    if (provisionalDocumentLoader->loadType() == FrameLoadTypeBackForward) {
+      historyItem = provisionalDocumentLoader->historyItem();

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

Where is this line (i.e., L2059) from?

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

Yeah, I should have mentioned this, sorry. This line is why I felt I had to move
this logic out of determineFrameLoadType.

This block handles the case where a back/forward navigation fails and we want to
replace it with an error page. Previously, the history state portion of that
logic worked magically: FrameLoader::m_provisionalItem would be set by the
back/forward navgation, and it would *not* be cleared before the error page
navigation started. So the error page would just pick up the existing
m_provisionalItem and use it when it committed.

That doesn't work with this change, because the provisional HistoryItem lives on
the provisional DocumentLoader, and the error page navigation will replace the
provsional DocumentLoader. Therefore, we need to grab the provisional
DocumentLoader's HistoryItem before we create the error page's provisional
DocumentLoader. determineFrameLoadType() could grab it, but would need a special
case to return it to the caller. Since some of the error page replacement logic
was here already, I thought it best to move it all here, especially since we're
passing a HistoryItem here anyway.

[kinuko, 2017/03/30 05:05:00]

Could we note some of this as code comment, e.g. 'this is for error page
handling and we need the history item before provisional docloader is replaced'
?

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

Done.

+      webFrameLoadType = WebFrameLoadType::BackForward;
+    }
+  }
   request.setURL(baseURL);
   request.setCheckForBrowserSideNavigation(false);
 
   FrameLoadRequest frameRequest(
       0, request, SubstituteData(data, mimeType, textEncoding, unreachableURL));
   DCHECK(frameRequest.substituteData().isValid());
   frameRequest.setReplacesCurrentItem(replace);
   if (isClientRedirect)
     frameRequest.setClientRedirect(ClientRedirectPolicy::ClientRedirect);
 
-  HistoryItem* historyItem = item;
   frame()->loader().load(
       frameRequest, static_cast<FrameLoadType>(webFrameLoadType), historyItem,
       static_cast<HistoryLoadType>(webHistoryLoadType));
 }
 
 bool WebLocalFrameImpl::maybeRenderFallbackContent(
     const WebURLError& error) const {
   DCHECK(frame());
 
   if (!frame()->owner() || !frame()->owner()->canRenderFallbackContent())
     return false;
 
-  FrameLoader& frameloader = frame()->loader();
-  DCHECK(frameloader.provisionalDocumentLoader());
-  frameloader.loadFailed(frameloader.provisionalDocumentLoader(), error);
+  DCHECK(frame()->loader().provisionalDocumentLoader());
+  frame()->loader().provisionalDocumentLoader()->loadFailed(error);
   return true;
 }
 
 // Called when a navigation is blocked because a Content Security Policy (CSP)
 // is infringed.
 void WebLocalFrameImpl::reportContentSecurityPolicyViolation(
     const blink::WebContentSecurityPolicyViolation& violation) {
   DCHECK(frame() && frame()->document());
   Document* document = frame()->document();
   Vector<String> reportEndpoints;
@@ skipping 411 matching lines @@
         createMarkup(startPosition, endPosition, AnnotateForInterchange,
                      ConvertBlocksToInlines::NotConvert, ResolveNonLocalURLs);
   } else {
     clipHtml =
         createMarkup(endPosition, startPosition, AnnotateForInterchange,
                      ConvertBlocksToInlines::NotConvert, ResolveNonLocalURLs);
   }
 }
 
 }  // namespace blink