Fix invalid initializer in InterfaceRegistry

Subtle: It's not OK to take the address of client_handle_ before
delegating to the other ctor impl, because that ctor will implicitly
reinitialize client_handle_'s memory.

Not sure if this new code is the behavior we actually want
InterfaceRegistry to have, but it seems reasonable and nobody's
using it just yet.

BUG=None
R=ben@chromium.org

Review URL: https://codereview.chromium.org/1783183002

Cr-Commit-Position: refs/heads/master@{#380652}

2 files changed, 10 insertions, 12 deletions

diff --git a/mojo/shell/public/cpp/lib/interface_registry.cc b/mojo/shell/public/cpp/lib/interface_registry.cc
index c8f9703..98a91ce 100644
--- a/mojo/shell/public/cpp/lib/interface_registry.cc
+++ b/mojo/shell/public/cpp/lib/interface_registry.cc
@@ -9,7 +9,7 @@
 namespace mojo {
 
 InterfaceRegistry::InterfaceRegistry(Connection* connection)
-    : InterfaceRegistry(GetProxy(&client_handle_), connection) {}
+    : InterfaceRegistry(nullptr, connection) {}
 
 InterfaceRegistry::InterfaceRegistry(
     shell::mojom::InterfaceProviderRequest request,
@@ -17,8 +17,9 @@ InterfaceRegistry::InterfaceRegistry(
     : binding_(this),
       connection_(connection),
       default_binder_(nullptr) {
-  if (request.is_pending())
-    binding_.Bind(std::move(request));
+  if (!request.is_pending())
+    request = GetProxy(&client_handle_);
+  binding_.Bind(std::move(request));
 }
 
 InterfaceRegistry::~InterfaceRegistry() {
diff --git a/mojo/shell/public/cpp/tests/interface_registry_unittest.cc b/mojo/shell/public/cpp/tests/interface_registry_unittest.cc
index d93f63c..cf6441f 100644
--- a/mojo/shell/public/cpp/tests/interface_registry_unittest.cc
+++ b/mojo/shell/public/cpp/tests/interface_registry_unittest.cc
@@ -5,6 +5,7 @@
 #include "mojo/shell/public/cpp/interface_registry.h"
 
 #include "base/memory/scoped_ptr.h"
+#include "base/message_loop/message_loop.h"
 #include "mojo/shell/public/cpp/interface_binder.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -25,12 +26,12 @@ class TestBinder : public InterfaceBinder {
 };
 
 TEST(InterfaceRegistryTest, Ownership) {
+  base::MessageLoop message_loop_;
   int delete_count = 0;
 
   // Destruction.
   {
-    shell::mojom::InterfaceProviderRequest ir;
-    InterfaceRegistry registry(std::move(ir), nullptr);
+    InterfaceRegistry registry(nullptr);
     InterfaceRegistry::TestApi test_api(&registry);
     test_api.SetInterfaceBinderForName(new TestBinder(&delete_count), "TC1");
   }
@@ -38,9 +39,7 @@ TEST(InterfaceRegistryTest, Ownership) {
 
   // Removal.
   {
-    shell::mojom::InterfaceProviderRequest ir;
-    scoped_ptr<InterfaceRegistry> registry(
-        new InterfaceRegistry(std::move(ir), nullptr));
+    scoped_ptr<InterfaceRegistry> registry(new InterfaceRegistry(nullptr));
     InterfaceBinder* b = new TestBinder(&delete_count);
     InterfaceRegistry::TestApi test_api(registry.get());
     test_api.SetInterfaceBinderForName(b, "TC1");
@@ -51,8 +50,7 @@ TEST(InterfaceRegistryTest, Ownership) {
 
   // Multiple.
   {
-    shell::mojom::InterfaceProviderRequest ir;
-    InterfaceRegistry registry(std::move(ir), nullptr);
+    InterfaceRegistry registry(nullptr);
     InterfaceRegistry::TestApi test_api(&registry);
     test_api.SetInterfaceBinderForName(new TestBinder(&delete_count), "TC1");
     test_api.SetInterfaceBinderForName(new TestBinder(&delete_count), "TC2");
@@ -61,8 +59,7 @@ TEST(InterfaceRegistryTest, Ownership) {
 
   // Re-addition.
   {
-    shell::mojom::InterfaceProviderRequest ir;
-    InterfaceRegistry registry(std::move(ir), nullptr);
+    InterfaceRegistry registry(nullptr);
     InterfaceRegistry::TestApi test_api(&registry);
     test_api.SetInterfaceBinderForName(new TestBinder(&delete_count), "TC1");
     test_api.SetInterfaceBinderForName(new TestBinder(&delete_count), "TC1");