Keep a copy of page id in RenderViewHost.

content/renderer/render_frame_impl.cc

Index: content/renderer/render_frame_impl.cc
diff --git a/content/renderer/render_frame_impl.cc b/content/renderer/render_frame_impl.cc
index f5f7c774390858ba5800e73787c4d8d08acb0fb6..3229772219cbf287b87f087a59987baaedb4cea0 100644
--- a/content/renderer/render_frame_impl.cc
+++ b/content/renderer/render_frame_impl.cc
@@ -2230,6 +2230,8 @@ void RenderFrameImpl::didCommitProvisionalLoad(
     }
   }
 
+  Send(new FrameHostMsg_DidAssignPageId(routing_id_, render_view_->page_id_));

[Charlie Reis, 2014/09/08 16:04:37]

This part looks new to me, but I think it makes sense, to protect us from
unexpected behavior in the observers.

Note that there is a branch above that doesn't assign page_id_ (if the inner
check on line 2214 fails).  I don't know what an "unsuccessful history
navigation" is, so I'm not sure if it matters if we send the old page ID to the
browser in that case.  Probably shouldn't be a problem, but maybe we have a test
case that hits that branch to help us understand it?

[Avi (use Gerrit), 2014/09/08 16:40:48]

Yes, this is new. There is so much uncertainty about doing stuff before sending
DidCommitProvisionalLoad or whether we process it, etc, that I just wanted to
say f-it and just ipc.
I'm pretty sure sending an unneccessary IPC is OK here. Do we have a test case?
I don't know. I'd need to look.

[Charlie Reis, 2014/09/08 16:51:00]

I tossed in a NOTREACHED in the else block and these tests in
content_browsertests hit it:

    RenderViewHostTest.BasicRenderFrameHost
    RenderViewHostTest.ReleaseSessionOnCloseACK
    RenderViewImplTest.ImeComposition
    RenderViewImplTest.NavigateFrame

Perhaps they can help explain that case?

I agree that this will likely be fine, but it'd be nice to know why we're
getting here.

[Avi (use Gerrit), 2014/09/08 17:02:09]

It would indeed be good to know what's going on. During the next cycle of tests
(apply patch, crash on the canary, revert) I'll dig there.

[Charlie Reis, 2014/09/08 17:28:53]

I took a closer look, and we get there for this commit_type:
    // Reloads, client redirects, etc. Loads that neither originate from
    // nor add entries to the back/forward list.
    WebHistoryInertCommit

Sure enough, reloads make us hit my NOTREACHED.  But in that case, we should
just be telling the browser something it already knows with the new IPC, so I
think this change is fine.

+
   FOR_EACH_OBSERVER(RenderViewObserver, render_view_->observers_,
                     DidCommitProvisionalLoad(frame, is_new_navigation));
   FOR_EACH_OBSERVER(RenderFrameObserver, observers_,