Store the last committed origin in RenderFrameHostImpl.

content/browser/frame_host/frame_tree_node.h

 // 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.
 
 #ifndef CONTENT_BROWSER_FRAME_HOST_FRAME_TREE_NODE_H_
 #define CONTENT_BROWSER_FRAME_HOST_FRAME_TREE_NODE_H_
 
 #include <stddef.h>
 
 #include <memory>
@@ skipping 120 matching lines @@
   // Sets the last committed URL for this frame and updates
   // has_committed_real_load accordingly.
   void SetCurrentURL(const GURL& url);
 
   // Returns true iff SetCurrentURL has been called with a non-blank URL.
   bool has_committed_real_load() const {
     return has_committed_real_load_;
   }
 
   // Returns the origin of the last committed page in this frame.
+  // WARNING: To get the last committed origin for a particular
+  // RenderFrameHost, use RenderFrameHost::GetLastCommittedOrigin() instead,
+  // which will behave correctly even when the RenderFrameHost is not the
+  // current one for this frame (such as when it's pending deletion).

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

I'm not thrilled about storing the origin in two places (on RFH and in
replication state), but I don't know if there's a good way around it, given that
we have to send the origin as part of the replication state anyway, and the
quirk in the comment below.  I didn't really like some other options (don't keep
the current origin in replication state and fill it in from
current_frame_host()->GetLastCommittedOrigin() whenever we need to send the
replication state; separate origin out of the replication state). I'm hoping
this big warning clarifies when to use each, but let me know if you have
thoughts about it.

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

Yeah, I see the dilemma.  I'm ok with this at least for now, since we're adding
a way to avoid the original problem.  People tend not to notice these comments,
but if it proves to be a problem we can deprecate this later.

   const url::Origin& current_origin() const {
     return replication_state_.origin;

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

I tried putting a DCHECK here to see if any callers use this when this value
doesn't match current_frame_host()->GetLastCommittedOrigin().  Interestingly,
it's possible that for a brief time this is actually not true.  This is due to
the early CommitPending call in UpdateStateForNavigate if we're on the sad tab
-- we make the pending RFH the current one without waiting for it to commit, so
if we check current_origin() before the actual commit, the replication state
will have the old origin (from crashed page), and the current_frame_host() will
return the origin of the RFH about to be committed, i.e., null.  Sure enough,
that happens when we check current_origin() in
NavigationControllerImpl::IsURLInPageNavigation, as part of DidNavigate. 
Reading IsURLInPageNavigation, I *think* its use of current_origin() (and not
GetLastCommittedOrigin) is intentional, but it'd be nice if you can verify (
https://codereview.chromium.org/1457053004 introduced it).  It also seems the
value returned by current_origin() in this specific time window/situation is
actually correct (returning the old committed origin rather than the new
not-yet-committed-but-current RFH's null origin).

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

I agree that IsURLInPageNavigation needs to get the old origin.  Interesting
that the optimization for sad tab interferes; I guess that's ok.

   }
 
   // Set the current origin and notify proxies about the update.
   void SetCurrentOrigin(const url::Origin& origin,
                         bool is_potentially_trustworthy_unique_origin);
 
   // Set the current name and notify proxies about the update.
   void SetFrameName(const std::string& name, const std::string& unique_name);
 
   // Set the frame's feature policy from an HTTP header, clearing any existing
@@ skipping 225 matching lines @@
   // browser process activities to this node (when possible).  It is unrelated
   // to the core logic of FrameTreeNode.
   FrameTreeNodeBlameContext blame_context_;
 
   DISALLOW_COPY_AND_ASSIGN(FrameTreeNode);
 };
 
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_FRAME_HOST_FRAME_TREE_NODE_H_