Create a RegisterViewAssociate method in ViewManager

services/ui/view_manager/view_associate_table.cc

Index: services/ui/view_manager/view_associate_table.cc
diff --git a/services/ui/view_manager/view_associate_table.cc b/services/ui/view_manager/view_associate_table.cc
index 2088696f1a0193f8360a95b8d90c75dacd150a61..1b3e4193ebcc118b01208855290277f57a8a8403 100644
--- a/services/ui/view_manager/view_associate_table.cc
+++ b/services/ui/view_manager/view_associate_table.cc
@@ -33,22 +33,43 @@ void ViewAssociateTable::ConnectAssociates(
 
   for (auto& url : urls) {
     DVLOG(1) << "Connecting to view associate: url=" << url;
-    associates_.emplace_back(new AssociateData(url, inspector));
-    AssociateData* data = associates_.back().get();
 
-    mojo::ConnectToService(app_impl->shell(), url, GetProxy(&data->associate));
-    data->associate.set_connection_error_handler(
-        base::Bind(connection_error_callback, url));
+    mojo::ui::ViewAssociatePtr associate;
+    mojo::ConnectToService(app_impl->shell(), url, GetProxy(&associate));
+
+    // Wire up the associate to us
+    RegisterViewAssociate(inspector, associate.Pass(),
+                          connection_error_callback);
+  }
+}
+
+void ViewAssociateTable::RegisterViewAssociate(
+    mojo::ui::ViewInspector* inspector,
+    mojo::ui::ViewAssociatePtr associate,
+    const AssociateConnectionErrorCallback& connection_error_callback) {
+  DCHECK(inspector);
+  DCHECK(associate.is_bound());
 
-    mojo::ui::ViewInspectorPtr inspector;
-    data->inspector_binding.Bind(GetProxy(&inspector));
-    data->associate->Connect(
-        inspector.Pass(),
-        base::Bind(&ViewAssociateTable::OnConnected, base::Unretained(this),
-                   pending_connection_count_));
+  associates_.emplace_back(new AssociateData(associate.Pass(), inspector));

[jeffbrown, 2016/05/11 23:44:19]

for debugging, it might be useful if each associate had a label (like the View
and ViewTree labels)

[mikejurka, 2016/05/16 23:35:18]

how would we get/generate the label? should it be passed in with
RegisterViewAssociate? I was passing in a "url" before, that could be a label
instead.

+  AssociateData* data = associates_.back().get();
 
-    pending_connection_count_++;
+  if (connection_error_callback.is_null()) {

[jeffbrown, 2016/05/11 23:44:20]

I think this check is going to go away when we move the code that connects to
associates out of ViewManager and into Launcher.  Do you want to do that in this
patch or as a follow-up?

[mikejurka, 2016/05/16 23:35:18]

follow-up since the existing code should still work for now

+    // set it to use our error handler
+    data->associate.set_connection_error_handler(
+        base::Bind(&ViewAssociateTable::OnAssociateConnectionError,
+                   base::Unretained(this), data));
   }
+
+  // connect the associate to our view inspector

[jeffbrown, 2016/05/11 23:44:19]

style nit: Treat comments as complete sentences.

https://google.github.io/styleguide/cppguide.html#Punctuation,_Spelling_and_G...

[mikejurka, 2016/05/16 23:35:18]

Done.

+  mojo::ui::ViewInspectorPtr inspector_ptr;
+  data->inspector_binding.Bind(GetProxy(&inspector_ptr));
+  data->associate->Connect(
+      inspector_ptr.Pass(),
+      base::Bind(&ViewAssociateTable::OnConnected, base::Unretained(this),
+                 pending_connection_count_));
+
+  // wait for the associate to connect to our view inspector

[jeffbrown, 2016/05/11 23:44:16]

Technically we're waiting for it to give us back its AssociateInfo in response
to the call to Connect.  It might not do anything with the ViewInspector (could
even throw it away if it doesn't need it).

+  pending_connection_count_++;
 }
 
 void ViewAssociateTable::ConnectToViewService(
@@ -67,11 +88,11 @@ void ViewAssociateTable::ConnectToViewService(
     DCHECK(data->info);
     if (Contains(data->info->view_service_names, service_name)) {
       DVLOG(2) << "Connecting to view service: view_token=" << view_token
-               << ", service_name=" << service_name
-               << ", associate_url=" << data->url;
+               << ", service_name=" << service_name;
       DCHECK(data->associate);
       data->associate->ConnectToViewService(view_token.Pass(), service_name,
                                             client_handle.Pass());
+
       return;
     }
   }
@@ -81,6 +102,19 @@ void ViewAssociateTable::ConnectToViewService(
   // Allow pipe to be closed as an indication of failure.
 }
 
+void ViewAssociateTable::OnAssociateConnectionError(
+    AssociateData* associate_data) {
+  // Remove associate from our list
+  for (auto it = associates_.begin(); it != associates_.end(); it++) {
+    AssociateData* data = it->get();
+    if (associate_data == data) {
+      DVLOG(2) << "ViewAssociate disconnected, removing from table";
+      associates_.erase(it);
+      break;
+    }
+  }
+}
+
 void ViewAssociateTable::ConnectToViewTreeService(
     mojo::ui::ViewTreeTokenPtr view_tree_token,
     const mojo::String& service_name,
@@ -97,8 +131,7 @@ void ViewAssociateTable::ConnectToViewTreeService(
     DCHECK(data->info);
     if (Contains(data->info->view_tree_service_names, service_name)) {
       DVLOG(2) << "Connecting to view tree service: view_tree_token="
-               << view_tree_token << ", service_name=" << service_name
-               << ", associate_url=" << data->url;

[jeffbrown, 2016/05/11 23:44:20]

Yeah, it would be really useful to be able to print a label here.  I guess
everywhere you saw url before you could replace with label.

+               << view_tree_token << ", service_name=" << service_name;
       DCHECK(data->associate);
       data->associate->ConnectToViewTreeService(
           view_tree_token.Pass(), service_name, client_handle.Pass());
@@ -117,8 +150,7 @@ void ViewAssociateTable::OnConnected(uint32_t index,
   DCHECK(pending_connection_count_);
   DCHECK(!associates_[index]->info);
 
-  DVLOG(1) << "Connected to view associate: url=" << associates_[index]->url
-           << ", info=" << info;
+  DVLOG(1) << "Connected to view associate, info=" << info;
   associates_[index]->info = info.Pass();
 
   pending_connection_count_--;
@@ -135,9 +167,9 @@ void ViewAssociateTable::CompleteDeferredWork() {
 }
 
 ViewAssociateTable::AssociateData::AssociateData(
-    const std::string& url,
+    mojo::ui::ViewAssociatePtr associate,
     mojo::ui::ViewInspector* inspector)
-    : url(url), inspector_binding(inspector) {}
+    : associate(associate.Pass()), inspector_binding(inspector) {}
 
 ViewAssociateTable::AssociateData::~AssociateData() {}