Store the last committed origin in RenderFrameHostImpl.

content/browser/frame_host/navigator_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/browser/frame_host/navigator_impl.h"
 
 #include <utility>
 
 #include "base/logging.h"
 #include "base/metrics/histogram_macros.h"
@@ skipping 622 matching lines @@
   if (old_entry_count != controller_->GetEntryCount() ||
       details.previous_entry_index !=
           controller_->GetLastCommittedEntryIndex()) {
     frame_tree->root()->render_manager()->SendPageMessage(
         new PageMsg_SetHistoryOffsetAndLength(
             MSG_ROUTING_NONE, controller_->GetLastCommittedEntryIndex(),
             controller_->GetEntryCount()),
         site_instance);
   }
 
-  // Keep track of each frame's URL in its FrameTreeNode, whether it's for a net
-  // error or not.
-  // TODO(creis): Move the last committed URL to RenderFrameHostImpl.
+  // Keep track of the last committed URL and origin in the RenderFrameHost
+  // itself.  These allow GetLastCommittedURL and GetLastCommittedOrigin to
+  // stay correct even if the render_frame_host later becomes pending deletion.
+  // The URL is set regardless of whether it's for a net error or not.
   render_frame_host->frame_tree_node()->SetCurrentURL(params.url);
+  render_frame_host->set_last_committed_origin(params.origin);

[alexmos, 2016/12/06 17:22:40]

I originally tried to do this inside SetCurrentOrigin, which matches what we did
for SetCurrentURL.  But that didn't work -- SetCurrentOrigin is called above
this, intentionally before CommitPending when oopifs are on, so that the
replication state already has the latest origin when it's sent with
FrameMsg_SwapOut.  But at that point, current_frame_host() is still wrong, so
FTN::SetCurrentOrigin can't use it to update the last committed origin.

I didn't do this next to SetCurrentOrigin above, since I thought it's better to
set this on the RFH after it really becomes the current RFH.

[Charlie Reis, 2016/12/08 19:51:47]

Interesting.  Yes, I agree with your decisions.

 
   // Separately, update the frame's last successful URL except for net error
   // pages, since those do not end up in the correct process after transfers
   // (see https://crbug.com/560511).  Instead, the next cross-process navigation
   // or transfer should decide whether to swap as if the net error had not
   // occurred.
   // TODO(creis): Remove this block and always set the URL once transfers handle
   // network errors or PlzNavigate is enabled.  See https://crbug.com/588314.
   if (!params.url_is_unreachable)
     render_frame_host->set_last_successful_url(params.url);
@@ skipping 614 matching lines @@
     if (navigation_handle)
       navigation_handle->update_entry_id_for_transfer(entry->GetUniqueID());
 
     controller_->SetPendingEntry(std::move(entry));
     if (delegate_)
       delegate_->NotifyChangedNavigationState(content::INVALIDATE_TYPE_URL);
   }
 }
 
 }  // namespace content