Adding DRP specfic UMA for FirstContentfulPaint

chrome/browser/renderer_host/chrome_resource_dispatcher_host_delegate.cc

 // Copyright (c) 2012 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 "chrome/browser/renderer_host/chrome_resource_dispatcher_host_delegate.h"
 
 #include <stdint.h>
 #include <string>
 #include <utility>
 #include <vector>
@@ skipping 11 matching lines @@
 #include "chrome/browser/download/download_resource_throttle.h"
 #include "chrome/browser/mod_pagespeed/mod_pagespeed_metrics.h"
 #include "chrome/browser/net/resource_prefetch_predictor_observer.h"
 #include "chrome/browser/plugins/plugin_prefs.h"
 #include "chrome/browser/prerender/prerender_manager.h"
 #include "chrome/browser/prerender/prerender_manager_factory.h"
 #include "chrome/browser/prerender/prerender_resource_throttle.h"
 #include "chrome/browser/prerender/prerender_util.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_io_data.h"
+#include "chrome/browser/renderer_host/chrome_navigation_data.h"
 #include "chrome/browser/renderer_host/safe_browsing_resource_throttle.h"
 #include "chrome/browser/renderer_host/thread_hop_resource_throttle.h"
 #include "chrome/browser/safe_browsing/safe_browsing_service.h"
 #include "chrome/browser/signin/chrome_signin_helper.h"
 #include "chrome/browser/tab_contents/tab_util.h"
 #include "chrome/browser/ui/login/login_handler.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/features.h"
 #include "chrome/common/url_constants.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
+#include "components/data_reduction_proxy/core/browser/data_reduction_proxy_data.h"
 #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data.h"
 #include "components/google/core/browser/google_util.h"
 #include "components/policy/core/common/cloud/policy_header_io_helper.h"
 #include "components/variations/net/variations_http_headers.h"
 #include "content/public/browser/browser_thread.h"
+#include "content/public/browser/navigation_data.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/plugin_service.h"
 #include "content/public/browser/plugin_service_filter.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/resource_context.h"
 #include "content/public/browser/resource_dispatcher_host.h"
 #include "content/public/browser/resource_request_info.h"
 #include "content/public/browser/service_worker_context.h"
 #include "content/public/browser/stream_info.h"
@@ skipping 658 matching lines @@
     return data_reduction_proxy_io_data->ShouldEnableLoFiMode(url_request);
   return false;
 }
 
 // static
 void ChromeResourceDispatcherHostDelegate::
     SetExternalProtocolHandlerDelegateForTesting(
     ExternalProtocolHandler::Delegate* delegate) {
   g_external_protocol_handler_delegate = delegate;
 }
+
+std::unique_ptr<content::NavigationData>
+ChromeResourceDispatcherHostDelegate::GetNavigationData(
+    net::URLRequest* request) const {
+  std::unique_ptr<ChromeNavigationData> data(new ChromeNavigationData());
+  // Get the existing Data instance off of URLRequest.

[bengr, 2016/04/29 21:14:20]

Delete this comment.

[RyanSturm, 2016/05/02 19:52:19]

Done.

+  data_reduction_proxy::DataReductionProxyData* data_reduction_proxy_data =
+      data_reduction_proxy::DataReductionProxyData::DataForRequest(request,

[bengr, 2016/04/29 21:14:20]

Maybe the reason you felt compelled to add a comment is that the function name
doesn't clearly indicate what it does. How about GetData()?

[RyanSturm, 2016/05/02 19:52:19]

Done.

+                                                                   false);

[bengr, 2016/04/29 21:14:20]

This second parameter is probably the cleaner way to go. Did you consider having
a separate function called GetDataAndCreateIfNecessary()? 

[RyanSturm, 2016/05/02 19:52:19]

Because of the difference in const-ness of the URLRequest in the two functions,
the alternative makes a lot of sense. It allows the URLRequest* in
GetNavigationData to be const for instance.

+  // DeepCopy the DataReductionProxyData from the URLRequest to prevent the
+  // URLRequest and DataReductionProxyData from both having ownership of the
+  // same object. This copy will be shortlived as it will be DeepCopy'd again

[bengr, 2016/04/29 21:14:20]

DeepCopy'd -> deep copied

[RyanSturm, 2016/05/02 19:52:19]

Done.

+  // when content makes a clone of NavigationData for the UI thread.

[bengr, 2016/04/29 21:14:20]

You should make a note somewhere (probably on the DeepCopy method) that we
assume the Data object is reasonably inexpensive to deep copy.

[RyanSturm, 2016/05/02 19:52:19]

Done.

+  if (data_reduction_proxy_data)
+    data->SetDataReductionProxyData(data_reduction_proxy_data->DeepCopy());
+  return std::move(data);
+}