Log First User Interaction in Page Load Metrics

components/page_load_metrics/browser/metrics_web_contents_observer.cc

 // Copyright 2015 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 "components/page_load_metrics/browser/metrics_web_contents_observer.h"
 
 #include <algorithm>
 #include <string>
 #include <utility>
 
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
 #include "base/metrics/histogram.h"
 #include "base/metrics/user_metrics.h"
 #include "components/page_load_metrics/browser/page_load_metrics_util.h"
 #include "components/page_load_metrics/common/page_load_metrics_messages.h"
 #include "components/page_load_metrics/common/page_load_timing.h"
+#include "content/browser/renderer_host/render_view_host_impl.h"

[Bryan McQuade, 2016/05/25 21:32:29]

My understanding is that you can't depend on content/browser from anywhere
outside of content. See this code search:
https://code.google.com/p/chromium/codesearch#search/&q=include%5C%20%5C%22co...

There are a few exceptions but for the most part only other content code is
allowed to depend on content/browser.

In your case, I believe this means you'll need to add virtual
AddInputEventCallback/RemoveInputEventCallback methods to RenderWidgetHost,
which are then implemented in RenderWidgetHostImpl.

 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/navigation_details.h"
 #include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_observer.h"
 #include "content/public/browser/web_contents_user_data.h"
 #include "ipc/ipc_message.h"
 #include "ipc/ipc_message_macros.h"
 #include "ui/base/page_transition_types.h"
 
 DEFINE_WEB_CONTENTS_USER_DATA_KEY(
     page_load_metrics::MetricsWebContentsObserver);
 
+class RenderViewHost;
+
 namespace page_load_metrics {
 
 namespace internal {
 
 const char kErrorEvents[] = "PageLoad.Events.InternalError";
 const char kAbortChainSizeReload[] =
     "PageLoad.Internal.ProvisionalAbortChainSize.Reload";
 const char kAbortChainSizeForwardBack[] =
     "PageLoad.Internal.ProvisionalAbortChainSize.ForwardBack";
 const char kAbortChainSizeNewNavigation[] =
@@ skipping 315 matching lines @@
     observer->OnFailedProvisionalLoad(navigation_handle);
   }
 }
 
 void PageLoadTracker::Redirect(content::NavigationHandle* navigation_handle) {
   for (const auto& observer : observers_) {
     observer->OnRedirect(navigation_handle);
   }
 }
 
