PageLoadMetrics renderer and browser implementation (deprecated)

chrome/renderer/chrome_content_renderer_client.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/renderer/chrome_content_renderer_client.h"
 
 #include "base/command_line.h"
 #include "base/debug/crash_logging.h"
 #include "base/logging.h"
 #include "base/metrics/field_trial.h"
@@ skipping 46 matching lines @@
 #include "chrome/renderer/worker_content_settings_client_proxy.h"
 #include "components/autofill/content/renderer/autofill_agent.h"
 #include "components/autofill/content/renderer/password_autofill_agent.h"
 #include "components/autofill/content/renderer/password_generation_agent.h"
 #include "components/content_settings/core/common/content_settings_pattern.h"
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_headers.h"
 #include "components/dom_distiller/core/url_constants.h"
 #include "components/nacl/renderer/ppb_nacl_private.h"
 #include "components/nacl/renderer/ppb_nacl_private_impl.h"
 #include "components/network_hints/renderer/prescient_networking_dispatcher.h"
+#include "components/page_load_metrics/renderer/page_load_metrics_render_frame_observer.h"
 #include "components/password_manager/content/renderer/credential_manager_client.h"
 #include "components/pdf/renderer/pepper_pdf_host.h"
 #include "components/plugins/renderer/mobile_youtube_plugin.h"
 #include "components/signin/core/common/profile_management_switches.h"
 #include "components/version_info/version_info.h"
 #include "components/visitedlink/renderer/visitedlink_slave.h"
 #include "components/web_cache/renderer/web_cache_render_process_observer.h"
 #include "content/public/common/content_constants.h"
 #include "content/public/renderer/plugin_instance_throttler.h"
 #include "content/public/renderer/render_frame.h"
@@ skipping 475 matching lines @@
       new prerender::PrerenderHelper(render_frame);
     }
   }
 
   if (render_frame->GetRenderView()->GetMainRenderFrame() == render_frame) {
     // Only attach NetErrorHelper to the main frame, since only the main frame
     // should get error pages.
     new NetErrorHelper(render_frame);
   }
 
+  if (render_frame->GetRenderView()->GetMainRenderFrame() == render_frame) {
+    // Only attach PageLoadMetricsRenderFrameObserver to the main frame, since
+    // we only want to log page load metrics for the main frame.
+    new page_load_metrics::PageLoadMetricsRenderFrameObserver(render_frame);

[Randy Smith (Not in Mondays), 2015/08/17 19:31:07]

This name is very long, and not particularly in keeping with the names of the
other observers attached to the render_frame.  Based on common usage, I'd
suggest shrinking it to just "PageLoadMetrics" or PageLoadMetricsObserver".  

I have no opinion as to whether standard practice on the creation of a component
suggests the extra namespace, but I wouldn't add the namespace unless you see
some evidence that it is a normal part of creating a component (and/or rely
heavily on guidance from a components/OWNER.)

[Randy Smith (Not in Mondays), 2015/08/17 19:31:07]

Same comment about size and nature of name.  Maybe
PageLoadMetricsSender/Receiver?

[Bryan McQuade, 2015/08/25 13:53:35]

I'm mixed on using the Sender/Receiver name. I think it's helpful to document
the main interface the class implements in the name (self documenting), and I
now have a helper 'PageTimingMetricsSender' class that's technically responsible
for sending the metrics, so I feel that calling the RFO a 'Sender' is not quite
right. I haven't renamed these classes yet though - will do that shortly.

[Bryan McQuade, 2015/08/25 13:53:35]

Agree about shortening names. I used these very verbose names because I have
seen the Windows linker blow up if two object files have the same name, and I
wanted to choose sufficiently unique names to avoid that. But I don't actually
know if Chromium windows builds suffer from this issue - I'll find out. If not,
my inclination is to name these page_load_metrics::MainRenderFrameObserver
(there's some precedent for including 'RenderFrameObserver' in the names of
these classes i.e. ChromeRenderFrameObserver,
extensions::ExtensionsRenderFrameObserver) and
page_load_metrics::WebContentsObserver (precedent:
offline_pages::OfflinePageWebContentsObserver and
extensions::ChromeExtensionWebContentsObserver). 

RE: the namespace,
https://www.chromium.org/developers/design-documents/browser-components/cookbook
says that components should be enclosed in a namespace with the name of the
component directory.

[Randy Smith (Not in Mondays), 2015/08/26 19:11:48]

This all SG TM, though I don't consider myself the authority on these issues
(i.e. in case of disagreement, upvote hypnoOWNERS, ignore Randy :-}).

+  }
+
   PasswordAutofillAgent* password_autofill_agent =
       new PasswordAutofillAgent(render_frame);
   PasswordGenerationAgent* password_generation_agent =
       new PasswordGenerationAgent(render_frame, password_autofill_agent);
   new AutofillAgent(render_frame, password_autofill_agent,
                     password_generation_agent);
 }
 
 void ChromeContentRendererClient::RenderViewCreated(
     content::RenderView* render_view) {
@@ skipping 1055 matching lines @@
   WebString header_key(ASCIIToUTF16(
       data_reduction_proxy::chrome_proxy_header()));
   if (!response.httpHeaderField(header_key).isNull() &&
       response.httpHeaderField(header_key).utf8().find(
           data_reduction_proxy::chrome_proxy_lo_fi_directive()) !=
               std::string::npos) {
     (*properties)[data_reduction_proxy::chrome_proxy_header()] =
         data_reduction_proxy::chrome_proxy_lo_fi_directive();
   }
 }