PageLoadMetrics renderer and browser implementation (deprecated)

chrome/renderer/chrome_content_renderer_client.cc

Index: chrome/renderer/chrome_content_renderer_client.cc
diff --git a/chrome/renderer/chrome_content_renderer_client.cc b/chrome/renderer/chrome_content_renderer_client.cc
index cfb68bf1308749ea7256b5468f71e29d27643311..e4c4b534f502530f0c2393f1cf0d1a4c86617221 100644
--- a/chrome/renderer/chrome_content_renderer_client.cc
+++ b/chrome/renderer/chrome_content_renderer_client.cc
@@ -64,6 +64,7 @@
 #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"
@@ -559,6 +560,12 @@ void ChromeContentRendererClient::RenderFrameCreated(
     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 =