Ensure the new navigation classifier works in the real world.

content/browser/frame_host/render_frame_host_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/render_frame_host_impl.h"
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/containers/hash_tables.h"
 #include "base/lazy_instance.h"
@@ skipping 770 matching lines @@
 // notification containing parameters identifying the navigation.
 //
 // Subframes are identified by the page transition type.  For subframes loaded
 // as part of a wider page load, the page_id will be the same as for the top
 // level frame.  If the user explicitly requests a subframe navigation, we will
 // get a new page_id because we need to create a new navigation entry for that
 // action.
 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());
 
   // Sanity-check the page transition for frame type.
   DCHECK_EQ(ui::PageTransitionIsMainFrame(validated_params.transition),
             !GetParent());
 
   // If we're waiting for a cross-site beforeunload ack from this renderer and
   // we receive a Navigate message from the main frame, then the renderer was
   // navigating already and sent it before hearing the FrameMsg_Stop message.
   // We do not want to cancel the pending navigation in this case, since the
   // old page will soon be stopped.  Instead, treat this as a beforeunload ack
   // to allow the pending navigation to continue.
   if (is_waiting_for_beforeunload_ack_ &&
       unload_ack_is_for_navigation_ &&
       !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;
   }
 
   // If we're waiting for an unload ack from this renderer and we receive a
   // Navigate message, then the renderer was navigating before it received the
   // 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) {
     UMA_HISTOGRAM_CUSTOM_TIMES(
         "Navigation.UI_OnCommitProvisionalLoad.Link",
         base::TimeTicks::Now() - validated_params.ui_timestamp,
         base::TimeDelta::FromMilliseconds(10), base::TimeDelta::FromMinutes(10),
         100);
   } else if (validated_params.report_type ==
              FrameMsg_UILoadMetricsReportType::REPORT_INTENT) {
@@ skipping 31 matching lines @@
     process->FilterURL(false, &(*it));
   }
   process->FilterURL(true, &validated_params.searchable_form_url);
 
   // Without this check, the renderer can trick the browser into using
   // filenames it can't access in a future session restore.
   if (!render_view_host_->CanAccessFilesOfPageState(
           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;
   }
 
   accessibility_reset_count_ = 0;
   frame_tree_node()->navigator()->DidNavigate(this, validated_params);
 
   // PlzNavigate
   if (base::CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kEnableBrowserSideNavigation)) {
     pending_commit_ = false;
@@ skipping 1182 matching lines @@
 
   // We may be returning to an existing NavigationEntry that had been granted
   // file access.  If this is a different process, we will need to grant the
   // access again.  The files listed in the page state are validated when they
   // are received from the renderer to prevent abuse.
   if (request_params.page_state.IsValid()) {
     render_view_host_->GrantFileAccessFromPageState(request_params.page_state);
   }
 }
 
+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