Ensure we properly set PageTransition for iframes.

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 1045 matching lines @@
 
   WebFrame* frame = frame_;
   if (!commit_params.frame_to_navigate.empty()) {
     // TODO(nasko): Move this lookup to the browser process.
     frame = render_view_->webview()->findFrameByName(
         WebString::fromUTF8(commit_params.frame_to_navigate));
     CHECK(frame) << "Invalid frame name passed: "
                  << commit_params.frame_to_navigate;
   }
 
+  // If this frame isn't in the same process as its parent, it will naively
+  // assume that this is the first navigation in the iframe, but this may not
+  // actually be the case. The PageTransition differentiates between the first
+  // navigation in a subframe and subsequent navigations, so if this is a
+  // subsequent navigation, force the frame's state machine forward.
+  if (ui::PageTransitionCoreTypeIs(common_params.transition,
+                                   ui::PAGE_TRANSITION_MANUAL_SUBFRAME)) {
+    CHECK(frame_->parent());
+    if (frame_->parent()->isWebRemoteFrame()) {
+      CHECK_EQ(frame, frame_);
+      frame_->setCommittedFirstRealLoad();
+    }
+  }
+
   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;
   }
 
   render_view_->pending_navigation_params_.reset(new NavigationParams(
       common_params, start_params, commit_params, history_params));
 
@@ skipping 1260 matching lines @@
   if (is_top_most) {
     render_view_->set_navigation_gesture(
         WebUserGestureIndicator::isProcessingUserGesture() ?
             NavigationGestureUser : NavigationGestureAuto);
   } else if (ds->replacesCurrentHistoryItem()) {
     // Subframe navigations that don't add session history items must be
     // marked with AUTO_SUBFRAME. See also didFailProvisionalLoad for how we
     // handle loading of error pages.
     document_state->navigation_state()->set_transition_type(
         ui::PAGE_TRANSITION_AUTO_SUBFRAME);
+  } else if (ui::PageTransitionCoreTypeIs(
+                 document_state->navigation_state()->transition_type(),
+                 ui::PAGE_TRANSITION_LINK)) {

[Charlie Reis, 2015/03/13 23:22:14]

Why is PAGE_TRANSITION_LINK the right way to detect this?  Is that somehow
equivalent to "creating a new history item?"

(I thought it came in as PAGE_TRANSITION_MANUAL_SUBFRAME.  When did it change?)

[Nate Chapin, 2015/03/13 23:28:38]

This is the initial classification of a navigation, before the browser process
sees the PageTransition. It starts as PAGE_TRANSITION_LINK, and the only time a
subframe will have any other state at this point is if it was set to
PAGE_TRANSITION_FORM_SUBMIT in RenderFrameImpl::willSubmitForm(). The
PageTransition selected by this function will be attached to the main resource
request, which will then get intercepted by CrossSiteResourceHandler and
transfered to a different process.

Basically, this happens in the original process of a navigation, while the
changes in OnNavigate() happen after it moves to the new process.

[Charlie Reis, 2015/03/14 00:03:49]

Ah!  It makes so much more sense now.  :)  Can you put that in the comment?

+    // Subframe navigations that are creating a new history item should be
+    // marked MANUAL_SUBFRAME, unless it has already been marked as a
+    // FORM_SUBMIT.
+    document_state->navigation_state()->set_transition_type(
+        ui::PAGE_TRANSITION_MANUAL_SUBFRAME);
   }
 
   FOR_EACH_OBSERVER(RenderViewObserver, render_view_->observers(),
                     DidStartProvisionalLoad(frame));
   FOR_EACH_OBSERVER(RenderFrameObserver, observers_, DidStartProvisionalLoad());
 
   Send(new FrameHostMsg_DidStartProvisionalLoadForFrame(
        routing_id_, ds->request().url(), is_transition_navigation));
 }
 
@@ skipping 612 matching lines @@
       // TODO(nasko): When the top-level frame is remote, there is no document.
       // This is broken and should be fixed to propagate the first party.
       WebFrame* top = frame->top();
       if (top->isWebLocalFrame()) {
         request.setFirstPartyForCookies(
             frame->top()->document().firstPartyForCookies());
       }
     }
   }
 
-  WebFrame* top_frame = frame->top();
-  // TODO(nasko): Hack around asking about top-frame data source. This means
-  // for out-of-process iframes we are treating the current frame as the
-  // top-level frame, which is wrong.
-  if (!top_frame || top_frame->isWebRemoteFrame())
-    top_frame = frame;
-  WebDataSource* provisional_data_source = top_frame->provisionalDataSource();
-  WebDataSource* top_data_source = top_frame->dataSource();
+  WebDataSource* provisional_data_source = frame->provisionalDataSource();
   WebDataSource* data_source =
-      provisional_data_source ? provisional_data_source : top_data_source;
+      provisional_data_source ? provisional_data_source : frame->dataSource();
 
   DocumentState* document_state = DocumentState::FromDataSource(data_source);
   DCHECK(document_state);
   InternalDocumentStateData* internal_data =
       InternalDocumentStateData::FromDocumentState(document_state);
   NavigationState* navigation_state = document_state->navigation_state();
   ui::PageTransition transition_type = navigation_state->transition_type();
   WebDataSource* frame_ds = frame->provisionalDataSource();
   if (frame_ds && frame_ds->isClientRedirect()) {
     transition_type = ui::PageTransitionFromInt(
@@ skipping 116 matching lines @@
   extra_data->set_transition_type(transition_type);
   extra_data->set_should_replace_current_entry(should_replace_current_entry);
   extra_data->set_transferred_request_child_id(
       navigation_state->transferred_request_child_id());
   extra_data->set_transferred_request_request_id(
       navigation_state->transferred_request_request_id());
   extra_data->set_service_worker_provider_id(provider_id);
   extra_data->set_stream_override(stream_override.Pass());
   request.setExtraData(extra_data);
 
+  WebFrame* top_frame = frame->top();
+  // TODO(nasko): Hack around asking about top-frame data source. This means
+  // for out-of-process iframes we are treating the current frame as the
+  // top-level frame, which is wrong.
+  if (!top_frame || top_frame->isWebRemoteFrame())
+    top_frame = frame;
   DocumentState* top_document_state =
-      DocumentState::FromDataSource(top_data_source);
+      DocumentState::FromDataSource(top_frame->dataSource());
   if (top_document_state) {
     // TODO(gavinp): separate out prefetching and prerender field trials
     // if the rel=prerender rel type is sticking around.
     if (request.requestContext() == WebURLRequest::RequestContextPrefetch)
       top_document_state->set_was_prefetcher(true);

[Charlie Reis, 2015/03/13 23:22:14]

Ah, sad that we still need this hack.  No worries for now; we'll fix it later.

   }
 
   // This is an instance where we embed a copy of the routing id
   // into the data portion of the message. This can cause problems if we
   // don't register this id on the browser side, since the download manager
   // expects to find a RenderViewHost based off the id.
   request.setRequestorID(render_view_->GetRoutingID());
   request.setHasUserGesture(WebUserGestureIndicator::isProcessingUserGesture());
 
   if (!navigation_state->extra_headers().empty()) {
@@ skipping 1382 matching lines @@
 
 #if defined(ENABLE_BROWSER_CDMS)
 RendererCdmManager* RenderFrameImpl::GetCdmManager() {
   if (!cdm_manager_)
     cdm_manager_ = new RendererCdmManager(this);
   return cdm_manager_;
 }
 #endif  // defined(ENABLE_BROWSER_CDMS)
 
 }  // namespace content