[Prerender] first contentful paint histograms.

chrome/browser/prerender/prerender_test_utils.cc

Index: chrome/browser/prerender/prerender_test_utils.cc
diff --git a/chrome/browser/prerender/prerender_test_utils.cc b/chrome/browser/prerender/prerender_test_utils.cc
index 364bfe579f57baa7b1c17ef274cb65c77f01f73d..930c03b2ec5263b90d61e969be8ded621f156c86 100644
--- a/chrome/browser/prerender/prerender_test_utils.cc
+++ b/chrome/browser/prerender/prerender_test_utils.cc
@@ -11,7 +11,8 @@
 #include <vector>
 
 #include "base/command_line.h"
-#include "base/strings/string_split.h"
+#include "base/memory/ptr_util.h"
+#include "base/run_loop.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/loader/chrome_resource_dispatcher_host_delegate.h"
 #include "chrome/browser/prerender/prerender_manager.h"
@@ -427,6 +428,35 @@ void TestPrerender::OnPrerenderStop(PrerenderContents* contents) {
     load_waiter_->Quit();
 }
 
+// static
+FirstContentfulPaintManagerWaiter* FirstContentfulPaintManagerWaiter::Create(
+    PrerenderManager* manager) {
+  auto fcp_waiter = base::WrapUnique(new FirstContentfulPaintManagerWaiter());

[pasko, 2016/11/17 19:52:16]

WrapUnique is discouraged, is MakeUnique somehow not suitable here?

[mattcary, 2016/11/18 09:21:11]

The constructor for FirstContentfulPaintManagerWaiter is private in order to
ensure that it's only created via this factory function, which means it's not
visible to the unique_ptr<FCPMW> class.

IIRC the unique_ptr discussions, I think this is the most common reason for
WrapUnique. The other may be some sort of polymorphism issue.

+  auto fcp_waiter_ptr = fcp_waiter.get();
+  manager->AddObserver(std::move(fcp_waiter));
+  return fcp_waiter_ptr;
+}
+
+FirstContentfulPaintManagerWaiter::FirstContentfulPaintManagerWaiter()
+    : saw_fcp_(false) {}
+
+FirstContentfulPaintManagerWaiter::~FirstContentfulPaintManagerWaiter() {}
+
+void FirstContentfulPaintManagerWaiter::OnFirstContentfulPaint() {
+  saw_fcp_ = true;
+  if (waiter_)
+    content::BrowserThread::PostTask(content::BrowserThread::UI, FROM_HERE,

[pasko, 2016/11/17 19:52:16]

Confused.

I assumed this class lives on UI thread (please add a comment in the .h to
clarify this). If so, why posting a task into UI thread from UI thread? (I know
it can be necessary, but would like to know whether it was actually necessary)

[mattcary, 2016/11/18 09:21:11]

This is how I think things are happening. I may very well be wrong, so please
correct/improve the idiom:

on UI thread, browser test calls NavigateToDestURL
  ->renderer starts doing its thing in another process
fcp_waiter->Wait() called on UI thread
  ->a message loop is created which blocks

In renderer, the FCP happens. A timing update is sent to the browser, which then
runs OnFirstContentfulPaint in the UI thread
 -> we post the quit closure to our spinning message loop in the same thread

Is it possible to just call waiter_->Quit() directly? I don't have a solid
understanding of Chrome's message loops.

+                                     waiter_->QuitClosure());
+}
+
+void FirstContentfulPaintManagerWaiter::Wait() {
+  if (saw_fcp_)
+    return;
+  waiter_ = base::MakeUnique<base::RunLoop>();
+  waiter_->Run();
+  waiter_.reset();
+}
+
 TestPrerenderContentsFactory::TestPrerenderContentsFactory() {}
 
 TestPrerenderContentsFactory::~TestPrerenderContentsFactory() {