Ensure that PageSignalReceiver creates at most one mojo channel.

This changes the guard of lazy mojo channel construction from the one
that is based on observer list emptiness to a separate boolean flag.

The existing guard is fragile because the observer list can become
empty after channel construction. See details in the bug and the test.

Bug: 855114
Tbr: fdoray@chromium.org
Change-Id: I5585c3ac2aa6425d9401bebd3a6c132199f2e011
Reviewed-on: https://chromium-review.googlesource.com/1110365Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569565}

chrome/browser/resource_coordinator/page_signal_receiver.cc

@@ -84,7 +84,7 @@ void PageSignalReceiver::OnLoadTimePerformanceEstimate(
 void PageSignalReceiver::AddObserver(PageSignalObserver* observer) {
   // When PageSignalReceiver starts to have observer, construct the mojo
   // channel.
-  if (!observers_.might_have_observers()) {
+  if (!binding_.is_bound()) {
     content::ServiceManagerConnection* service_manager_connection =
         content::ServiceManagerConnection::GetForProcess();
     // Ensure service_manager is active before trying to connect to it.

chrome/browser/resource_coordinator/page_signal_receiver_unittest.cc

@@ -5,6 +5,8 @@
 #include "chrome/browser/resource_coordinator/page_signal_receiver.h"
 
 #include "chrome/test/base/chrome_render_view_host_test_harness.h"
+#include "content/public/browser/browser_thread.h"
+#include "content/public/common/service_manager_connection.h"
 #include "content/public/test/web_contents_tester.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -79,4 +81,27 @@ TEST_F(PageSignalReceiverUnitTest,
       mojom::LifecycleState::kDiscarded);
 }
 
+// Regression test for crbug.com/855114.
+TEST_F(PageSignalReceiverUnitTest, ConstructMojoChannelOnce) {
+  // Create a dummy service manager.
+  service_manager::mojom::ServicePtr service;
+  content::ServiceManagerConnection::SetForProcess(
+      content::ServiceManagerConnection::Create(
+          mojo::MakeRequest(&service),
+          content::BrowserThread::GetTaskRunnerForThread(
+              content::BrowserThread::IO)));
+  // Add and remove an observer.
+  {
+    TestPageSignalObserver observer1(Action::kObserve, page_cu_id_,
+                                     page_signal_receiver_.get());
+  }
+  // Add and remove another observer. This causes the page signal receiver to
+  // construct the mojo channel again because the observer list is empty.
+  {
+    TestPageSignalObserver observer2(Action::kObserve, page_cu_id_,
+                                     page_signal_receiver_.get());
+  }
+  content::ServiceManagerConnection::DestroyForProcess();
+}
+
 }  // namespace resource_coordinator