Changes view manager test to connect via shell

Previously code was linking with ViewManager, now I'm connecting
through the shell. As part of this I'm adding a general class that
should make it easy to connect to any server through the shell.

For the time being I'm making the shell explicitly link with the
viewmanager. Eventually that'll have to change.

BUG=365012
TEST=covered by tests
R=davemoore@chromium.org

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@266937 0039d316-1c4b-4281-b951-d872f2087c98

mojo/mojo.gyp

@@ -438,6 +438,7 @@
       'dependencies': [
         '../base/base.gyp:base',
         '../base/base.gyp:base_static',
+        '../base/third_party/dynamic_annotations/dynamic_annotations.gyp:dynamic_annotations',
         '../net/net.gyp:net',
         '../url/url.gyp:url_lib',
         'mojo_external_service_bindings',
@@ -501,6 +502,30 @@
             '../dbus/dbus.gyp:dbus',
           ],
         }],
+        ['use_aura==1', {
+          'dependencies': [
+            # These are only necessary as long as we hard code use of ViewManager.
+            '../skia/skia.gyp:skia',
+            'mojo_shell_client',
+            'mojo_view_manager',
+          ],
+        }],
+      ],
+    },
+    {
+      'target_name': 'mojo_shell_test_support',
+      'type': 'static_library',
+      'dependencies': [
+        '../base/base.gyp:base',
+        '../base/base.gyp:base_static',
+        '../url/url.gyp:url_lib',
+        'mojo_service_manager',
+        'mojo_shell_lib',
+        'mojo_system_impl',
+      ],
+      'sources': [
+        'shell/shell_test_helper.cc',
+        'shell/shell_test_helper.h',
       ],
     },
     {

mojo/mojo_services.gypi

@@ -216,14 +216,14 @@
           'type': 'executable',
           'dependencies': [
             '../base/base.gyp:base',
+            '../base/base.gyp:run_all_unittests',
             '../skia/skia.gyp:skia',
             '../testing/gtest.gyp:gtest',
             '../ui/aura/aura.gyp:aura',
             'mojo_environment_chromium',
-            'mojo_run_all_unittests',
             'mojo_shell_client',
+            'mojo_shell_test_support',
             'mojo_system_impl',
-            'mojo_view_manager',
             'mojo_view_manager_bindings',
           ],
           'sources': [

mojo/service_manager/service_manager.cc

@@ -58,14 +58,59 @@ class ServiceManager::ServiceFactory : public Shell, public ErrorHandler {
   ServiceManager* const manager_;
   const GURL url_;
   RemotePtr<ShellClient> shell_client_;
+
   DISALLOW_COPY_AND_ASSIGN(ServiceFactory);
 };
 
+class ServiceManager::TestAPI::TestShellConnection
+    : public Shell,
+      public ErrorHandler {
+ public:
+  explicit TestShellConnection(ServiceManager* manager) : manager_(manager) {
+    InterfacePipe<Shell> pipe;
+    shell_client_.reset(pipe.handle_to_peer.Pass(), this, this);
+    shell_handle_ = pipe.handle_to_self.Pass();
+  }
+  virtual ~TestShellConnection() {}
+
+  ScopedShellHandle GetShellHandle() {
+    return shell_handle_.Pass();
+  }
+
+  // Shell:
+  virtual void Connect(const String& url,
+                       ScopedMessagePipeHandle client_pipe) OVERRIDE {
+    manager_->Connect(GURL(url.To<std::string>()), client_pipe.Pass());
+  }
+
+  virtual void OnError() OVERRIDE {
+  }
+
+ private:
+  ServiceManager* manager_;
+  RemotePtr<ShellClient> shell_client_;
+  ScopedShellHandle shell_handle_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestShellConnection);
+};
+
 // static
+ServiceManager::TestAPI::TestAPI(ServiceManager* manager) : manager_(manager) {
+}
+
+ServiceManager::TestAPI::~TestAPI() {
+}
+
 bool ServiceManager::TestAPI::HasCreatedInstance() {
   return has_created_instance;
 }
 
+ScopedShellHandle ServiceManager::TestAPI::GetShellHandle() {
+  if (!shell_connection_.get())
+    shell_connection_.reset(new TestShellConnection(manager_));
+  return shell_connection_->GetShellHandle().Pass();
+}
+
 bool ServiceManager::TestAPI::HasFactoryForURL(const GURL& url) const {
   return manager_->url_to_service_factory_.find(url) !=
       manager_->url_to_service_factory_.end();

mojo/service_manager/service_manager.h

@@ -16,27 +16,32 @@
 #include "mojo/service_manager/service_manager_export.h"
 #include "url/gurl.h"
 
-namespace content {
-  class MojoTest;
-}
-
 namespace mojo {
 
 class MOJO_SERVICE_MANAGER_EXPORT ServiceManager {
  public:
   // API for testing.
   class MOJO_SERVICE_MANAGER_EXPORT TestAPI {
-   private:
-    friend class ServiceManagerTest;
-    friend class content::MojoTest;
+   public:
+    explicit TestAPI(ServiceManager* manager);
+    ~TestAPI();
+
+    // Returns a handle to the shell.
+    ScopedShellHandle GetShellHandle();
 
-    explicit TestAPI(ServiceManager* manager) : manager_(manager) {}
     // Returns true if the shared instance has been created.
     static bool HasCreatedInstance();
     // Returns true if there is a ServiceFactory for this URL.
     bool HasFactoryForURL(const GURL& url) const;
 
+   private:
+    class TestShellConnection;
+
     ServiceManager* manager_;
+
+    scoped_ptr<TestShellConnection> shell_connection_;
+
+    DISALLOW_COPY_AND_ASSIGN(TestAPI);
   };
 
   // Interface class for debugging only.

mojo/services/view_manager/view_manager_connection_unittest.cc

@@ -2,8 +2,6 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "mojo/services/view_manager/view_manager_connection.h"
-
 #include <string>
 #include <vector>
 
@@ -14,7 +12,8 @@
 #include "base/strings/stringprintf.h"
 #include "mojo/public/cpp/bindings/allocation_scope.h"
 #include "mojo/public/cpp/environment/environment.h"
-#include "mojo/services/view_manager/root_node_manager.h"
+#include "mojo/services/public/interfaces/view_manager/view_manager.mojom.h"
+#include "mojo/shell/shell_test_helper.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace mojo {
@@ -25,6 +24,17 @@ namespace {
 
 base::RunLoop* current_run_loop = NULL;
 
+// TODO(sky): remove and include a common header.
+typedef uint32_t ChangeId;
+
+uint16_t FirstIdFromTransportId(uint32_t id) {
+  return static_cast<uint16_t>((id >> 16) & 0xFFFF);
+}
+
+uint16_t SecondIdFromTransportId(uint32_t id) {
+  return static_cast<uint16_t>(id & 0xFFFF);
+}
+
 // Sets |current_run_loop| and runs it. It is expected that someone else quits
 // the loop.
 void DoRunLoop() {
@@ -50,12 +60,12 @@ void BooleanCallback(bool* result_cache, bool result) {
 
 // Creates an id used for transport from the specified parameters.
 uint32_t CreateNodeId(uint16_t connection_id, uint16_t node_id) {
-  return NodeIdToTransportId(NodeId(connection_id, node_id));
+  return (connection_id << 16) | node_id;
 }
 
 // Creates an id used for transport from the specified parameters.
 uint32_t CreateViewId(uint16_t connection_id, uint16_t view_id) {
-  return ViewIdToTransportId(ViewId(connection_id, view_id));
+  return (connection_id << 16) | view_id;
 }
 
 // Creates a node with the specified id. Returns true on success. Blocks until
@@ -133,8 +143,6 @@ class ViewManagerClientImpl : public ViewManagerClient {
  public:
   ViewManagerClientImpl() : id_(0), quit_count_(0) {}
 
-  void set_quit_count(int count) { quit_count_ = count; }
-
   uint16_t id() const { return id_; }
 
   Changes GetAndClearChanges() {
@@ -143,11 +151,24 @@ class ViewManagerClientImpl : public ViewManagerClient {
     return changes;
   }
 
+  void WaitForId() {
+    if (id_ == 0)
+      DoRunLoop();
+  }
+
+  void DoRunLoopUntilChangesCount(size_t count) {
+    if (changes_.size() >= count)
+      return;
+    quit_count_ = count - changes_.size();
+    DoRunLoop();
+  }
+
  private:
   // ViewManagerClient overrides:
   virtual void OnConnectionEstablished(uint16_t connection_id) OVERRIDE {
     id_ = connection_id;
-    current_run_loop->Quit();
+    if (current_run_loop)
+      current_run_loop->Quit();
   }
   virtual void OnNodeHierarchyChanged(uint32_t node,
                                       uint32_t new_parent,
@@ -182,7 +203,7 @@ class ViewManagerClientImpl : public ViewManagerClient {
   uint16_t id_;
 
   // Used to determine when/if to quit the run loop.
-  int quit_count_;
+  size_t quit_count_;
 
   Changes changes_;
 
@@ -191,47 +212,45 @@ class ViewManagerClientImpl : public ViewManagerClient {
 
 class ViewManagerConnectionTest : public testing::Test {
  public:
-  ViewManagerConnectionTest() : service_factory_(&root_node_manager_) {}
+  ViewManagerConnectionTest() {}
 
   virtual void SetUp() OVERRIDE {
-    InterfacePipe<ViewManagerClient, ViewManager> pipe;
-    view_manager_.reset(pipe.handle_to_peer.Pass(), &client_);
-    connection_.Initialize(
-        &service_factory_,
-        ScopedMessagePipeHandle::From(pipe.handle_to_self.Pass()));
-    // Wait for the id.
-    DoRunLoop();
+    AllocationScope allocation_scope;
+
+    test_helper_.Init();
+
+    InterfacePipe<ViewManager, AnyInterface> pipe;
+    test_helper_.shell()->Connect("mojo:mojo_view_manager",
+                                  pipe.handle_to_peer.Pass());
+    view_manager_.reset(pipe.handle_to_self.Pass(), &client_);
+
+    client_.WaitForId();
   }
 
  protected:
   // Creates a second connection to the viewmanager.
   void EstablishSecondConnection() {
-    connection2_.reset(new ViewManagerConnection);
-    InterfacePipe<ViewManagerClient, ViewManager> pipe;
-    view_manager2_.reset(pipe.handle_to_peer.Pass(), &client2_);
-    connection2_->Initialize(
-        &service_factory_,
-        ScopedMessagePipeHandle::From(pipe.handle_to_self.Pass()));
-    // Wait for the id.
-    DoRunLoop();
+    AllocationScope allocation_scope;
+    InterfacePipe<ViewManager, AnyInterface> pipe;
+    test_helper_.shell()->Connect("mojo:mojo_view_manager",
+                                  pipe.handle_to_peer.Pass());
+    view_manager2_.reset(pipe.handle_to_self.Pass(), &client2_);
+
+    client2_.WaitForId();
   }
 
   void DestroySecondConnection() {
-    connection2_.reset();
     view_manager2_.reset();
   }
 
-  Environment env_;
   base::MessageLoop loop_;
-  RootNodeManager root_node_manager_;
-  ServiceConnector<ViewManagerConnection, RootNodeManager> service_factory_;
-  ViewManagerConnection connection_;
+  shell::ShellTestHelper test_helper_;
+
   ViewManagerClientImpl client_;
   RemotePtr<ViewManager> view_manager_;
 
   ViewManagerClientImpl client2_;
   RemotePtr<ViewManager> view_manager2_;
-  scoped_ptr<ViewManagerConnection> connection2_;
 
   DISALLOW_COPY_AND_ASSIGN(ViewManagerConnectionTest);
 };
@@ -317,12 +336,8 @@ TEST_F(ViewManagerConnectionTest, AddRemoveNotifyMultipleConnections) {
 
   // Second client should also have received the change.
   {
+    client2_.DoRunLoopUntilChangesCount(1);
     Changes changes(client2_.GetAndClearChanges());
-    if (changes.empty()) {
-      client2_.set_quit_count(1);
-      DoRunLoop();
-      changes = client2_.GetAndClearChanges();
-    }
     ASSERT_EQ(1u, changes.size());
     EXPECT_EQ("change_id=0 node=1,2 new_parent=1,1 old_parent=null",
               changes[0]);
@@ -500,10 +515,12 @@ TEST_F(ViewManagerConnectionTest, SetViewFromSecondConnection) {
                         CreateNodeId(client_.id(), 1),
                         CreateViewId(client2_.id(), 51),
                         22));
+    client_.DoRunLoopUntilChangesCount(1);
     Changes changes(client_.GetAndClearChanges());
     ASSERT_EQ(1u, changes.size());
     EXPECT_EQ("change_id=0 node=1,1 new_view=2,51 old_view=null", changes[0]);
 
+    client2_.DoRunLoopUntilChangesCount(1);
     changes = client2_.GetAndClearChanges();
     ASSERT_EQ(1u, changes.size());
     EXPECT_EQ("change_id=22 node=1,1 new_view=2,51 old_view=null", changes[0]);
@@ -512,8 +529,7 @@ TEST_F(ViewManagerConnectionTest, SetViewFromSecondConnection) {
   // Shutdown the second connection and verify view is removed.
   {
     DestroySecondConnection();
-    client_.set_quit_count(1);
-    DoRunLoop();
+    client_.DoRunLoopUntilChangesCount(1);
 
     Changes changes(client_.GetAndClearChanges());
     ASSERT_EQ(1u, changes.size());

mojo/shell/context.cc

@@ -4,10 +4,13 @@
 
 #include "mojo/shell/context.h"
 
-#include "base/command_line.h"
 #include "build/build_config.h"
+#include "base/command_line.h"
+#include "base/lazy_instance.h"
+#include "base/memory/scoped_vector.h"
 #include "mojo/embedder/embedder.h"
 #include "mojo/gles2/gles2_support_impl.h"
+#include "mojo/public/cpp/shell/application.h"
 #include "mojo/service_manager/service_loader.h"
 #include "mojo/service_manager/service_manager.h"
 #include "mojo/services/native_viewport/native_viewport_service.h"
@@ -22,8 +25,62 @@
 #include "mojo/shell/dbus_service_loader_linux.h"
 #endif  // defined(OS_LINUX)
 
+#if defined(USE_AURA)
+#include "mojo/services/view_manager/root_node_manager.h"
+#include "mojo/services/view_manager/view_manager_connection.h"
+#endif
+
 namespace mojo {
 namespace shell {
+namespace {
+
+// Used to ensure we only init once.
+class Setup {
+ public:
+  Setup() {
+    embedder::Init();
+    gles2::GLES2SupportImpl::Init();
+  }
+
+  ~Setup() {
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(Setup);
+};
+
+static base::LazyInstance<Setup> setup = LAZY_INSTANCE_INITIALIZER;
+
+#if defined(USE_AURA)
+class ViewManagerLoader : public ServiceLoader {
+ public:
+  ViewManagerLoader() {}
+  virtual ~ViewManagerLoader() {}
+
+ private:
+  virtual void LoadService(ServiceManager* manager,
+                           const GURL& url,
+                           ScopedShellHandle shell_handle) OVERRIDE {
+    scoped_ptr<Application> app(new Application(shell_handle.Pass()));
+    app->AddServiceConnector(
+        new ServiceConnector<services::view_manager::ViewManagerConnection,
+                             services::view_manager::RootNodeManager>(
+                                 &root_node_manager_));
+    apps_.push_back(app.release());
+  }
+
+  virtual void OnServiceError(ServiceManager* manager,
+                              const GURL& url) OVERRIDE {
+  }
+
+  services::view_manager::RootNodeManager root_node_manager_;
+  ScopedVector<Application> apps_;
+
+  DISALLOW_COPY_AND_ASSIGN(ViewManagerLoader);
+};
+#endif
+
+}  // namespace
 
 class Context::NativeViewportServiceLoader : public ServiceLoader {
  public:
@@ -54,9 +111,7 @@ Context::Context()
               task_runners_.cache_runner(),
               scoped_ptr<net::NetworkDelegate>(new NetworkDelegate()),
               storage_.profile_path()) {
-  embedder::Init();
-  gles2::GLES2SupportImpl::Init();
-
+  setup.Get();
   CommandLine* cmdline = CommandLine::ForCurrentProcess();
   scoped_ptr<DynamicServiceRunnerFactory> runner_factory;
   if (cmdline->HasSwitch(switches::kEnableMultiprocess))
@@ -70,6 +125,12 @@ Context::Context()
   service_manager_.SetLoaderForURL(
       scoped_ptr<ServiceLoader>(new NativeViewportServiceLoader(this)),
       GURL("mojo:mojo_native_viewport_service"));
+#if defined(USE_AURA)
+  // TODO(sky): need a better way to find this. It shouldn't be linked in.
+  service_manager_.SetLoaderForURL(
+      scoped_ptr<ServiceLoader>(new ViewManagerLoader()),
+      GURL("mojo:mojo_view_manager"));
+#endif
 
 #if defined(OS_LINUX)
   service_manager_.SetLoaderForScheme(

mojo/shell/run.cc

@@ -37,7 +37,6 @@ void Run(Context* context) {
     ScopedMessagePipeHandle no_handle;
     context->service_manager()->Connect(GURL(*it), no_handle.Pass());
   }
-  // TODO(davemoore): Currently we leak |service_manager|.
 }
 
 }  // namespace shell

mojo/shell/shell_test_helper.cc

+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "mojo/shell/shell_test_helper.h"
+
+#include "base/command_line.h"
+#include "base/logging.h"
+#include "base/message_loop/message_loop.h"
+#include "mojo/shell/context.h"
+#include "mojo/shell/init.h"
+
+namespace mojo {
+namespace shell {
+
+// State used on the background thread. Be careful, this is created on the main
+// thread than passed to the shell thread. Destruction happens on the shell
+// thread.
+struct ShellTestHelper::State {
+  scoped_ptr<Context> context;
+  scoped_ptr<ServiceManager::TestAPI> test_api;
+  ScopedShellHandle shell_handle;
+};
+
+namespace {
+
+void StartShellOnShellThread(ShellTestHelper::State* state) {
+  state->context.reset(new Context);
+  state->test_api.reset(
+      new ServiceManager::TestAPI(state->context->service_manager()));
+  state->shell_handle = state->test_api->GetShellHandle();
+}
+
+}  // namespace
+
+class ShellTestHelper::TestShellClient : public ShellClient {
+ public:
+  TestShellClient() {}
+  virtual ~TestShellClient() {}
+
+  // ShellClient:
+  virtual void AcceptConnection(
+      const mojo::String& url,
+      ScopedMessagePipeHandle client_handle) OVERRIDE {
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(TestShellClient);
+};
+
+ShellTestHelper::ShellTestHelper()
+    : shell_thread_("Test Shell Thread"),
+      state_(NULL) {
+  CommandLine::Init(0, NULL);
+  mojo::shell::InitializeLogging();
+}
+
+ShellTestHelper::~ShellTestHelper() {
+  if (state_) {
+    // |state_| contains data created on the background thread. Destroy it
+    // there so that there aren't any race conditions.
+    shell_thread_.message_loop()->DeleteSoon(FROM_HERE, state_);
+    state_ = NULL;
+  }
+}
+
+void ShellTestHelper::Init() {
+  DCHECK(!state_);
+  state_ = new State;
+  shell_thread_.Start();
+  shell_thread_.message_loop()->message_loop_proxy()->PostTaskAndReply(
+      FROM_HERE,
+      base::Bind(&StartShellOnShellThread, state_),
+      base::Bind(&ShellTestHelper::OnShellStarted, base::Unretained(this)));
+  run_loop_.reset(new base::RunLoop);
+  run_loop_->Run();
+}
+
+void ShellTestHelper::OnShellStarted() {
+  DCHECK(state_);
+  shell_client_.reset(new TestShellClient);
+  shell_.reset(state_->shell_handle.Pass(), shell_client_.get());
+  run_loop_->Quit();
+}
+
+}  // namespace shell
+}  // namespace mojo

mojo/shell/shell_test_helper.h

+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef MOJO_SHELL_SHELL_TEST_HELPER_
+#define MOJO_SHELL_SHELL_TEST_HELPER_
+
+#include "base/macros.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/run_loop.h"
+#include "base/threading/thread.h"
+#include "mojo/public/cpp/bindings/remote_ptr.h"
+#include "mojo/public/cpp/environment/environment.h"
+#include "mojo/public/interfaces/shell/shell.mojom.h"
+
+namespace base {
+class MessageLoopProxy;
+class RunLoop;
+}
+
+namespace mojo {
+namespace shell {
+
+// ShellTestHelper is useful for tests to establish a connection to the Shell.
+// ShellTestHelper does this by spawning a thread and connecting. Invoke Init()
+// to do this. Once done, shell() returns the handle to the Shell.
+class ShellTestHelper {
+ public:
+  struct State;
+
+  ShellTestHelper();
+  ~ShellTestHelper();
+
+  void Init();
+
+  // Returns a handle to the Shell. ShellTestHelper owns the shell.
+  Shell* shell() { return shell_.get(); }
+
+ private:
+  class TestShellClient;
+
+  // Invoked once connection has been established.
+  void OnShellStarted();
+
+  Environment environment_;
+
+  base::Thread shell_thread_;
+
+  // If non-null we're in Init() and waiting for connection.
+  scoped_ptr<base::RunLoop> run_loop_;
+
+  // See comment in declaration for details.
+  State* state_;
+
+  // Client interface for the shell.
+  scoped_ptr<TestShellClient> shell_client_;
+
+  RemotePtr<Shell> shell_;
+
+  DISALLOW_COPY_AND_ASSIGN(ShellTestHelper);
+};
+
+}  // namespace shell
+}  // namespace mojo
+
+#endif  // MOJO_SHELL_SHELL_TEST_HELPER_