+void PageLoadTracker::OnInputEvent(const blink::WebInputEvent& event) {
+  // Only log the first user interaction in a given page load.
+  if (user_interaction_time_.is_null())
+    user_interaction_time_ = base::TimeTicks::Now();
+
+  for (const auto& observer : observers_) {
+    observer->OnUserInput(event);
+  }
+}
+
 bool PageLoadTracker::UpdateTiming(const PageLoadTiming& new_timing,
                                    const PageLoadMetadata& new_metadata) {
   // Throw away IPCs that are not relevant to the current navigation.
   // Two timing structures cannot refer to the same navigation if they indicate
   // that a navigation started at different times, so a new timing struct with a
   // different start time from an earlier struct is considered invalid.
   bool valid_timing_descendent =
       timing_.navigation_start.is_null() ||
       timing_.navigation_start == new_timing.navigation_start;
   // Ensure flags sent previously are still present in the new metadata fields.
@@ skipping 26 matching lines @@
 }
 
 void PageLoadTracker::AddObserver(
     std::unique_ptr<PageLoadMetricsObserver> observer) {
   observers_.push_back(std::move(observer));
 }
 
 PageLoadExtraInfo PageLoadTracker::GetPageLoadMetricsInfo() {
   base::TimeDelta first_background_time;
   base::TimeDelta first_foreground_time;
+  base::TimeDelta first_user_interaction_time;
   base::TimeDelta time_to_abort;
   base::TimeDelta time_to_commit;
   if (!background_time_.is_null())
     first_background_time = background_time_ - navigation_start_;
   if (!foreground_time_.is_null())
     first_foreground_time = foreground_time_ - navigation_start_;
+  if (!user_interaction_time_.is_null())
+    first_user_interaction_time = user_interaction_time_ - navigation_start_;
   if (abort_type_ != ABORT_NONE) {
     DCHECK_GT(abort_time_, navigation_start_);
     time_to_abort = abort_time_ - navigation_start_;
   } else {
     DCHECK(abort_time_.is_null());
   }
 
   if (!commit_time_.is_null()) {
     DCHECK_GT(commit_time_, navigation_start_);
     time_to_commit = commit_time_ - navigation_start_;
   } else {
     DCHECK(commit_time_.is_null());
   }
   return PageLoadExtraInfo(
-      first_background_time, first_foreground_time, started_in_foreground_,
-      commit_time_.is_null() ? GURL() : url_, time_to_commit, abort_type_,
-      time_to_abort, metadata_);
+      first_background_time, first_foreground_time, first_user_interaction_time,
+      started_in_foreground_, commit_time_.is_null() ? GURL() : url_,
+      time_to_commit, abort_type_, time_to_abort, metadata_);
 }
 
 void PageLoadTracker::NotifyAbort(UserAbortType abort_type,
                                   base::TimeTicks timestamp) {
   DCHECK_NE(abort_type, ABORT_NONE);
   // Use UpdateAbort to update an already notified PageLoadTracker.
   if (abort_type_ != ABORT_NONE)
     return;
 
   UpdateAbortInternal(abort_type, timestamp);
@@ skipping 60 matching lines @@
     content::WebContents* web_contents,
     std::unique_ptr<PageLoadMetricsEmbedderInterface> embedder_interface) {
   DCHECK(web_contents);
 
   MetricsWebContentsObserver* metrics = FromWebContents(web_contents);
   if (!metrics) {
     metrics = new MetricsWebContentsObserver(web_contents,
                                              std::move(embedder_interface));
     web_contents->SetUserData(UserDataKey(), metrics);
   }
+
+  content::RenderWidgetHostImpl* rwhi = content::RenderWidgetHostImpl::From(

[Bryan McQuade, 2016/05/25 20:47:04]

since we're unlistening in the destructor, let's listen in the construct rather
than in the CreateForWebContents method. We call the constructo in
CreateForWebContents and in the unit test, so in the latter case, if we keep
this code here, we'll attempt to unlisten in the destructor without having ever
attached ourselves as a listener.

+      metrics->web_contents()->GetRenderViewHost()->GetWidget());
+  rwhi->latency_tracker()->AddInputEventCallback(base::Bind(
+      &MetricsWebContentsObserver::OnInputEvent, base::Unretained(metrics)));
+
   return metrics;
 }
 
 MetricsWebContentsObserver::~MetricsWebContentsObserver() {
-  NotifyAbortAllLoads(ABORT_CLOSE);
+  content::RenderWidgetHostImpl* rwhi = content::RenderWidgetHostImpl::From(
+      web_contents()->GetRenderViewHost()->GetWidget());

[Bryan McQuade, 2016/05/25 19:02:47]

My understanding is that a RenderViewHost's lifetime may be shorter than a
WebContents, so if the RVH associated with the web contents changes, and the
RenderWidgetHost is bound to the RVH and not the WebContents, we'll stop getting
events once the RVH changes.

That said, my understanding of these relationships is not great, so I could very
well be mistaken. My understanding is that an RVH is bound to a render process,
which can change for a given WebContents if e.g. the user navigates between
origins in the same tab (since we use different render processes for different
origins).

Anyway, it is worth digging into this to make sure this either is or is not an
issue for us. Can you dig in further and see what you find? Or if another
reviewer knows for sure, please reply & let us know.

[Bryan McQuade, 2016/05/25 20:47:04]

I think you can address this issue by implementing RenderViewHostChanged:

https://code.google.com/p/chromium/codesearch#chromium/src/content/public/bro...

+  rwhi->latency_tracker()->RemoveInputEventCallback(base::Bind(
+      &MetricsWebContentsObserver::OnInputEvent, base::Unretained(this)));
 }
 
 bool MetricsWebContentsObserver::OnMessageReceived(
     const IPC::Message& message,
     content::RenderFrameHost* render_frame_host) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   bool handled = true;
   IPC_BEGIN_MESSAGE_MAP_WITH_PARAM(MetricsWebContentsObserver, message,
                                    render_frame_host)
     IPC_MESSAGE_HANDLER(PageLoadMetricsMsg_TimingUpdated, OnTimingUpdated)
@@ skipping 117 matching lines @@
   committed_load_->set_renderer_tracked(
       IsRelevantNavigation(navigation_handle, browser_url, mime_type));
 
   committed_load_->Commit(navigation_handle);
 }
 
 void MetricsWebContentsObserver::NavigationStopped() {
   NotifyAbortAllLoads(ABORT_STOP);
 }
 
+bool MetricsWebContentsObserver::OnInputEvent(
+    const blink::WebInputEvent& event) {
+  // ignore browser navigation or reload which comes with type Undefined

[tdresser, 2016/05/24 14:28:38]

Capitalization and end with .

+  if (event.type != blink::WebInputEvent::Type::Undefined) {
+    if (!committed_load_) {
+      RecordInternalError(ERR_USER_INTERACTION_WITH_NO_RELEVANT_LOAD);
+      return false;
+    }
+    committed_load_->OnInputEvent(event);
+    return true;
+  }
+
+  return false;
+}
+
 void MetricsWebContentsObserver::DidRedirectNavigation(
     content::NavigationHandle* navigation_handle) {
   if (!navigation_handle->IsInMainFrame())
     return;
   auto it = provisional_loads_.find(navigation_handle);
   if (it == provisional_loads_.end())
     return;
   it->second->Redirect(navigation_handle);
 }
 
@@ skipping 112 matching lines @@
 
   if (!committed_load_->UpdateTiming(timing, metadata)) {
     // If the page load tracker cannot update its timing, something is wrong
     // with the IPC (it's from another load, or it's invalid in some other way).
     // We expect this to be a rare occurrence.
     RecordInternalError(ERR_BAD_TIMING_IPC);
   }
 }
 
 }  // namespace page_load_metrics