Use WebFrame::loadRequest for reloads and history navigations

content/renderer/render_frame_impl.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/renderer/render_frame_impl.h"
 
 #include <map>
 #include <string>
 
 #include "base/auto_reset.h"
@@ skipping 4397 matching lines @@
   if (is_reload && !render_view_->history_controller()->GetCurrentEntry()) {
     // We cannot reload if we do not have any history state.  This happens, for
     // example, when recovering from a crash.
     is_reload = false;
     cache_policy = WebURLRequest::ReloadIgnoringCacheData;
   }
 
   pending_navigation_params_.reset(
       new NavigationParams(common_params, start_params, request_params));
 
-  // If we are reloading, then Blink will use the history state of the current
-  // page, so we should just ignore any given history state.  Otherwise, if we
-  // have history state, then we need to navigate to it, which corresponds to a
-  // back/forward navigation event.
-  if (is_reload && !browser_side_navigation) {
-    // TODO(clamy): adapt this code for PlzNavigate. In particular the stream
-    // override should be given to the generated request.
-    bool reload_original_url =
-        (common_params.navigation_type ==
-         FrameMsg_Navigate_Type::RELOAD_ORIGINAL_REQUEST_URL);
+  // Create parameters for a standard navigation.
+  WebFrame::WebFrameLoadType load_type = WebFrame::WebFrameLoadTypeStandard;
+  WebHistoryItem item_for_history_navigation;
+  WebURLRequest request = CreateURLRequestForNavigation(
+      common_params, stream_params.Pass(), frame_->isViewSourceModeEnabled());
+
+  // PlzNavigate: Make sure that Blink's loader will not try to use browser side
+  // navigation for this request (since it already went to the browser).
+  if (browser_side_navigation)
+    request.setCheckForBrowserSideNavigation(false);
+
+  // If we are reloading, then use the history state of the current frame.
+  // Otherwise, if we have history state, then we need to navigate to it, which
+  // corresponds to a back/forward navigation event. Update the parameters
+  // depending on the navigation type.
+  if (is_reload) {
     bool ignore_cache = (common_params.navigation_type ==
                          FrameMsg_Navigate_Type::RELOAD_IGNORING_CACHE);
+    load_type = ignore_cache ? WebFrame::WebFrameLoadTypeReloadFromOrigin
+                             : WebFrame::WebFrameLoadTypeReload;
 
-    if (reload_original_url)
-      frame_->reloadWithOverrideURL(common_params.url, true);
-    else
-      frame_->reload(ignore_cache);
-  } else if (is_history_navigation && !browser_side_navigation) {
-    // TODO(clamy): adapt this code for PlzNavigate. In particular the stream
-    // override should be given to the generated request.
-
+    if (!browser_side_navigation) {
+      WebHistoryItem current_item = frame_->CurrentItem();
+      if (current_item.isNull()) {
+        Send(new FrameHostMsg_DidDropNavigation(routing_id_));

[Charlie Reis, 2015/05/27 23:34:36]

I don't understand this case.  When would this happen?

[clamy, 2015/05/29 14:47:01]

Following interface changes in the blink patch this is no longer needed.

+        pending_navigation_params_.reset();
+        return;
+      }
+      const GURL override_url =
+          (common_params.navigation_type ==
+           FrameMsg_Navigate_Type::RELOAD_ORIGINAL_REQUEST_URL)
+              ? common_params.url
+              : GURL();
+      request = frame_->RequestForReload(current_item, load_type, override_url);
+    }
+  } else if (is_history_navigation) {
     // We must know the page ID of the page we are navigating back to.
     DCHECK_NE(request_params.page_id, -1);
     // We must know the nav entry ID of the page we are navigating back to,
     // which should be the case because history navigations are routed via the
     // browser.
     DCHECK_NE(0, request_params.nav_entry_id);
     scoped_ptr<HistoryEntry> entry =
         PageStateToHistoryEntry(request_params.page_state);
-    if (entry) {
-      // Ensure we didn't save the swapped out URL in UpdateState, since the
-      // browser should never be telling us to navigate to swappedout://.
-      CHECK(entry->root().urlString() != WebString::fromUTF8(kSwappedOutURL));
-      scoped_ptr<NavigationParams> navigation_params(
-          new NavigationParams(*pending_navigation_params_.get()));
+    if (!entry) {
+      Send(new FrameHostMsg_DidDropNavigation(routing_id_));

[Charlie Reis, 2015/05/27 23:34:36]

It feels awkward to have this in multiple places, especially when we didn't need
it before.

[clamy, 2015/05/29 14:47:01]

I can remove it. It just seemed weird to me that the renderer did not inform the
browser it did not proceed with the navigation it was asked to do.

+      pending_navigation_params_.reset();
+      return;
+    }
+
+    // Ensure we didn't save the swapped out URL in UpdateState, since the
+    // browser should never be telling us to navigate to swappedout://.
+    CHECK(entry->root().urlString() != WebString::fromUTF8(kSwappedOutURL));
+    scoped_ptr<NavigationParams> navigation_params(
+        new NavigationParams(*pending_navigation_params_.get()));
+    if (!browser_side_navigation) {
       render_view_->history_controller()->GoToEntry(
           entry.Pass(), navigation_params.Pass(), cache_policy);
+      pending_navigation_params_.reset();
+      return;
+    } else {

[Charlie Reis, 2015/05/27 23:34:36]

No else needed after a return.

[clamy, 2015/05/29 14:47:01]

Done.

+      item_for_history_navigation =
+          render_view_->history_controller()->FindItemForFrame(

[Charlie Reis, 2015/05/27 23:34:36]

This isn't going to make sense soon, and I'm not sure I understand it now. 
Soon, the browser process will be telling each frame which history item to load
directly.

For now, we only get to NavigateInternal for the main frame, so this would only
find a single item for the main frame.  How would we handle the case of going
back in a subframe?

[clamy, 2015/05/29 14:47:01]

With PlzNavigate we also get to NavigateInternal on subframes (we get there when
the response for the navigation was received). If we get the history item from
the browser it's even better since we would just set item_for_history_navigation
to whatever we received in the meantime (and no need to find the relevant one).
Once you're done, I will just have to make sure that the proper history item is
sent to the subframes, hence the TODO.

I hope what we are doing is clearer in the new patchset with the call to
GetHistoryNodeForFrame.

+              entry.Pass(), this, cache_policy, &load_type);
     }
-  } else if (!common_params.base_url_for_data_url.is_empty() ||
-             (browser_side_navigation &&
-              common_params.url.SchemeIs(url::kDataScheme))) {
-    LoadDataURL(common_params, frame_);
   } else {
     // Navigate to the given URL.
-    WebURLRequest request = CreateURLRequestForNavigation(
-        common_params, stream_params.Pass(), frame_->isViewSourceModeEnabled());
-
     if (!start_params.extra_headers.empty() && !browser_side_navigation) {
       for (net::HttpUtil::HeadersIterator i(start_params.extra_headers.begin(),
                                             start_params.extra_headers.end(),
                                             "\n");
            i.GetNext();) {
         request.addHTTPHeaderField(WebString::fromUTF8(i.name()),
                                    WebString::fromUTF8(i.values()));
       }
     }
 
     if (start_params.is_post && !browser_side_navigation) {
       request.setHTTPMethod(WebString::fromUTF8("POST"));
 
       // Set post data.
       WebHTTPBody http_body;
       http_body.initialize();
       const char* data = nullptr;
       if (start_params.browser_initiated_post_data.size()) {
         data = reinterpret_cast<const char*>(
             &start_params.browser_initiated_post_data.front());
       }
       http_body.appendData(
           WebData(data, start_params.browser_initiated_post_data.size()));
       request.setHTTPBody(http_body);
     }
 
     // A session history navigation should have been accompanied by state.
     CHECK_EQ(request_params.page_id, -1);
 
-    // PlzNavigate: Make sure that Blink's loader will not try to use browser
-    // side navigation for this request (since it already went to the browser).
-    if (browser_side_navigation)
-      request.setCheckForBrowserSideNavigation(false);
-
     // Record this before starting the load. We need a lower bound of this time
     // to sanitize the navigationStart override set below.
     base::TimeTicks renderer_navigation_start = base::TimeTicks::Now();
-    frame_->loadRequest(request);
-
     UpdateFrameNavigationTiming(frame_, request_params.browser_navigation_start,
                                 renderer_navigation_start);
   }
 
+  // Perform a navigation to a data url if needed.
+  if (!common_params.base_url_for_data_url.is_empty() ||
+      (browser_side_navigation &&
+       common_params.url.SchemeIs(url::kDataScheme))) {
+    LoadDataURL(common_params, frame_);
+    pending_navigation_params_.reset();
+    return;
+  }
+
+  // Load the request.
+  frame_->loadRequest(request, load_type, item_for_history_navigation);
+
   // In case LoadRequest failed before didCreateDataSource was called.
   pending_navigation_params_.reset();
 }
 
 void RenderFrameImpl::UpdateEncoding(WebFrame* frame,
                                      const std::string& encoding_name) {
   // Only update main frame's encoding_name.
   if (!frame->parent())
     Send(new FrameHostMsg_UpdateEncoding(routing_id_, encoding_name));
 }
@@ skipping 398 matching lines @@
 #elif defined(ENABLE_BROWSER_CDMS)
         cdm_manager_,
 #endif
         this);
   }
 
   return cdm_factory_;
 }
 
 }  // namespace content