Changes PlzNavitate histograms to try and simplify their multi-modal characteristic.

content/browser/frame_host/navigator.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_NAVIGATOR_H_
 #define CONTENT_BROWSER_FRAME_HOST_NAVIGATOR_H_
 
 #include "base/memory/ref_counted.h"
 #include "base/time/time.h"
 #include "content/common/content_export.h"
@@ skipping 123 matching lines @@
   // PlzNavigate
   // Cancel a NavigationRequest for |frame_tree_node|. Called when
   // |frame_tree_node| is destroyed.
   virtual void CancelNavigation(FrameTreeNode* frame_tree_node) {}
 
   // Called when the first resource request for a given navigation is executed
   // so that it can be tracked into an histogram.
   virtual void LogResourceRequestTime(
     base::TimeTicks timestamp, const GURL& url) {};
 
+  // Called to record the time it took to execute the before unload hook for the
+  // current navigation.
+  virtual void LogBeforeUnloadTime(

[clamy, 2014/10/09 19:02:32]

Note: if we only discount the time it took to run the beforeunload event on the
renderer side, we may still get a bi-modal distribution due to the waiting time
for the BeforeUnload IPC to get process on the renderer side (due to javascript
currently executing). We have to be okay with that.

[carlosk, 2014/10/10 10:00:03]

So you mean that javascript CPU load could delay the processing of the
FrameMsg_BeforeUnload IPC message on the renderer side? I though that javascript
would only play a role inside RenderFrameImpl::OnBeforeUnload @ line 1027:

bool proceed = frame_->dispatchBeforeUnloadEvent();

I understand that for PlzNavigate we'd probably want to remove the dependency on
the current javascript load and I think that might be the way to go. But the
risk there is that we'll be ignoring this user-facing delay which is not due to
his own action delay (of pressing the "Leave page" button quickly) like the
current measured time is.

Maybe also log that wait time?

<time for the full round trip call from the browser> - <currently measured
before-unload dispatching time>

I'm not sure... WDYT?

[clamy, 2014/10/10 19:59:12]

No the delay is between the moment we send a BeforeUnload IPC and the moment
RenderFrameImpl::OnBeforeUnload is called. With the Unload event I measured up
to 3s delay on really bad Android devices. That said, I think we should include
it in those histograms. My comment is just pointing out that because of that, we
will probably still have multi-modal distributions in our histograms.

[carlosk, 2014/10/13 14:05:50]

That's precisely the delay I referred to. :)

I'll add another histogram (in another CL) to track the difference between the
browser side delay to get the whole of the on-before-unload even handled and the
renderer side value we are using. If we have a significant number of samples
with values above just a few 10ths of milliseconds than we'll have to see what
to do about this situation.

+      const base::TimeTicks& renderer_before_unload_start_time,
+      const base::TimeTicks& renderer_before_unload_end_time) {}
+
  protected:
   friend class base::RefCounted<Navigator>;
   virtual ~Navigator() {}
 };
 
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_FRAME_HOST_NAVIGATOR_H_