Ensure the new navigation classifier works in the real world.

content/browser/frame_host/render_frame_host_impl.cc

Index: content/browser/frame_host/render_frame_host_impl.cc
diff --git a/content/browser/frame_host/render_frame_host_impl.cc b/content/browser/frame_host/render_frame_host_impl.cc
index 0f5b5ce80f5c5d90e1363b53e76ea2395815b71d..8ea65c35412c043dcdf6d1b815c79b3aadcf178c 100644
--- a/content/browser/frame_host/render_frame_host_impl.cc
+++ b/content/browser/frame_host/render_frame_host_impl.cc
@@ -788,11 +788,15 @@ void RenderFrameHostImpl::OnDidFailLoadWithError(
 void RenderFrameHostImpl::OnDidCommitProvisionalLoad(const IPC::Message& msg) {
   // Read the parameters out of the IPC message directly to avoid making another
   // copy when we filter the URLs.
+  ++commit_count_;
   base::PickleIterator iter(msg);
   FrameHostMsg_DidCommitProvisionalLoad_Params validated_params;
   if (!IPC::ParamTraits<FrameHostMsg_DidCommitProvisionalLoad_Params>::
-      Read(&msg, &iter, &validated_params))
+      Read(&msg, &iter, &validated_params)) {
+    base::debug::SetCrashKeyValue("369661-earlyreturn",
+                                  CommitCountString() + "/badipc");
     return;
+  }
   TRACE_EVENT1("navigation", "RenderFrameHostImpl::OnDidCommitProvisionalLoad",
                "url", validated_params.url.possibly_invalid_spec());
 
@@ -811,6 +815,8 @@ void RenderFrameHostImpl::OnDidCommitProvisionalLoad(const IPC::Message& msg) {
       !GetParent()) {
     base::TimeTicks approx_renderer_start_time = send_before_unload_start_time_;
     OnBeforeUnloadACK(true, approx_renderer_start_time, base::TimeTicks::Now());
+    base::debug::SetCrashKeyValue("369661-earlyreturn",
+                                  CommitCountString() + "/beforeunloadwait");
     return;
   }
 
@@ -819,8 +825,11 @@ void RenderFrameHostImpl::OnDidCommitProvisionalLoad(const IPC::Message& msg) {
   // unload request.  It will either respond to the unload request soon or our
   // timer will expire.  Either way, we should ignore this message, because we
   // have already committed to closing this renderer.
-  if (IsWaitingForUnloadACK())
+  if (IsWaitingForUnloadACK()) {
+    base::debug::SetCrashKeyValue("369661-earlyreturn",
+                                  CommitCountString() + "/unloadwait");
     return;
+  }
 
   if (validated_params.report_type ==
       FrameMsg_UILoadMetricsReportType::REPORT_LINK) {
@@ -872,6 +881,8 @@ void RenderFrameHostImpl::OnDidCommitProvisionalLoad(const IPC::Message& msg) {
           validated_params.page_state)) {
     bad_message::ReceivedBadMessage(
         GetProcess(), bad_message::RFH_CAN_ACCESS_FILES_OF_PAGE_STATE);
+    base::debug::SetCrashKeyValue("369661-earlyreturn",
+                                  CommitCountString() + "/fileaccess");
     return;
   }
 
@@ -2074,4 +2085,11 @@ void RenderFrameHostImpl::UpdatePermissionsForNavigation(
   }
 }
 
+std::string RenderFrameHostImpl::CommitCountString() {
+  std::string result = base::Int64ToString(reinterpret_cast<int64_t>(this));

[Charlie Reis, 2015/06/09 23:32:13]

This seems very suspicious to me (signed int, reinterpret_cast, etc).  I'm not
sure the actual pointer value will help us much.  Can you just put the process
ID + routing ID instead?

[Avi (use Gerrit), 2015/06/10 16:34:52]

The *actual* value of the pointer is completely irrelevant. I only need to
correlate the commits per RFH (e.g. for renderframe X we get this stuff logged
for commit 1 and commit 2. I need some value, and whether it's the "this"
pointer or the routing id doesn't matter.

+  result += "/";
+  result += base::IntToString(commit_count_);
+  return result;
+}
+
 }  // namespace content